diff --git a/awscli/customizations/agenttoolkit/atomic.py b/awscli/customizations/agenttoolkit/atomic.py index 7f27a1f90c9b..66ab7bbc350a 100644 --- a/awscli/customizations/agenttoolkit/atomic.py +++ b/awscli/customizations/agenttoolkit/atomic.py @@ -116,18 +116,21 @@ def _rename(src, dst): _call_with_windows_retry(os.rename, src, dst) -def write_file_atomic(path, content, prefix): +def write_file_atomic(path, content, prefix, mode=None): """Replace ``path`` with ``content`` without ever leaving it partial. The temporary file is created as ``*.tmp`` next to ``path`` - with owner-only permissions, and removed if anything fails. + with owner-only permissions, and removed if anything fails. ``content`` + is written as is, without newline translation. """ fd, tmp_path = tempfile.mkstemp( dir=os.path.dirname(path), prefix=prefix, suffix='.tmp' ) try: - with os.fdopen(fd, 'w', encoding='utf-8') as f: + with os.fdopen(fd, 'w', encoding='utf-8', newline='') as f: f.write(content) + if mode is not None: + os.chmod(tmp_path, mode) _call_with_windows_retry(os.replace, tmp_path, path) except BaseException: with contextlib.suppress(OSError): diff --git a/awscli/customizations/agenttoolkit/configure.py b/awscli/customizations/agenttoolkit/configure.py index bc74aaba4f5a..00f36be58b9b 100644 --- a/awscli/customizations/agenttoolkit/configure.py +++ b/awscli/customizations/agenttoolkit/configure.py @@ -11,6 +11,7 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. import io +import os import sys from awscli.customizations.agenttoolkit.agents import ( @@ -20,6 +21,22 @@ collapse_home, universal_first, ) +from awscli.customizations.agenttoolkit.hooks import ( + CODEX_TRUST_NOTE, + KIRO_AGENT_NOTE, + KIRO_NO_AGENT_NOTE, + KIRO_VERSION_NOTE, + HookAction, + format_kiro_agent_manual_entry, + format_manual_entry, + hook_config_path, + install_hook, + install_kiro_agent_hook, + is_hook_supported, + kiro_default_agent_path, + remove_hook, + remove_kiro_agent_hooks, +) from awscli.customizations.agenttoolkit.state import ( StateFileError, is_auto_update_enabled, @@ -48,6 +65,12 @@ ) +_INSTALL_STATUS = { + HookAction.INSTALLED: 'hook installed', + HookAction.ALREADY_INSTALLED: 'already installed', +} + + class ConfigureAgentToolkitCommand(BasicCommand): NAME = 'agent-toolkit' DESCRIPTION = ( @@ -141,7 +164,7 @@ def _run_main(self, parsed_args, parsed_globals): client = self._client or create_client(self._session, parsed_globals) self._install_default_skills(install_targets, client, yes=yes) self._configure_mcp(selected_agents, yes=yes) - self._configure_auto_update(yes=yes) + self._configure_auto_update(selected_agents, yes=yes) uni_print( '\nYou can discover additional skills with ' @@ -210,7 +233,7 @@ def _install_default_skills(self, selected_agents, client, yes=False): '\nNo skills were installed successfully.\n', self._stream ) - def _configure_auto_update(self, yes=False): + def _configure_auto_update(self, agents, yes=False): # This wizard is the only place allowed to change the setting, so it # always asks, letting customers opt out as well as in. currently_enabled = is_auto_update_enabled() @@ -220,20 +243,165 @@ def _configure_auto_update(self, yes=False): else AUTO_UPDATE_PROMPT ) enabled = yes or yes_no_choice(prompt, default=True) - if enabled == currently_enabled: - return + if enabled != currently_enabled: + try: + set_auto_update(enabled) + except StateFileError as e: + uni_print( + '\nCould not save the automatic skill update setting ' + f'to {e}\n', + self._stream, + ) + return + if enabled: + uni_print('\nAutomatic skill updates enabled.\n', self._stream) + else: + uni_print( + '\nAutomatic skill updates turned off.\n', self._stream + ) + if enabled: + self._install_hooks(agents) + else: + self._remove_hooks(agents) + + def _install_hooks(self, agents): + uni_print('\nAutomatic update hooks:\n', self._stream) + unsupported = [] + for agent in agents: + if not is_hook_supported(agent.config.id): + unsupported.append(agent) + continue + try: + action, path = install_hook(agent) + except OSError as e: + path = hook_config_path(agent.config, agent.base_dir) + self._print_hook_line( + '\u2717', agent, path, f'could not install the hook ({e})' + ) + continue + if action is HookAction.UNREADABLE: + self._print_hook_line( + '\u2717', + agent, + path, + 'could not be updated because it is not plain JSON', + ) + uni_print( + format_manual_entry(agent.config.id, collapse_home(path)), + self._stream, + ) + continue + self._print_hook_line( + '\u2713', agent, path, _INSTALL_STATUS[action] + ) + if agent.config.id == 'codex': + uni_print(f' {CODEX_TRUST_NOTE}\n', self._stream) + elif agent.config.id == 'kiro': + uni_print(f' {KIRO_VERSION_NOTE}\n', self._stream) + self._install_kiro_agent_hook(agent) + for agent in unsupported: + uni_print( + f' \u2717 {agent.display_name} ' + '(automatic updates not supported yet)\n', + self._stream, + ) + + def _install_kiro_agent_hook(self, agent): try: - set_auto_update(enabled) - except StateFileError as e: + action, path = install_kiro_agent_hook(agent) + except OSError as e: + path = kiro_default_agent_path(agent) or os.path.join( + agent.base_dir, 'agents' + ) + self._print_hook_line( + '\u2717', agent, path, f'could not install the hook ({e})' + ) + return + if path is None: uni_print( - f'\nCould not save the automatic skill update setting to {e}\n', + f' \u2717 {agent.display_name} \u2014 {KIRO_NO_AGENT_NOTE}\n' + + format_kiro_agent_manual_entry( + collapse_home( + os.path.join(agent.base_dir, 'agents', '.json') + ) + ), self._stream, ) return - if enabled: - uni_print('\nAutomatic skill updates enabled.\n', self._stream) + if action is HookAction.UNREADABLE: + self._print_hook_line( + '\u2717', + agent, + path, + 'could not be updated because it is not plain JSON', + ) + uni_print( + format_kiro_agent_manual_entry(collapse_home(path)), + self._stream, + ) + elif action is HookAction.NOT_NEEDED: + self._print_hook_line( + '\u2713', agent, path, 'not needed for V3 agents' + ) + return else: - uni_print('\nAutomatic skill updates turned off.\n', self._stream) + self._print_hook_line( + '\u2713', agent, path, _INSTALL_STATUS[action] + ) + uni_print(f' {KIRO_AGENT_NOTE}\n', self._stream) + + def _remove_hooks(self, agents): + lines = [] + for agent in agents: + config = agent.config + if not is_hook_supported(config.id): + continue + try: + results = [remove_hook(agent)] + except OSError as e: + path = hook_config_path(agent.config, agent.base_dir) + lines.append( + ('\u2717', agent, path, f'could not remove the hook ({e})') + ) + continue + if config.id == 'kiro': + try: + results.extend(remove_kiro_agent_hooks(agent)) + except OSError as e: + path = os.path.join(agent.base_dir, 'agents') + lines.append( + ( + '\u2717', + agent, + path, + f'could not remove the hook ({e})', + ) + ) + for action, path in results: + if action is HookAction.REMOVED: + lines.append(('\u2713', agent, path, 'hook removed')) + elif action is HookAction.UNREADABLE: + lines.append( + ( + '\u2717', + agent, + path, + 'could not be updated; remove the "aws ' + 'agent-toolkit update-skill" hook from it by hand', + ) + ) + if not lines: + return + uni_print('\nAutomatic update hooks:\n', self._stream) + for line in lines: + self._print_hook_line(*line) + + def _print_hook_line(self, mark, agent, path, status): + uni_print( + f' {mark} {agent.display_name} \u2014 ' + f'{collapse_home(path)}: {status}\n', + self._stream, + ) def _configure_mcp(self, agents, yes=False): if not yes and not yes_no_choice( diff --git a/awscli/customizations/agenttoolkit/hooks.py b/awscli/customizations/agenttoolkit/hooks.py index 44950b8f7288..5cac84a741e0 100644 --- a/awscli/customizations/agenttoolkit/hooks.py +++ b/awscli/customizations/agenttoolkit/hooks.py @@ -11,36 +11,177 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. +import dataclasses +import enum +import errno +import glob import json import os +import stat +from typing import Callable, Optional + +from awscli.compat import is_windows +from awscli.customizations.agenttoolkit.atomic import write_file_atomic + +HOOK_NAME = 'aws-agent-toolkit' +_COMMAND_MARKERS = ('agent-toolkit update-skill', '--hook') +_TEMP_PREFIX = '.aws-agent-toolkit-' + + +CODEX_TRUST_NOTE = ( + 'Open Codex and run /hooks to trust the hook. It does not run until ' + 'you do.' +) +KIRO_VERSION_NOTE = 'Used by Kiro IDE 1.0.182 or later and kiro-cli --v3.' +KIRO_AGENT_NOTE = ( + 'kiro-cli 2.x runs it in interactive sessions of this agent only.' +) +KIRO_NO_AGENT_NOTE = ( + 'kiro-cli 2.x only runs hooks from agent configs and no default agent ' + 'is set.' +) HOOK_ERROR_MESSAGE = ( 'AWS agent skills could not be checked for updates; run ' '"aws agent-toolkit update-skill --all" for details.' ) -# The file each agent's session start hook lives in, relative to the agent's -# base directory. Hook runs are recorded against it. -_HOOK_FILENAMES = { + +class HookAction(enum.Enum): + INSTALLED = 'installed' + ALREADY_INSTALLED = 'already_installed' + REMOVED = 'removed' + NOT_INSTALLED = 'not_installed' + UNREADABLE = 'unreadable' + NOT_SUPPORTED = 'not_supported' + NOT_NEEDED = 'not_needed' + + +@dataclasses.dataclass(frozen=True) +class _HookFormat: + filename: str + """Hook file, relative to the agent's base directory.""" + + event: str + """Key under ``hooks`` holding the session start entries.""" + + matcher: Optional[str] + """Matcher of the group our handler is in. ``None`` for agents whose + event list holds handlers directly.""" + + handler: Callable[[str], dict] + """Builds our handler from the hook command.""" + + version: Optional[int] = None + """Top-level ``version`` the file needs, if any.""" + + +_HOOK_FORMATS = { # https://code.claude.com/docs/en/hooks - 'claude-code': 'settings.json', + 'claude-code': _HookFormat( + filename='settings.json', + event='SessionStart', + matcher='startup', + handler=lambda command: { + 'type': 'command', + 'command': command, + 'timeout': 60, + }, + ), # https://github.com/google-gemini/gemini-cli/blob/main/docs/hooks/reference.md - 'gemini-cli': 'settings.json', + # Timeouts are in milliseconds. + 'gemini-cli': _HookFormat( + filename='settings.json', + event='SessionStart', + matcher='*', + handler=lambda command: { + 'name': HOOK_NAME, + 'type': 'command', + 'command': command, + 'timeout': 60000, + }, + ), # https://developers.openai.com/codex/hooks - 'codex': 'hooks.json', + 'codex': _HookFormat( + filename='hooks.json', + event='SessionStart', + matcher='startup', + handler=lambda command: { + 'type': 'command', + 'command': command, + 'timeout': 60, + }, + ), # https://cursor.com/docs/agent/hooks - 'cursor': 'hooks.json', - # https://kiro.dev/docs/hooks/ - 'kiro': os.path.join('hooks', 'aws-agent-toolkit.json'), + 'cursor': _HookFormat( + filename='hooks.json', + event='sessionStart', + matcher=None, + handler=lambda command: {'command': command, 'timeout': 60}, + version=1, + ), } -HOOK_AGENT_IDS = tuple(_HOOK_FILENAMES) +# https://kiro.dev/docs/hooks/ +_KIRO_ID = 'kiro' +_KIRO_FILENAME = os.path.join('hooks', f'{HOOK_NAME}.json') + +# kiro-cli 2.x ignores the standalone hook file and only runs hooks from the +# active agent's config, so the hook is also added to the default agent. +# https://kiro.dev/docs/cli/2x-reference/ +_KIRO_AGENTS_DIR = 'agents' +_KIRO_CLI_SETTINGS = os.path.join('settings', 'cli.json') +_KIRO_DEFAULT_AGENT_KEY = 'chat.defaultAgent' +_KIRO_AGENT_FORMAT = _HookFormat( + filename='', + event='agentSpawn', + matcher=None, + handler=lambda command: {'command': command, 'timeout_ms': 60000}, +) + +HOOK_AGENT_IDS = ('claude-code', 'gemini-cli', 'codex', 'cursor', _KIRO_ID) + + +class _UnreadableError(Exception): + pass def is_hook_supported(agent_id): return agent_id in HOOK_AGENT_IDS +def hook_command(agent_id): + # Agents run hooks without a terminal, where auto-prompt turned on in + # the user's config would fail before the command runs. + return ( + f'aws agent-toolkit update-skill --hook --agent {agent_id} ' + '--no-cli-auto-prompt' + ) + + +def hook_shell_command(agent_id): + """Return the hook command wrapped so the hook always exits zero. + + ``aws`` exits non-zero with ``--hook`` when a skill could not be updated, + and it can also be missing, fail to start, or crash. The agent would + report any of these as a failed hook. The wrapper depends on the shell + the agent runs hooks with. + """ + return hook_command(agent_id) + _exit_zero_suffix(agent_id) + + +def _exit_zero_suffix(agent_id): + if not is_windows: + return ' || true' + if agent_id == _KIRO_ID: + # Kiro runs hooks with cmd.exe, where ``verify`` resets the error + # level. + return ' || verify > nul' + # The others run hooks with PowerShell, except Claude Code which uses + # Git Bash when it is installed. This works in both. + return '; exit 0' + + def hook_config_path(agent_config, base_dir=None): """Return the file the agent's hook lives in, or ``None`` if unsupported. @@ -55,7 +196,171 @@ def hook_config_path(agent_config, base_dir=None): os.sep ) ) - return os.path.join(base_dir, _HOOK_FILENAMES[agent_config.id]) + if agent_config.id == _KIRO_ID: + return os.path.join(base_dir, _KIRO_FILENAME) + return os.path.join(base_dir, _HOOK_FORMATS[agent_config.id].filename) + + +def install_hook(agent): + """Install the session start hook for a detected agent.""" + agent_id = agent.config.id + path = hook_config_path(agent.config, agent.base_dir) + if path is None: + return HookAction.NOT_SUPPORTED, None + if agent_id == _KIRO_ID: + return _install_kiro_hook(agent, path), path + hook_format = _HOOK_FORMATS[agent_id] + try: + text, data = _read_json(path) + action = _add_entry(data, hook_format, _handler(agent_id)) + except _UnreadableError: + return HookAction.UNREADABLE, path + if action is not HookAction.INSTALLED: + return action, path + if text is None: + _make_parent_dir(agent, path) + _write(path, _dump_json(data)) + return HookAction.INSTALLED, path + + +def remove_hook(agent): + """Remove the session start hook from a detected agent.""" + agent_id = agent.config.id + path = hook_config_path(agent.config, agent.base_dir) + if path is None: + return HookAction.NOT_SUPPORTED, None + if agent_id == _KIRO_ID: + return _remove_kiro_hook(path), path + try: + text, data = _read_json(path) + removed = _remove_entries(data, _HOOK_FORMATS[agent_id]) + except _UnreadableError: + # Most files we can't edit never had our hook, such as a Gemini + # settings file with comments, so only those that do are reported. + if _mentions_hook(path): + return HookAction.UNREADABLE, path + return HookAction.NOT_INSTALLED, path + if not removed: + return HookAction.NOT_INSTALLED, path + _write(path, _dump_json(data)) + return HookAction.REMOVED, path + + +def kiro_default_agent_path(agent): + """Return the config file of Kiro's default agent, or ``None``.""" + name = _kiro_default_agent_name(agent.base_dir) + if name is None: + return None + paths = _kiro_agent_paths(agent) + # Agents are found by the name in the file, which tools that install + # agents often make different from the file name. + for path in paths: + if _agent_name(path) == name: + return path + for path in paths: + if os.path.splitext(os.path.basename(path))[0] == name: + return path + return None + + +def install_kiro_agent_hook(agent): + """Add the hook to Kiro's default agent, for kiro-cli 2.x.""" + path = kiro_default_agent_path(agent) + if path is None: + return HookAction.NOT_SUPPORTED, None + try: + text, data = _read_json(path) + if isinstance(data.get('hooks'), list): + return HookAction.NOT_NEEDED, path + action = _add_entry(data, _KIRO_AGENT_FORMAT, _kiro_agent_handler()) + except _UnreadableError: + return HookAction.UNREADABLE, path + if text is None or not text.strip(): + # Removed since it was found, or empty. Adding only our hook would + # not make it an agent. + return HookAction.NOT_SUPPORTED, None + if action is not HookAction.INSTALLED: + return action, path + _write(path, _dump_json(data)) + return HookAction.INSTALLED, path + + +def remove_kiro_agent_hooks(agent): + """Remove the hook from every Kiro agent config that has it.""" + results = [] + for path in _kiro_agent_paths(agent): + if not _mentions_hook(path): + continue + try: + _, data = _read_json(path) + if isinstance(data.get('hooks'), list): + # We never add the hook to V3 agents. + continue + removed = _remove_entries(data, _KIRO_AGENT_FORMAT) + except _UnreadableError: + results.append((HookAction.UNREADABLE, path)) + continue + if removed: + _write(path, _dump_json(data)) + results.append((HookAction.REMOVED, path)) + return results + + +def _kiro_default_agent_name(base_dir): + try: + with open( + os.path.join(base_dir, _KIRO_CLI_SETTINGS), encoding='utf-8' + ) as f: + settings = json.load(f) + except (OSError, ValueError): + return None + if not isinstance(settings, dict): + return None + name = settings.get(_KIRO_DEFAULT_AGENT_KEY) + return name if isinstance(name, str) and name else None + + +def _kiro_agent_paths(agent): + return sorted( + glob.glob(os.path.join(agent.base_dir, _KIRO_AGENTS_DIR, '*.json')) + ) + + +def _agent_name(path): + try: + with open(path, encoding='utf-8') as f: + data = json.load(f) + except (OSError, ValueError): + return None + return data.get('name') if isinstance(data, dict) else None + + +def format_manual_entry(agent_id, path, indent=' '): + """Return instructions for adding the hook to ``path`` by hand.""" + return _format_manual_entry( + _HOOK_FORMATS[agent_id], _handler(agent_id), path, indent + ) + + +def format_kiro_agent_manual_entry(path, indent=' '): + """Return instructions for adding the hook to a Kiro agent config.""" + return _format_manual_entry( + _KIRO_AGENT_FORMAT, _kiro_agent_handler(), path, indent + ) + + +def _format_manual_entry(hook_format, handler, path, indent): + if hook_format.matcher is None: + snippet = {'hooks': {hook_format.event: [handler]}} + else: + snippet = { + 'hooks': {hook_format.event: [_group(hook_format, handler)]} + } + if hook_format.version is not None: + snippet = {'version': hook_format.version, **snippet} + lines = [f'Add this hook to {path}:'] + lines.extend(json.dumps(snippet, indent=2).splitlines()) + return ''.join(f'{indent}{line}\n' for line in lines) def format_hook_message( @@ -91,11 +396,7 @@ def format_hook_message( def format_hook_output(agent_id, message, skills_updated=False): - """Return what the hook prints for ``agent_id``, or ``''`` if nothing. - - :param skills_updated: Whether any skill changed on disk, so the agent - should load them again. - """ + """Return what the hook prints for ``agent_id``, or ``''`` if nothing.""" if not message: return '' if agent_id == 'claude-code': @@ -124,3 +425,203 @@ def format_hook_output(agent_id, message, skills_updated=False): # Codex and Kiro add plain text output to the agent's context. return f'{message}\n' return json.dumps(output, separators=(',', ':')) + '\n' + + +def _handler(agent_id): + return _HOOK_FORMATS[agent_id].handler(hook_shell_command(agent_id)) + + +def _kiro_agent_handler(): + return _KIRO_AGENT_FORMAT.handler(hook_shell_command(_KIRO_ID)) + + +def _group(hook_format, handler): + return {'matcher': hook_format.matcher, 'hooks': [handler]} + + +def _is_ours(handler): + if not isinstance(handler, dict): + return False + command = handler.get('command') + return isinstance(command, str) and _has_markers(command) + + +def _has_markers(text): + return all(marker in text for marker in _COMMAND_MARKERS) + + +def _has_handlers(group): + return isinstance(group, dict) and isinstance(group.get('hooks'), list) + + +def _entries(data, event): + """Return the ``hooks`` object and its ``event`` list, or ``None`` for + each one that is missing.""" + hooks = data.get('hooks') + if hooks is None: + return None, None + if not isinstance(hooks, dict): + raise _UnreadableError('"hooks" is not an object') + entries = hooks.get(event) + if entries is not None and not isinstance(entries, list): + raise _UnreadableError(f'"hooks.{event}" is not a list') + return hooks, entries + + +def _handlers(entries, hook_format): + if hook_format.matcher is None: + return entries + return [h for g in entries if _has_handlers(g) for h in g['hooks']] + + +def _add_entry(data, hook_format, handler): + """Add our handler to ``data`` unless one of ours is there.""" + _, entries = _entries(data, hook_format.event) + installed = any(_is_ours(h) for h in _handlers(entries or [], hook_format)) + needs_version = hook_format.version is not None and 'version' not in data + if installed and not needs_version: + return HookAction.ALREADY_INSTALLED + if needs_version: + data['version'] = hook_format.version + if not installed: + hooks, entries = _entries(data, hook_format.event) + if hooks is None: + hooks = data['hooks'] = {} + if entries is None: + entries = hooks[hook_format.event] = [] + if hook_format.matcher is None: + entries.append(handler) + else: + entries.append(_group(hook_format, handler)) + return HookAction.INSTALLED + + +def _remove_entries(data, hook_format): + """Remove every handler of ours, and groups left empty. Returns whether + any were removed.""" + hooks, entries = _entries(data, hook_format.event) + if not entries: + return False + removed = False + kept = [] + for entry in entries: + if hook_format.matcher is None: + if _is_ours(entry): + removed = True + else: + kept.append(entry) + continue + if not _has_handlers(entry): + kept.append(entry) + continue + others = [h for h in entry['hooks'] if not _is_ours(h)] + if len(others) == len(entry['hooks']): + kept.append(entry) + continue + removed = True + if others: + entry['hooks'] = others + kept.append(entry) + if not removed: + return False + if kept: + hooks[hook_format.event] = kept + else: + del hooks[hook_format.event] + if not hooks: + del data['hooks'] + return True + + +def _mentions_hook(path): + with open(path, encoding='utf-8', errors='replace') as f: + return _has_markers(f.read()) + + +def _read_json(path): + """Return ``(text, data)`` for the JSON file at ``path``.""" + try: + with open(path, encoding='utf-8') as f: + text = f.read() + except FileNotFoundError: + return None, {} + except UnicodeDecodeError as e: + raise _UnreadableError(str(e)) from e + if not text.strip(): + return text, {} + try: + data = json.loads(text) + except ValueError as e: + raise _UnreadableError(str(e)) from e + if not isinstance(data, dict): + raise _UnreadableError('not a JSON object') + return text, data + + +def _dump_json(data): + # Written like the MCP server config. + return json.dumps(data, indent=2) + '\n' + + +def _kiro_hook_content(): + return _dump_json( + { + 'version': 'v1', + 'hooks': [ + { + 'name': HOOK_NAME, + 'description': ( + 'Keeps AWS agent skills up to date ' + '(managed by aws agent-toolkit)' + ), + 'trigger': 'SessionStart', + 'action': { + 'type': 'command', + 'command': hook_shell_command(_KIRO_ID), + }, + 'timeout': 60, + } + ], + } + ) + + +def _install_kiro_hook(agent, path): + # The file is named for us, so it is ours. It is left alone while it + # runs our command. + try: + with open(path, encoding='utf-8', errors='replace', newline='') as f: + if _has_markers(f.read()): + return HookAction.ALREADY_INSTALLED + except FileNotFoundError: + _make_parent_dir(agent, path) + _write(path, _kiro_hook_content()) + return HookAction.INSTALLED + + +def _remove_kiro_hook(path): + try: + os.remove(path) + except FileNotFoundError: + return HookAction.NOT_INSTALLED + return HookAction.REMOVED + + +def _make_parent_dir(agent, path): + # Directories are only created inside the agent's own directory, never + # in place of it. + root = agent.base_dir + if not os.path.isdir(root): + raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), root) + os.makedirs(os.path.dirname(path), exist_ok=True) + + +def _write(path, content): + # Write through symlinks, such as a settings file kept in a dotfiles + # repository, rather than replacing the link. + path = os.path.realpath(path) + try: + mode = stat.S_IMODE(os.stat(path).st_mode) + except FileNotFoundError: + mode = None + write_file_atomic(path, content, prefix=_TEMP_PREFIX, mode=mode) diff --git a/awscli/customizations/agenttoolkit/update_skill.py b/awscli/customizations/agenttoolkit/update_skill.py index 6eb599d90e64..a2220e775474 100644 --- a/awscli/customizations/agenttoolkit/update_skill.py +++ b/awscli/customizations/agenttoolkit/update_skill.py @@ -140,9 +140,10 @@ def summary(self): connect_timeout=5, read_timeout=10, retries={'max_attempts': 1} ) -# Several sessions can start at once, and an agent may run more than one -# hook at session start. Hook runs take turns, so a run that starts right -# after another finds its skills roots already checked and does nothing. +# Several sessions can start at once, and Kiro V3 runs both the standalone +# hook and the default agent's agentSpawn hook at session start. Hook runs +# take turns, so a run that starts right after another finds its skills +# roots already checked and does nothing. HOOK_LOCK_FILENAME = 'hook.lock' HOOK_LOCK_TIMEOUT = 30 @@ -343,6 +344,7 @@ def _run_hook_locked(self, agent_id, parsed_globals): ) if not agents: return 0 + self._record_check_started(due_roots) if self._collect_updates( agents, parsed_globals, None, results, HOOK_CLIENT_CONFIG ): @@ -378,6 +380,20 @@ def _run_hook_locked(self, agent_id, parsed_globals): return 1 return 0 + def _record_check_started(self, roots): + # Agents kill a hook that runs past their timeout, before it records + # anything. Recording the check as failed up front means it's tried + # again after HOOK_RETRY_INTERVAL rather than in every new session. + for root in roots: + try: + record_hook_run( + root, + is_successful=False, + summary='The check did not finish', + ) + except StateFileError as e: + LOG.debug('Could not record the hook run: %s', e) + def _record_hook_runs(self, roots, results, error, summary): timestamp = datetime.datetime.now(datetime.timezone.utc) for root in roots: diff --git a/tests/unit/customizations/agenttoolkit/test_atomic.py b/tests/unit/customizations/agenttoolkit/test_atomic.py index f35c48e00861..748fa201e397 100644 --- a/tests/unit/customizations/agenttoolkit/test_atomic.py +++ b/tests/unit/customizations/agenttoolkit/test_atomic.py @@ -346,3 +346,23 @@ def test_recover_finishes_interrupted_remove(live): atomic.recover(str(live)) assert not trash.exists() assert not live.exists() + + +def test_write_file_atomic_new_file_is_owner_only(tmp_path): + path = tmp_path / 'f.json' + atomic.write_file_atomic(str(path), 'new', prefix='.f-') + if os.name != 'nt': + assert path.stat().st_mode & 0o777 == 0o600 + + +def test_write_file_atomic_sets_mode(tmp_path): + path = tmp_path / 'f.json' + atomic.write_file_atomic(str(path), 'new', prefix='.f-', mode=0o644) + if os.name != 'nt': + assert path.stat().st_mode & 0o777 == 0o644 + + +def test_write_file_atomic_keeps_newlines(tmp_path): + path = tmp_path / 'f.json' + atomic.write_file_atomic(str(path), 'a\r\nb\n', prefix='.f-') + assert path.read_bytes() == b'a\r\nb\n' diff --git a/tests/unit/customizations/agenttoolkit/test_configure.py b/tests/unit/customizations/agenttoolkit/test_configure.py index f3964b738b5f..19bec8f90562 100644 --- a/tests/unit/customizations/agenttoolkit/test_configure.py +++ b/tests/unit/customizations/agenttoolkit/test_configure.py @@ -11,18 +11,32 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. import json +import os from io import StringIO from unittest.mock import MagicMock, patch import pytest from awscli.customizations.agenttoolkit import state -from awscli.customizations.agenttoolkit.agents import AgentConfig +from awscli.customizations.agenttoolkit.agents import ( + AgentConfig, + collapse_home, +) from awscli.customizations.agenttoolkit.configure import ( AUTO_UPDATE_KEEP_PROMPT, AUTO_UPDATE_PROMPT, ConfigureAgentToolkitCommand, ) +from awscli.customizations.agenttoolkit.hooks import ( + CODEX_TRUST_NOTE, + KIRO_AGENT_NOTE, + KIRO_NO_AGENT_NOTE, + KIRO_VERSION_NOTE, + format_kiro_agent_manual_entry, + format_manual_entry, + hook_command, + hook_shell_command, +) from awscli.customizations.exceptions import ConfigurationError from tests.unit.customizations.agenttoolkit.utils import ( make_config, @@ -453,3 +467,298 @@ def test_auto_update_not_prompted_when_no_agents_selected( cmd._run_main(parsed_args, None) assert not yes_no.called assert not state_file.exists() + + +def _hook_agent_configs(tmp_path, agent_ids, detected=True): + configs = [] + for agent_id in agent_ids: + agent_dir = tmp_path / f'.{agent_id}' + if detected: + agent_dir.mkdir() + configs.append( + make_config( + tmp_path, + id=agent_id, + display_name=agent_id.title(), + detection_path=str(agent_dir), + ) + ) + return configs + + +def _hook_file(tmp_path, agent_id): + return ( + tmp_path + / f'.{agent_id}' + / { + 'claude-code': 'settings.json', + 'codex': 'hooks.json', + 'cursor': 'hooks.json', + }.get(agent_id, os.path.join('hooks', 'aws-agent-toolkit.json')) + ) + + +def _collapse(path): + return collapse_home(str(path)) + + +def _run_selecting(configs, selected_ids, yes_no_return=True): + stream = StringIO() + cmd = ConfigureAgentToolkitCommand( + make_session(), + stream=stream, + agent_configs=configs, + client=_make_client(), + ) + parsed_args = MagicMock() + parsed_args.yes = False + with ( + patch( + 'awscli.customizations.agenttoolkit.configure.multiselect_choice', + side_effect=lambda msg, items, **kw: [ + a for a in items if a.config.id in selected_ids + ], + ), + patch( + 'awscli.customizations.agenttoolkit.configure.yes_no_choice', + return_value=yes_no_return, + ), + ): + cmd._run_main(parsed_args, None) + return stream.getvalue() + + +def test_auto_update_installs_hooks_for_selected_agents(tmp_path): + configs = _hook_agent_configs( + tmp_path, ['claude-code', 'codex', 'kiro', 'cline'] + ) + _, stream = _run(configs, yes_no_return=True) + output = stream.getvalue() + for agent_id in ('claude-code', 'codex', 'kiro'): + path = _hook_file(tmp_path, agent_id) + assert hook_command(agent_id) in path.read_text() + assert ( + f' ✓ {agent_id.title()} — {_collapse(path)}: hook installed\n' + ) in output + assert ' ✗ Cline (automatic updates not supported yet)\n' in output + assert f' {CODEX_TRUST_NOTE}\n' in output + assert f' {KIRO_VERSION_NOTE}\n' in output + assert os.listdir(tmp_path / '.cline') == [] + + +def test_auto_update_kept_reports_already_installed(tmp_path, state_file): + _write_state(state_file, {'agent_toolkit_auto_update': True}) + configs = _hook_agent_configs(tmp_path, ['claude-code']) + _run(configs, yes_no_return=True) + path = _hook_file(tmp_path, 'claude-code') + before = path.read_bytes() + _, stream = _run(configs, yes_no_return=True) + assert path.read_bytes() == before + assert f'{_collapse(path)}: already installed\n' in stream.getvalue() + + +def _change_claude_hook(path): + data = json.loads(path.read_text()) + data['hooks']['SessionStart'][0]['matcher'] = '*' + path.write_text(json.dumps(data, indent=2) + '\n') + return path.read_bytes() + + +def test_auto_update_keeps_our_hook_changed_by_customer(tmp_path, state_file): + _write_state(state_file, {'agent_toolkit_auto_update': True}) + configs = _hook_agent_configs(tmp_path, ['claude-code']) + _run(configs, yes_no_return=True) + path = _hook_file(tmp_path, 'claude-code') + before = _change_claude_hook(path) + _, stream = _run(configs, yes_no_return=True) + assert path.read_bytes() == before + assert f'{_collapse(path)}: already installed\n' in stream.getvalue() + + +def test_auto_update_off_removes_our_hook_changed_by_customer( + tmp_path, state_file +): + _write_state(state_file, {'agent_toolkit_auto_update': True}) + configs = _hook_agent_configs(tmp_path, ['claude-code']) + _run(configs, yes_no_return=True) + path = _hook_file(tmp_path, 'claude-code') + _change_claude_hook(path) + _, stream = _run(configs, yes_no_return=False) + assert json.loads(path.read_text()) == {} + assert ( + f' ✓ Claude-Code — {_collapse(path)}: hook removed\n' + ) in stream.getvalue() + + +def test_auto_update_yes_flag_installs_hooks(tmp_path): + configs = _hook_agent_configs(tmp_path, ['cursor']) + _run(configs, yes=True) + assert hook_command('cursor') in _hook_file(tmp_path, 'cursor').read_text() + + +def test_auto_update_installs_only_for_selected_agents(tmp_path): + configs = _hook_agent_configs(tmp_path, ['claude-code', 'cursor']) + _run_selecting(configs, {'cursor'}) + assert _hook_file(tmp_path, 'cursor').exists() + assert not _hook_file(tmp_path, 'claude-code').exists() + + +def test_auto_update_unreadable_file_prints_manual_entry(tmp_path): + configs = _hook_agent_configs(tmp_path, ['claude-code']) + path = _hook_file(tmp_path, 'claude-code') + path.write_text('{\n // my settings\n}\n') + _, stream = _run(configs, yes_no_return=True) + output = stream.getvalue() + assert path.read_text() == '{\n // my settings\n}\n' + assert f' ✗ Claude-Code — {_collapse(path)}: could not' in (output) + assert format_manual_entry('claude-code', _collapse(path)) in output + + +def test_auto_update_hook_error_is_reported(tmp_path): + configs = _hook_agent_configs(tmp_path, ['claude-code', 'codex']) + with patch( + 'awscli.customizations.agenttoolkit.hooks.write_file_atomic', + side_effect=[OSError('disk full'), None], + ): + rc, stream = _run(configs, yes_no_return=True) + assert rc == 0 + path = _hook_file(tmp_path, 'claude-code') + assert ( + f' ✗ Claude-Code — {_collapse(path)}: could not install ' + 'the hook (disk full)\n' + ) in stream.getvalue() + assert 'Codex —' in stream.getvalue() + + +def test_auto_update_off_removes_hooks_only_from_selected_agents( + tmp_path, state_file +): + _write_state(state_file, {'agent_toolkit_auto_update': True}) + configs = _hook_agent_configs(tmp_path, ['claude-code', 'kiro', 'cline']) + _run(configs, yes_no_return=True) + settings = _hook_file(tmp_path, 'claude-code') + kiro_hook = _hook_file(tmp_path, 'kiro') + kiro_before = kiro_hook.read_bytes() + + output = _run_selecting( + configs, {'claude-code', 'cline'}, yes_no_return=False + ) + + assert not state.is_auto_update_enabled() + assert json.loads(settings.read_text()) == {} + assert kiro_hook.read_bytes() == kiro_before + assert ( + f' ✓ Claude-Code — {_collapse(settings)}: hook removed\n' + ) in output + after = output.split('Automatic skill updates turned off.')[1] + assert 'Kiro —' not in after + assert 'Cline' not in after + + +def test_auto_update_declined_prints_nothing_without_hooks(tmp_path): + configs = _hook_agent_configs(tmp_path, ['claude-code']) + settings = _hook_file(tmp_path, 'claude-code') + settings.write_text('{\n // comments\n}\n') + _, stream = _run(configs, yes_no_return=False) + assert 'hook' not in stream.getvalue() + assert settings.read_text() == '{\n // comments\n}\n' + + +def test_auto_update_state_error_changes_no_hooks(tmp_path, state_file): + configs = _hook_agent_configs(tmp_path, ['claude-code']) + with patch( + 'awscli.customizations.agenttoolkit.configure.set_auto_update', + side_effect=state.StateFileError('/x/state.json: disk full'), + ): + _, stream = _run(configs, yes_no_return=True) + assert not _hook_file(tmp_path, 'claude-code').exists() + assert 'hook' not in stream.getvalue() + + +def test_auto_update_state_error_keeps_hooks_when_disabling( + tmp_path, state_file +): + _write_state(state_file, {'agent_toolkit_auto_update': True}) + configs = _hook_agent_configs(tmp_path, ['claude-code']) + _run(configs, yes_no_return=True) + with patch( + 'awscli.customizations.agenttoolkit.configure.set_auto_update', + side_effect=state.StateFileError('/x/state.json: disk full'), + ): + _run(configs, yes_no_return=False) + assert hook_command('claude-code') in ( + _hook_file(tmp_path, 'claude-code').read_text() + ) + + +def _kiro_default_agent(tmp_path, content=None): + settings = tmp_path / '.kiro' / 'settings' / 'cli.json' + settings.parent.mkdir(parents=True) + settings.write_text(json.dumps({'chat.defaultAgent': 'my-agent'})) + path = tmp_path / '.kiro' / 'agents' / 'my-agent.json' + path.parent.mkdir() + if content is None: + content = json.dumps({'name': 'my-agent'}, indent=2) + '\n' + path.write_text(content) + return path + + +def test_auto_update_adds_hook_to_kiro_default_agent(tmp_path): + configs = _hook_agent_configs(tmp_path, ['kiro']) + agent_file = _kiro_default_agent(tmp_path) + _, stream = _run(configs, yes_no_return=True) + output = stream.getvalue() + assert json.loads(agent_file.read_text())['hooks'] == { + 'agentSpawn': [ + {'command': hook_shell_command('kiro'), 'timeout_ms': 60000} + ] + } + assert ( + f' ✓ Kiro — {_collapse(_hook_file(tmp_path, "kiro"))}: ' + 'hook installed\n' + f' {KIRO_VERSION_NOTE}\n' + f' ✓ Kiro — {_collapse(agent_file)}: hook installed\n' + f' {KIRO_AGENT_NOTE}\n' + ) in output + + +def test_auto_update_kiro_without_default_agent_prints_manual_entry( + tmp_path, +): + configs = _hook_agent_configs(tmp_path, ['kiro']) + _, stream = _run(configs, yes_no_return=True) + output = stream.getvalue() + assert f' ✗ Kiro — {KIRO_NO_AGENT_NOTE}\n' in output + assert ( + format_kiro_agent_manual_entry( + _collapse(tmp_path / '.kiro' / 'agents' / '.json') + ) + in output + ) + assert not (tmp_path / '.kiro' / 'agents').exists() + + +def test_auto_update_kiro_agent_with_comments_prints_manual_entry( + tmp_path, +): + configs = _hook_agent_configs(tmp_path, ['kiro']) + content = '{\n // mine\n "name": "my-agent"\n}\n' + agent_file = _kiro_default_agent(tmp_path, content) + _, stream = _run(configs, yes_no_return=True) + output = stream.getvalue() + assert agent_file.read_text() == content + assert f' ✗ Kiro — {_collapse(agent_file)}: could not' in output + assert format_kiro_agent_manual_entry(_collapse(agent_file)) in output + + +def test_auto_update_off_removes_hook_from_kiro_agents(tmp_path, state_file): + _write_state(state_file, {'agent_toolkit_auto_update': True}) + configs = _hook_agent_configs(tmp_path, ['kiro']) + agent_file = _kiro_default_agent(tmp_path) + before = agent_file.read_text() + _run(configs, yes_no_return=True) + _, stream = _run(configs, yes_no_return=False) + assert agent_file.read_text() == before + assert f' ✓ Kiro — {_collapse(agent_file)}: hook removed\n' in ( + stream.getvalue() + ) diff --git a/tests/unit/customizations/agenttoolkit/test_hooks.py b/tests/unit/customizations/agenttoolkit/test_hooks.py index ea37b3845eb6..913458dfa259 100644 --- a/tests/unit/customizations/agenttoolkit/test_hooks.py +++ b/tests/unit/customizations/agenttoolkit/test_hooks.py @@ -12,17 +12,30 @@ # language governing permissions and limitations under the License. import json import os +from unittest.mock import patch import pytest from awscli.customizations.agenttoolkit import hooks from awscli.customizations.agenttoolkit.hooks import ( + HookAction, format_hook_message, format_hook_output, + format_kiro_agent_manual_entry, + format_manual_entry, + hook_command, hook_config_path, - is_hook_supported, + hook_shell_command, + install_hook, + install_kiro_agent_hook, + kiro_default_agent_path, + remove_hook, + remove_kiro_agent_hooks, +) +from tests.unit.customizations.agenttoolkit.utils import ( + make_config, + requires_symlinks, ) -from tests.unit.customizations.agenttoolkit.utils import make_config _ENV_VARS = ('CLAUDE_CONFIG_DIR', 'CODEX_HOME', 'GEMINI_CLI_HOME') @@ -31,7 +44,12 @@ 'gemini-cli': 'settings.json', 'codex': 'hooks.json', 'cursor': 'hooks.json', - 'kiro': os.path.join('hooks', 'aws-agent-toolkit.json'), +} + +_GROUP_EVENTS = { + 'claude-code': 'SessionStart', + 'gemini-cli': 'SessionStart', + 'codex': 'SessionStart', } @@ -55,41 +73,477 @@ def _agent(tmp_path, agent_id, create=True, **overrides): return config.detect() if create else config +def _expected_entry(agent_id): + command = hook_shell_command(agent_id) + if agent_id == 'claude-code': + return { + 'hooks': { + 'SessionStart': [ + { + 'matcher': 'startup', + 'hooks': [ + { + 'type': 'command', + 'command': command, + 'timeout': 60, + } + ], + } + ] + } + } + if agent_id == 'gemini-cli': + return { + 'hooks': { + 'SessionStart': [ + { + 'matcher': '*', + 'hooks': [ + { + 'name': 'aws-agent-toolkit', + 'type': 'command', + 'command': command, + 'timeout': 60000, + } + ], + } + ] + } + } + if agent_id == 'codex': + return { + 'hooks': { + 'SessionStart': [ + { + 'matcher': 'startup', + 'hooks': [ + { + 'type': 'command', + 'command': command, + 'timeout': 60, + } + ], + } + ] + } + } + return { + 'version': 1, + 'hooks': {'sessionStart': [{'command': command, 'timeout': 60}]}, + } + + +def _our_group(agent_id): + return _expected_entry(agent_id)['hooks'][_GROUP_EVENTS[agent_id]][0] + + +@pytest.mark.parametrize('agent_id', sorted(_HOOK_FILES)) +def test_install_creates_exact_entry(tmp_path, agent_id): + agent = _agent(tmp_path, agent_id) + action, path = install_hook(agent) + expected_path = tmp_path / f'.{agent_id}' / _HOOK_FILES[agent_id] + assert (action, path) == (HookAction.INSTALLED, str(expected_path)) + text = expected_path.read_text() + assert json.loads(text) == _expected_entry(agent_id) + # New files use two spaces, LF and a trailing newline. + assert text == json.dumps(_expected_entry(agent_id), indent=2) + '\n' + if os.name != 'nt': + assert expected_path.stat().st_mode & 0o777 == 0o600 + + +def test_cursor_new_file_puts_version_first(tmp_path): + agent = _agent(tmp_path, 'cursor') + install_hook(agent) + text = (tmp_path / '.cursor' / 'hooks.json').read_text() + assert list(json.loads(text)) == ['version', 'hooks'] + + +@pytest.mark.parametrize('agent_id', sorted(_HOOK_FILES) + ['kiro']) +def test_install_twice_is_already_installed(tmp_path, agent_id): + agent = _agent(tmp_path, agent_id) + install_hook(agent) + path = hook_config_path(agent.config, agent.base_dir) + before = open(path, 'rb').read() + mtime = os.stat(path).st_mtime_ns + action, _ = install_hook(agent) + assert action is HookAction.ALREADY_INSTALLED + assert open(path, 'rb').read() == before + assert os.stat(path).st_mtime_ns == mtime + + +def _existing_settings(agent_id, indent, newline): + if agent_id == 'cursor': + data = { + 'version': 1, + 'hooks': { + 'sessionStart': [{'command': 'echo mine'}], + 'stop': [{'command': 'echo stop'}], + }, + } + else: + data = { + 'model': 'some-model', + 'permissions': {'allow': ['Bash(ls)']}, + 'hooks': { + 'SessionStart': [ + { + 'matcher': 'startup', + 'hooks': [{'type': 'command', 'command': 'echo hi'}], + } + ], + 'Stop': [ + {'hooks': [{'type': 'command', 'command': 'echo bye'}]} + ], + }, + } + return (json.dumps(data, indent=indent) + '\n').replace('\n', newline) + + @pytest.mark.parametrize('agent_id', sorted(_HOOK_FILES)) -def test_hook_config_path_for_detected_agent(tmp_path, agent_id): +@pytest.mark.parametrize( + 'indent,newline', + [(2, '\n'), (4, '\n'), ('\t', '\n'), (2, '\r\n')], + ids=['2-spaces', '4-spaces', 'tab', 'crlf'], +) +def test_install_then_remove_keeps_settings( + tmp_path, agent_id, indent, newline +): agent = _agent(tmp_path, agent_id) - assert hook_config_path(agent.config, agent.base_dir) == str( - tmp_path / f'.{agent_id}' / _HOOK_FILES[agent_id] + path = tmp_path / f'.{agent_id}' / _HOOK_FILES[agent_id] + original = _existing_settings(agent_id, indent, newline) + path.write_bytes(original.encode()) + + assert install_hook(agent)[0] is HookAction.INSTALLED + installed = path.read_bytes() + text = installed.decode() + # Files are written like the MCP server config. + assert text == json.dumps(json.loads(text), indent=2) + '\n' + data = json.loads(text) + if agent_id == 'cursor': + assert data['hooks']['sessionStart'] == [ + {'command': 'echo mine'}, + _expected_entry('cursor')['hooks']['sessionStart'][0], + ] + assert data['hooks']['stop'] == [{'command': 'echo stop'}] + else: + assert data['model'] == 'some-model' + assert data['hooks']['SessionStart'] == [ + { + 'matcher': 'startup', + 'hooks': [{'type': 'command', 'command': 'echo hi'}], + }, + _our_group(agent_id), + ] + assert data['hooks']['Stop'] == [ + {'hooks': [{'type': 'command', 'command': 'echo bye'}]} + ] + + assert install_hook(agent)[0] is HookAction.ALREADY_INSTALLED + assert path.read_bytes() == installed + assert remove_hook(agent)[0] is HookAction.REMOVED + assert json.loads(path.read_text()) == json.loads(original) + + +@pytest.mark.parametrize('agent_id', sorted(_HOOK_FILES)) +def test_remove_drops_empty_containers(tmp_path, agent_id): + agent = _agent(tmp_path, agent_id) + path = tmp_path / f'.{agent_id}' / _HOOK_FILES[agent_id] + path.write_text('{\n "theme": "dark"\n}\n') + install_hook(agent) + assert remove_hook(agent)[0] is HookAction.REMOVED + expected = {'theme': 'dark'} + if agent_id == 'cursor': + # The version is kept since we can't tell whether we added it. + expected['version'] = 1 + assert json.loads(path.read_text()) == expected + + +def test_install_into_empty_object(tmp_path): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + path.write_text('{}') + install_hook(agent) + assert path.read_text() == ( + json.dumps(_expected_entry('claude-code'), indent=2) + '\n' ) - assert hook_config_path(agent.config) == str( - tmp_path / f'.{agent_id}' / _HOOK_FILES[agent_id] + remove_hook(agent) + assert path.read_text() == '{}\n' + + +def test_install_into_empty_file(tmp_path): + agent = _agent(tmp_path, 'codex') + path = tmp_path / '.codex' / 'hooks.json' + path.write_text('') + assert install_hook(agent)[0] is HookAction.INSTALLED + assert json.loads(path.read_text()) == _expected_entry('codex') + + +@pytest.mark.parametrize( + 'content', + [ + '{"model": "x"}\n', + '{\n "model":"x"\n}\n', + '{\n "model" : "x"\n}', + '{\n "model": "x"\n}\n\n', + ], + ids=['one-line', 'compact-separators', 'jackson', 'extra-newline'], +) +def test_install_reformats_file(tmp_path, content): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + path.write_text(content) + assert install_hook(agent)[0] is HookAction.INSTALLED + text = path.read_text() + assert text == json.dumps(json.loads(text), indent=2) + '\n' + assert json.loads(text)['model'] == 'x' + remove_hook(agent) + assert path.read_text() == '{\n "model": "x"\n}\n' + + +def test_install_keeps_unicode_escapes(tmp_path): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + original = '{\n "name": "caf\\u00e9"\n}\n' + path.write_text(original) + install_hook(agent) + assert '"caf\\u00e9"' in path.read_text() + remove_hook(agent) + assert path.read_text() == original + + +def test_install_escapes_raw_unicode(tmp_path): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + path.write_text('{\n "name": "café"\n}\n', encoding='utf-8') + install_hook(agent) + remove_hook(agent) + assert path.read_text(encoding='utf-8') == '{\n "name": "caf\\u00e9"\n}\n' + + +def _change_our_group(group, change): + if change == 'matcher': + group['matcher'] = 'startup|resume|clear' + elif change == 'timeout': + group['hooks'][0]['timeout'] = 10 + else: + group['hooks'].insert(0, {'type': 'command', 'command': 'echo mine'}) + return group + + +@pytest.mark.parametrize('agent_id', ['claude-code', 'gemini-cli', 'codex']) +@pytest.mark.parametrize('change', ['matcher', 'timeout', 'shared group']) +def test_hook_running_our_command_is_ours(tmp_path, agent_id, change): + agent = _agent(tmp_path, agent_id) + path = tmp_path / f'.{agent_id}' / _HOOK_FILES[agent_id] + other = {'hooks': [{'type': 'command', 'command': 'echo other'}]} + changed = _change_our_group(_our_group(agent_id), change) + path.write_text( + json.dumps({'hooks': {'SessionStart': [changed, other]}}, indent=2) + + '\n' + ) + before = path.read_bytes() + assert install_hook(agent)[0] is HookAction.ALREADY_INSTALLED + assert path.read_bytes() == before + assert remove_hook(agent)[0] is HookAction.REMOVED + left = json.loads(path.read_text())['hooks']['SessionStart'] + if change == 'shared group': + assert left == [{**changed, 'hooks': changed['hooks'][:1]}, other] + else: + assert left == [other] + + +def test_cursor_handler_running_our_command_is_ours(tmp_path): + agent = _agent(tmp_path, 'cursor') + path = tmp_path / '.cursor' / 'hooks.json' + changed = dict(_expected_entry('cursor')['hooks']['sessionStart'][0]) + changed['timeout'] = 10 + path.write_text( + json.dumps({'version': 1, 'hooks': {'sessionStart': [changed]}}) + ) + before = path.read_bytes() + assert install_hook(agent)[0] is HookAction.ALREADY_INSTALLED + assert path.read_bytes() == before + assert remove_hook(agent)[0] is HookAction.REMOVED + assert json.loads(path.read_text()) == {'version': 1} + + +def test_remove_keeps_handlers_running_other_commands(tmp_path): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + ours = _our_group('claude-code') + variant = _our_group('claude-code') + variant['hooks'][0]['command'] += ' --debug' + theirs = {'hooks': [{'type': 'command', 'command': 'aws s3 ls'}]} + path.write_text( + json.dumps({'hooks': {'SessionStart': [ours, variant, theirs]}}) ) + assert remove_hook(agent)[0] is HookAction.REMOVED + assert json.loads(path.read_text())['hooks']['SessionStart'] == [theirs] + + +def test_install_leaves_duplicates(tmp_path): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + ours = _our_group('claude-code') + path.write_text(json.dumps({'hooks': {'SessionStart': [ours, ours]}})) + before = path.read_bytes() + assert install_hook(agent)[0] is HookAction.ALREADY_INSTALLED + assert path.read_bytes() == before + assert remove_hook(agent)[0] is HookAction.REMOVED + assert json.loads(path.read_text()) == {} + + +def test_cursor_install_adds_missing_version(tmp_path): + agent = _agent(tmp_path, 'cursor') + path = tmp_path / '.cursor' / 'hooks.json' + ours = _expected_entry('cursor')['hooks']['sessionStart'][0] + mine = {'command': 'echo mine'} + path.write_text(json.dumps({'hooks': {'sessionStart': [mine, ours]}})) + assert install_hook(agent)[0] is HookAction.INSTALLED + assert json.loads(path.read_text()) == { + 'hooks': {'sessionStart': [mine, ours]}, + 'version': 1, + } + + +def test_cursor_keeps_existing_version(tmp_path): + agent = _agent(tmp_path, 'cursor') + path = tmp_path / '.cursor' / 'hooks.json' + path.write_text(json.dumps({'version': 2})) + install_hook(agent) + assert json.loads(path.read_text())['version'] == 2 + + +@pytest.mark.parametrize( + 'content', + [ + '{\n // comment\n "a": 1\n}\n', + '{ not json', + '[]\n', + '{\n "hooks": []\n}\n', + '{\n "hooks": {\n "SessionStart": {}\n }\n}\n', + '\ufeff{}', + ], + ids=[ + 'comments', + 'invalid', + 'root-list', + 'hooks-list', + 'event-object', + 'bom', + ], +) +def test_unreadable_file_is_left_untouched(tmp_path, content): + agent = _agent(tmp_path, 'gemini-cli') + path = tmp_path / '.gemini-cli' / 'settings.json' + path.write_bytes(content.encode('utf-8')) + assert install_hook(agent) == (HookAction.UNREADABLE, str(path)) + assert path.read_bytes() == content.encode('utf-8') + # Nothing of ours is in there, so there is nothing to remove. + assert remove_hook(agent)[0] is HookAction.NOT_INSTALLED + assert path.read_bytes() == content.encode('utf-8') + assert sorted(os.listdir(path.parent)) == ['settings.json'] + + +def test_remove_reports_unreadable_file_with_our_hook(tmp_path): + agent = _agent(tmp_path, 'gemini-cli') + path = tmp_path / '.gemini-cli' / 'settings.json' + content = ( + '{\n // added by hand\n "hooks": {"SessionStart": [{"hooks": ' + f'[{{"command": "{hook_shell_command("gemini-cli")}"}}]}}]}}\n}}\n' + ) + path.write_text(content) + assert remove_hook(agent)[0] is HookAction.UNREADABLE + assert path.read_text() == content + + +def test_install_preserves_permissions(tmp_path): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + path.write_text('{}\n') + os.chmod(path, 0o640) + mode = path.stat().st_mode + install_hook(agent) + assert path.stat().st_mode == mode + remove_hook(agent) + assert path.stat().st_mode == mode + + +@requires_symlinks +def test_install_writes_through_symlink(tmp_path): + agent = _agent(tmp_path, 'claude-code') + real = tmp_path / 'dotfiles' / 'settings.json' + real.parent.mkdir() + real.write_text('{}\n') + link = tmp_path / '.claude-code' / 'settings.json' + link.symlink_to(real) + install_hook(agent) + assert link.is_symlink() + assert json.loads(real.read_text()) == _expected_entry('claude-code') + + +def test_other_users_hooks_untouched_on_remove(tmp_path): + agent = _agent(tmp_path, 'codex') + path = tmp_path / '.codex' / 'hooks.json' + content = json.dumps( + { + 'hooks': { + 'SessionStart': [ + { + 'hooks': [ + { + 'type': 'command', + 'command': 'aws agent-toolkit update-skill', + } + ] + } + ] + } + }, + indent=2, + ) + path.write_text(content) + assert remove_hook(agent)[0] is HookAction.NOT_INSTALLED + assert path.read_text() == content + + +def test_remove_missing_file_is_not_installed(tmp_path): + agent = _agent(tmp_path, 'claude-code') + assert remove_hook(agent)[0] is HookAction.NOT_INSTALLED + assert not (tmp_path / '.claude-code' / 'settings.json').exists() def test_claude_code_honours_config_dir(tmp_path, monkeypatch): override = tmp_path / 'claude-config' override.mkdir() monkeypatch.setenv('CLAUDE_CONFIG_DIR', str(override)) - config = _agent( + agent = _agent( tmp_path, 'claude-code', create=False, detection_path_env_override='CLAUDE_CONFIG_DIR', + ).detect() + action, path = install_hook(agent) + assert (action, path) == ( + HookAction.INSTALLED, + str(override / 'settings.json'), ) - assert hook_config_path(config) == str(override / 'settings.json') def test_codex_honours_codex_home(tmp_path, monkeypatch): override = tmp_path / 'codex-home' override.mkdir() monkeypatch.setenv('CODEX_HOME', str(override)) - config = _agent( + agent = _agent( tmp_path, 'codex', create=False, detection_path_env_override='CODEX_HOME', + ).detect() + assert install_hook(agent) == ( + HookAction.INSTALLED, + str(override / 'hooks.json'), ) - assert hook_config_path(config) == str(override / 'hooks.json') def _gemini_with_home(tmp_path, monkeypatch, create=True): @@ -110,9 +564,10 @@ def _gemini_with_home(tmp_path, monkeypatch, create=True): def test_gemini_honours_gemini_cli_home(tmp_path, monkeypatch): home, config = _gemini_with_home(tmp_path, monkeypatch) agent = config.detect() - assert hook_config_path(agent.config, agent.base_dir) == str( - home / '.gemini' / 'settings.json' - ) + expected = home / '.gemini' / 'settings.json' + assert install_hook(agent) == (HookAction.INSTALLED, str(expected)) + assert json.loads(expected.read_text()) == _expected_entry('gemini-cli') + assert remove_hook(agent) == (HookAction.REMOVED, str(expected)) def test_gemini_cli_home_path_for_undetected_agent(tmp_path, monkeypatch): @@ -126,10 +581,379 @@ def test_hook_config_path_for_undetected_agent(tmp_path): assert hook_config_path(config) == str(tmp_path / '.codex' / 'hooks.json') +def test_kiro_owns_its_file(tmp_path): + agent = _agent(tmp_path, 'kiro') + path = tmp_path / '.kiro' / 'hooks' / 'aws-agent-toolkit.json' + assert install_hook(agent) == (HookAction.INSTALLED, str(path)) + assert json.loads(path.read_text()) == { + 'version': 'v1', + 'hooks': [ + { + 'name': 'aws-agent-toolkit', + 'description': ( + 'Keeps AWS agent skills up to date ' + '(managed by aws agent-toolkit)' + ), + 'trigger': 'SessionStart', + 'action': { + 'type': 'command', + 'command': hook_shell_command('kiro'), + }, + 'timeout': 60, + } + ], + } + assert remove_hook(agent) == (HookAction.REMOVED, str(path)) + assert not path.exists() + assert path.parent.is_dir() + assert remove_hook(agent)[0] is HookAction.NOT_INSTALLED + + +def test_kiro_rewrites_file_without_our_command(tmp_path): + agent = _agent(tmp_path, 'kiro') + path = tmp_path / '.kiro' / 'hooks' / 'aws-agent-toolkit.json' + path.parent.mkdir() + path.write_text('old') + assert install_hook(agent)[0] is HookAction.INSTALLED + assert json.loads(path.read_text())['version'] == 'v1' + + +def test_kiro_leaves_file_running_our_command(tmp_path): + agent = _agent(tmp_path, 'kiro') + install_hook(agent) + path = tmp_path / '.kiro' / 'hooks' / 'aws-agent-toolkit.json' + data = json.loads(path.read_text()) + data['hooks'][0]['timeout'] = 10 + path.write_text(json.dumps(data)) + before = path.read_bytes() + assert install_hook(agent)[0] is HookAction.ALREADY_INSTALLED + assert path.read_bytes() == before + assert remove_hook(agent)[0] is HookAction.REMOVED + assert not path.exists() + + +def test_kiro_leaves_other_hooks_alone(tmp_path): + agent = _agent(tmp_path, 'kiro') + other = tmp_path / '.kiro' / 'hooks' / 'mine.json' + other.parent.mkdir() + other.write_text('{}') + install_hook(agent) + remove_hook(agent) + assert os.listdir(other.parent) == ['mine.json'] + + +def _kiro_with_agents(tmp_path, agents, default='my-agent'): + """Return a detected Kiro with ``agents`` as ``{file name: content}``.""" + agent = _agent(tmp_path, 'kiro') + if default is not None: + settings = tmp_path / '.kiro' / 'settings' / 'cli.json' + settings.parent.mkdir() + settings.write_text(json.dumps({'chat.defaultAgent': default})) + agents_dir = tmp_path / '.kiro' / 'agents' + agents_dir.mkdir() + for name, content in agents.items(): + if not isinstance(content, str): + content = json.dumps(content, indent=2) + '\n' + (agents_dir / name).write_text(content) + return agent + + +def _kiro_agent_handler(): + return {'command': hook_shell_command('kiro'), 'timeout_ms': 60000} + + +def test_kiro_default_agent_found_by_name(tmp_path): + agent = _kiro_with_agents( + tmp_path, + { + 'my-agent.json': {'name': 'other'}, + 'Tool-my-agent.json': {'name': 'my-agent'}, + }, + ) + assert kiro_default_agent_path(agent) == str( + tmp_path / '.kiro' / 'agents' / 'Tool-my-agent.json' + ) + + +def test_kiro_default_agent_found_by_file_name(tmp_path): + agent = _kiro_with_agents(tmp_path, {'my-agent.json': {'tools': []}}) + assert kiro_default_agent_path(agent) == str( + tmp_path / '.kiro' / 'agents' / 'my-agent.json' + ) + + +@pytest.mark.parametrize( + 'settings', [None, '{}', '{"chat.defaultAgent": ""}', 'not json', '[]'] +) +def test_kiro_without_default_agent_is_not_supported(tmp_path, settings): + agent = _kiro_with_agents( + tmp_path, {'my-agent.json': {'name': 'my-agent'}}, default=None + ) + if settings is not None: + path = tmp_path / '.kiro' / 'settings' / 'cli.json' + path.parent.mkdir() + path.write_text(settings) + assert kiro_default_agent_path(agent) is None + assert install_kiro_agent_hook(agent) == (HookAction.NOT_SUPPORTED, None) + assert json.loads( + (tmp_path / '.kiro' / 'agents' / 'my-agent.json').read_text() + ) == {'name': 'my-agent'} + + +def test_kiro_missing_default_agent_file_is_not_supported(tmp_path): + agent = _kiro_with_agents(tmp_path, {'other.json': {'name': 'other'}}) + assert install_kiro_agent_hook(agent) == (HookAction.NOT_SUPPORTED, None) + assert os.listdir(tmp_path / '.kiro' / 'agents') == ['other.json'] + + +@pytest.mark.parametrize('content', ['', ' \n']) +def test_kiro_empty_default_agent_file_is_not_supported(tmp_path, content): + agent = _kiro_with_agents(tmp_path, {'my-agent.json': content}) + path = tmp_path / '.kiro' / 'agents' / 'my-agent.json' + assert install_kiro_agent_hook(agent) == (HookAction.NOT_SUPPORTED, None) + assert path.read_text() == content + + +def test_kiro_agent_hook_added_to_default_agent(tmp_path): + spawn = {'command': 'git status'} + other = {'matcher': 'fs_write', 'command': 'lint'} + agent = _kiro_with_agents( + tmp_path, + { + 'my-agent.json': { + 'name': 'my-agent', + 'hooks': {'agentSpawn': [spawn], 'preToolUse': [other]}, + } + }, + ) + path = tmp_path / '.kiro' / 'agents' / 'my-agent.json' + assert install_kiro_agent_hook(agent) == (HookAction.INSTALLED, str(path)) + assert json.loads(path.read_text()) == { + 'name': 'my-agent', + 'hooks': { + 'agentSpawn': [spawn, _kiro_agent_handler()], + 'preToolUse': [other], + }, + } + before = path.read_bytes() + assert install_kiro_agent_hook(agent)[0] is HookAction.ALREADY_INSTALLED + assert path.read_bytes() == before + + +def test_kiro_agent_hook_install_then_remove_restores_file(tmp_path): + original = {'name': 'my-agent', 'prompt': 'Hi'} + agent = _kiro_with_agents(tmp_path, {'my-agent.json': original}) + path = tmp_path / '.kiro' / 'agents' / 'my-agent.json' + before = path.read_text() + install_kiro_agent_hook(agent) + assert remove_kiro_agent_hooks(agent) == [(HookAction.REMOVED, str(path))] + assert path.read_text() == before + + +def test_kiro_agent_handler_running_our_command_is_ours(tmp_path): + changed = {'command': hook_shell_command('kiro'), 'timeout_ms': 10} + agent = _kiro_with_agents( + tmp_path, + { + 'my-agent.json': { + 'name': 'my-agent', + 'hooks': {'agentSpawn': [changed]}, + } + }, + ) + path = tmp_path / '.kiro' / 'agents' / 'my-agent.json' + before = path.read_bytes() + assert install_kiro_agent_hook(agent) == ( + HookAction.ALREADY_INSTALLED, + str(path), + ) + assert path.read_bytes() == before + assert remove_kiro_agent_hooks(agent) == [(HookAction.REMOVED, str(path))] + assert json.loads(path.read_text()) == {'name': 'my-agent'} + + +def test_kiro_remove_skips_v3_agent(tmp_path): + hook = { + 'trigger': 'AgentSpawn', + 'action': {'type': 'command', 'command': hook_shell_command('kiro')}, + } + agent = _kiro_with_agents( + tmp_path, {'my-agent.json': {'name': 'my-agent', 'hooks': [hook]}} + ) + path = tmp_path / '.kiro' / 'agents' / 'my-agent.json' + before = path.read_bytes() + assert remove_kiro_agent_hooks(agent) == [] + assert path.read_bytes() == before + + +def test_kiro_agent_hook_adds_ours_next_to_other_commands(tmp_path): + theirs = {'command': 'echo mine', 'timeout_ms': 10} + agent = _kiro_with_agents( + tmp_path, + { + 'my-agent.json': { + 'name': 'my-agent', + 'hooks': {'agentSpawn': [theirs]}, + } + }, + ) + assert install_kiro_agent_hook(agent)[0] is HookAction.INSTALLED + path = tmp_path / '.kiro' / 'agents' / 'my-agent.json' + assert json.loads(path.read_text())['hooks'] == { + 'agentSpawn': [theirs, _kiro_agent_handler()] + } + + +def test_kiro_agent_with_comments_is_left_untouched(tmp_path): + content = '{\n // mine\n "name": "my-agent"\n}\n' + agent = _kiro_with_agents(tmp_path, {'my-agent.json': content}) + path = tmp_path / '.kiro' / 'agents' / 'my-agent.json' + assert install_kiro_agent_hook(agent) == (HookAction.UNREADABLE, str(path)) + assert path.read_text() == content + + +@pytest.mark.parametrize('jackson', [False, True]) +def test_kiro_v3_agent_is_not_needed(tmp_path, jackson): + content = {'name': 'my-agent', 'hooks': [{'trigger': 'SessionStart'}]} + if jackson: + # Formatting written by tools such as Jackson. + content = json.dumps(content, indent=2).replace('": ', '" : ') + agent = _kiro_with_agents(tmp_path, {'my-agent.json': content}) + path = tmp_path / '.kiro' / 'agents' / 'my-agent.json' + before = path.read_text() + assert install_kiro_agent_hook(agent) == (HookAction.NOT_NEEDED, str(path)) + assert path.read_text() == before + + +def test_kiro_agent_hooks_removed_from_every_agent(tmp_path): + ours = {'hooks': {'agentSpawn': [_kiro_agent_handler()]}} + unreadable = ( + '{\n // mine\n "hooks": {"agentSpawn": [{"command": ' + f'{json.dumps(hook_shell_command("kiro"))}}}]}}\n}}' + ) + agent = _kiro_with_agents( + tmp_path, + { + 'a.json': {'name': 'a', **ours}, + 'b.json': {'name': 'b', **ours}, + 'c.json': {'name': 'c'}, + 'd.json': unreadable, + }, + default='b', + ) + agents_dir = tmp_path / '.kiro' / 'agents' + assert remove_kiro_agent_hooks(agent) == [ + (HookAction.REMOVED, str(agents_dir / 'a.json')), + (HookAction.REMOVED, str(agents_dir / 'b.json')), + (HookAction.UNREADABLE, str(agents_dir / 'd.json')), + ] + assert json.loads((agents_dir / 'a.json').read_text()) == {'name': 'a'} + assert (agents_dir / 'd.json').read_text() == unreadable + assert remove_kiro_agent_hooks(agent) == [ + (HookAction.UNREADABLE, str(agents_dir / 'd.json')) + ] + + +def test_kiro_agent_manual_entry_is_the_installed_entry(): + text = format_kiro_agent_manual_entry('~/x.json', indent='') + first, rest = text.split('\n', 1) + assert first == 'Add this hook to ~/x.json:' + assert json.loads(rest) == { + 'hooks': {'agentSpawn': [_kiro_agent_handler()]} + } + + def test_unsupported_agent(tmp_path): agent = _agent(tmp_path, 'cline') - assert not is_hook_supported('cline') + assert install_hook(agent) == (HookAction.NOT_SUPPORTED, None) + assert remove_hook(agent) == (HookAction.NOT_SUPPORTED, None) assert hook_config_path(agent.config) is None + assert os.listdir(tmp_path / '.cline') == [] + + +def test_write_failure_leaves_file(tmp_path, monkeypatch): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + path.write_text('{}\n') + + def fail(*args, **kwargs): + raise OSError('disk full') + + monkeypatch.setattr(hooks, 'write_file_atomic', fail) + with pytest.raises(OSError): + install_hook(agent) + assert path.read_text() == '{}\n' + + +@pytest.mark.parametrize('agent_id', sorted(_HOOK_FILES)) +def test_manual_entry_is_the_installed_entry(agent_id): + text = format_manual_entry(agent_id, '~/x.json', indent='') + first, rest = text.split('\n', 1) + assert first == 'Add this hook to ~/x.json:' + assert json.loads(rest) == _expected_entry(agent_id) + + +def test_hook_command_has_no_shell_syntax(): + for agent_id in hooks.HOOK_AGENT_IDS: + assert hook_command(agent_id) == ( + f'aws agent-toolkit update-skill --hook --agent {agent_id} ' + '--no-cli-auto-prompt' + ) + + +@pytest.mark.parametrize( + 'agent_id, windows, suffix', + [ + ('claude-code', False, ' || true'), + ('kiro', False, ' || true'), + ('claude-code', True, '; exit 0'), + ('gemini-cli', True, '; exit 0'), + ('codex', True, '; exit 0'), + ('cursor', True, '; exit 0'), + ('kiro', True, ' || verify > nul'), + ], +) +def test_hook_shell_command_always_exits_zero(agent_id, windows, suffix): + with patch.object(hooks, 'is_windows', windows): + assert hook_shell_command(agent_id) == hook_command(agent_id) + suffix + + +@pytest.mark.parametrize( + 'command', + [ + # Earlier forms of our command, and one the customer changed. + 'aws agent-toolkit update-skill --all --hook claude-code', + 'aws agent-toolkit update-skill --all --hook claude-code ' + '--no-cli-auto-prompt', + '/opt/aws/bin/aws agent-toolkit update-skill --hook claude-code ' + '--debug', + ], +) +def test_handler_running_update_skill_hook_is_ours(tmp_path, command): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + variant = _our_group('claude-code') + variant['hooks'][0]['command'] = command + path.write_text(json.dumps({'hooks': {'SessionStart': [variant]}})) + before = path.read_bytes() + assert install_hook(agent)[0] is HookAction.ALREADY_INSTALLED + assert path.read_bytes() == before + assert remove_hook(agent)[0] is HookAction.REMOVED + assert json.loads(path.read_text()) == {} + + +def test_handler_running_other_command_is_not_ours(tmp_path): + agent = _agent(tmp_path, 'claude-code') + path = tmp_path / '.claude-code' / 'settings.json' + theirs = _our_group('claude-code') + theirs['hooks'][0]['command'] = 'aws agent-toolkit update-skill --all' + path.write_text(json.dumps({'hooks': {'SessionStart': [theirs]}})) + assert remove_hook(agent)[0] is HookAction.NOT_INSTALLED + assert install_hook(agent)[0] is HookAction.INSTALLED + assert json.loads(path.read_text())['hooks']['SessionStart'] == [ + theirs, + _our_group('claude-code'), + ] def test_hook_message(): diff --git a/tests/unit/customizations/agenttoolkit/test_update_skill.py b/tests/unit/customizations/agenttoolkit/test_update_skill.py index 6b92c9cc91f1..c11f433b35db 100644 --- a/tests/unit/customizations/agenttoolkit/test_update_skill.py +++ b/tests/unit/customizations/agenttoolkit/test_update_skill.py @@ -1230,15 +1230,7 @@ def test_hook_command_ignores_auto_prompt_setting( for name in ('AWS_PROFILE', 'AWS_DEFAULT_PROFILE'): monkeypatch.delenv(name, raising=False) state.set_auto_update(False) - args = [ - 'agent-toolkit', - 'update-skill', - '--all', - '--hook', - '--agent', - 'codex', - '--no-cli-auto-prompt', - ] + args = hooks.hook_command('codex').split()[1:] assert AWSCLIEntryPoint().main(args) == 0 assert capsys.readouterr() == ('', '') @@ -1479,7 +1471,7 @@ def test_update_all_does_not_install_core_skills(tmp_path): def test_hook_run_right_after_another_does_nothing(tmp_path, auto_update_on): - # An agent can start the hook twice for one session. + # Kiro V3 runs both its standalone hook and the agent's agentSpawn hook. _install_skill_at_version(tmp_path, '.test-agent', 'aws-s3', 'v1') _, output = _run_hook(tmp_path, 'kiro', _make_skill_client()) assert output == 'AWS agent skills updated: aws-s3.\n' @@ -1595,6 +1587,49 @@ def test_hook_installs_core_skill_into_each_agent_missing_it( assert other_skill.read_text() == 'name: aws-iam\n' +def test_hook_records_unfinished_check_before_running( + tmp_path, auto_update_on +): + _install_skill_at_version(tmp_path, '.test-agent', 'aws-s3', 'v1') + client = _make_skill_client() + seen = [] + + def get_latest_skill_version(**kwargs): + seen.append(_last_run(tmp_path)) + return {'body': 'v2'} + + client.get_latest_skill_version.side_effect = get_latest_skill_version + _run_hook(tmp_path, 'kiro', client) + assert seen[0]['is_successful'] is False + assert seen[0]['summary'] == 'The check did not finish' + # The finished run replaces it. + assert _last_run(tmp_path)['is_successful'] is True + + +def test_hook_killed_mid_run_is_retried_after_retry_interval( + tmp_path, auto_update_on +): + _install_skill_at_version(tmp_path, '.test-agent', 'aws-s3', 'v1') + client = _make_skill_client() + # Stands in for the agent killing the hook at its timeout. + client.get_latest_skill_version.side_effect = SystemExit() + with pytest.raises(SystemExit): + _run_hook(tmp_path, 'kiro', client) + run = _last_run(tmp_path) + assert run['is_successful'] is False + assert run['summary'] == 'The check did not finish' + + client = _make_skill_client() + assert _run_hook(tmp_path, 'kiro', client) == (0, '') + assert not client.get_latest_skill_version.called + + _record_check( + tmp_path, update_skill.HOOK_RETRY_INTERVAL, is_successful=False + ) + _, output = _run_hook(tmp_path, 'kiro', client) + assert output == 'AWS agent skills updated: aws-s3.\n' + + def test_hook_leaves_skill_dropped_from_catalog(tmp_path, auto_update_on): _install_skill_at_version(tmp_path, '.test-agent', 'aws-gone', 'v1') _install_skill_at_version(tmp_path, '.test-agent', 'aws-s3', 'v2')