diff --git a/docs/users/features/hooks.md b/docs/users/features/hooks.md index 1d063413d..f61998128 100644 --- a/docs/users/features/hooks.md +++ b/docs/users/features/hooks.md @@ -152,6 +152,8 @@ Hooks fire at specific points during a Qwen Code session. Different events suppo | `PreCompact` | Before conversation compaction | Trigger (`manual`, `auto`) | | `Notification` | When notifications are sent | Type (`permission_prompt`, `idle_prompt`, `auth_success`) | | `PermissionRequest` | When permission dialog is shown | Tool name | +| `TodoCreated` | When a new todo item is created | None (always fires) | +| `TodoCompleted` | When a todo item is marked as completed | None (always fires) | ### Matcher Patterns @@ -165,6 +167,7 @@ Hooks fire at specific points during a Qwen Code session. Different events suppo | Session Events | `SessionEnd` | ✅ Regex | Reason: `clear`, `logout`, `prompt_input_exit`, etc. | | Notification Events | `Notification` | ✅ Exact match | Type: `permission_prompt`, `idle_prompt`, `auth_success` | | Compact Events | `PreCompact` | ✅ Exact match | Trigger: `manual`, `auto` | +| Todo Events | `TodoCreated`, `TodoCompleted` | ❌ No | N/A | | Prompt Events | `UserPromptSubmit` | ❌ No | N/A | | Stop Events | `Stop` | ❌ No | N/A | @@ -754,6 +757,204 @@ Hook output supports three categories of fields: } ``` +#### TodoCreated + +**Purpose**: Executed when a new todo item is created via the `todo_write` tool. Allows validation, logging, or blocking of todo creation. + +Todo hooks run in two phases: + +- `validation`: runs before persistence. Use this phase for validation only; returning `block` or `deny` prevents the write. +- `postWrite`: runs after persistence. Use this phase for side effects such as logging or syncing; `block` or `deny` is ignored in this phase. + +**Event-specific fields**: + +```json +{ + "todo_id": "unique identifier for the todo item", + "todo_content": "content/description of the todo item", + "todo_status": "pending | in_progress | completed", + "all_todos": "array of all todo items in the current list", + "phase": "validation | postWrite" +} +``` + +**Output Options**: + +- `decision`: "allow", "block", or "deny" +- `reason`: human-readable explanation for the decision (required when blocking) + +**Blocking Behavior**: + +During the `validation` phase, when `decision` is `block` or `deny` (exit code 2), todo creation is prevented. The todo list remains unchanged, and the reason is provided as feedback to the model. + +During the `postWrite` phase, the todo has already been persisted. Hooks may still return output, but `block` / `deny` does not undo the write and should not be used for validation. + +**Example Output (Allow)**: + +```json +{ + "decision": "allow", + "reason": "Todo content validated successfully" +} +``` + +**Example Output (Block)**: + +```json +{ + "decision": "block", + "reason": "Todo content too short. Minimum 5 characters required." +} +``` + +**Example Hook Script**: + +```bash +#!/bin/bash +# ~/.qwen/hooks/todo-validator.sh +# Validates todo content before creation + +INPUT=$(cat) +CONTENT=$(echo "$INPUT" | jq -r '.todo_content') + +# Check minimum length +if [ ${#CONTENT} -lt 5 ]; then + echo '{"decision": "block", "reason": "Todo content must be at least 5 characters"}' + exit 2 +fi + +# Block test-related todos +if [[ "$CONTENT" =~ "test" ]]; then + echo '{"decision": "block", "reason": "Test todos are not allowed in production"}' + exit 2 +fi + +echo '{"decision": "allow"}' +exit 0 +``` + +**Example Configuration**: + +```json +{ + "hooks": { + "TodoCreated": [ + { + "hooks": [ + { + "type": "command", + "command": "$HOME/.qwen/hooks/todo-validator.sh", + "name": "todo-validator", + "timeout": 5000 + } + ] + } + ] + } +} +``` + +#### TodoCompleted + +**Purpose**: Executed when a todo item is marked as completed. Allows validation, logging, or blocking of todo completion. + +Todo hooks run in two phases: + +- `validation`: runs before persistence. Use this phase for validation only; returning `block` or `deny` prevents the write. +- `postWrite`: runs after persistence. Use this phase for side effects such as logging or syncing; `block` or `deny` is ignored in this phase. + +**Event-specific fields**: + +```json +{ + "todo_id": "unique identifier for the todo item", + "todo_content": "content/description of the todo item", + "previous_status": "pending | in_progress (status before completion)", + "all_todos": "array of all todo items in the current list", + "phase": "validation | postWrite" +} +``` + +**Output Options**: + +- `decision`: "allow", "block", or "deny" +- `reason`: human-readable explanation for the decision (required when blocking) + +**Blocking Behavior**: + +During the `validation` phase, when `decision` is `block` or `deny` (exit code 2), todo completion is prevented. The todo item remains in its previous status, and the reason is provided as feedback to the model. + +During the `postWrite` phase, the todo has already been persisted. Hooks may still return output, but `block` / `deny` does not undo the write and should not be used for validation. + +**Example Output (Allow)**: + +```json +{ + "decision": "allow", + "reason": "Todo completion approved" +} +``` + +**Example Output (Block)**: + +```json +{ + "decision": "block", + "reason": "Cannot complete this todo until dependent tasks are finished." +} +``` + +**Example Hook Script**: + +```bash +#!/bin/bash +# ~/.qwen/hooks/todo-completion-validator.sh +# Validates todo completion conditions + +INPUT=$(cat) +TODO_ID=$(echo "$INPUT" | jq -r '.todo_id') +ALL_TODOS=$(echo "$INPUT" | jq -r '.all_todos') + +# Check if there are incomplete dependent todos (example logic) +INCOMPLETE_COUNT=$(echo "$ALL_TODOS" | jq '[.[] | select(.status != "completed")] | length') + +if [ "$INCOMPLETE_COUNT" -gt 5 ]; then + echo '{"decision": "block", "reason": "Too many incomplete todos. Complete other tasks first."}' + exit 2 +fi + +echo '{"decision": "allow"}' +exit 0 +``` + +**Example Configuration**: + +```json +{ + "hooks": { + "TodoCompleted": [ + { + "hooks": [ + { + "type": "command", + "command": "$HOME/.qwen/hooks/todo-completion-validator.sh", + "name": "completion-validator", + "timeout": 5000 + } + ] + } + ] + } +} +``` + +**Use Cases**: + +- **Logging**: Track todo creation and completion for audit or analytics +- **Validation**: Enforce content quality standards (minimum length, required keywords) +- **Workflow Control**: Block completion until prerequisites are met +- **Integration**: Sync todos with external task management systems (Jira, Trello, etc.) + ## Hook Configuration Hooks are configured in Qwen Code settings, typically in `.qwen/settings.json` or user configuration files: diff --git a/packages/cli/src/i18n/locales/de.js b/packages/cli/src/i18n/locales/de.js index c236d6493..39a49e9e2 100644 --- a/packages/cli/src/i18n/locales/de.js +++ b/packages/cli/src/i18n/locales/de.js @@ -625,6 +625,10 @@ export default { 'When a session is ending': 'Wenn eine Sitzung endet', 'When a permission dialog is displayed': 'Wenn ein Berechtigungsdialog angezeigt wird', + 'When a new todo item is created': + 'Wenn ein neues Todo-Element erstellt wird', + 'When a todo item is marked as completed': + 'Wenn ein Todo-Element als erledigt markiert wird', // Hooks - Event Descriptions (detailed) 'Input to command is JSON of tool call arguments.': 'Die Eingabe an den Befehl ist JSON der Tool-Aufruf-Argumente.', @@ -648,6 +652,10 @@ export default { 'Die Eingabe an den Befehl ist JSON mit Komprimierungsdetails.', 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.': 'Die Eingabe an den Befehl ist JSON mit tool_name, tool_input und tool_use_id. Ausgabe ist JSON mit hookSpecificOutput, das die Entscheidung zum Zulassen oder Ablehnen enthält.', + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'Die Eingabe an den Befehl ist JSON mit todo_id, todo_content, todo_status, all_todos und phase. In validation ist die Ausgabe JSON mit decision (allow/block/deny) und reason. In postWrite wird block/deny ignoriert.', + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'Die Eingabe an den Befehl ist JSON mit todo_id, todo_content, previous_status, all_todos und phase. In validation ist die Ausgabe JSON mit decision (allow/block/deny) und reason. In postWrite wird block/deny ignoriert.', // Hooks - Exit Code Descriptions 'stdout/stderr not shown': 'stdout/stderr nicht angezeigt', 'show stderr to model and continue conversation': @@ -674,6 +682,12 @@ export default { 'stderr nur dem Benutzer anzeigen, aber mit Komprimierung fortfahren', 'use hook decision if provided': 'Hook-Entscheidung verwenden, falls bereitgestellt', + 'allow todo creation': 'Todo-Erstellung zulassen', + 'block todo creation and show reason to model': + 'Todo-Erstellung blockieren und Grund dem Modell anzeigen', + 'allow todo completion': 'Todo-Abschluss zulassen', + 'block todo completion and show reason to model': + 'Todo-Abschluss blockieren und Grund dem Modell anzeigen', // Hooks - Messages 'Config not loaded.': 'Konfiguration nicht geladen.', 'Hooks are not enabled. Enable hooks in settings to use this feature.': diff --git a/packages/cli/src/i18n/locales/en.js b/packages/cli/src/i18n/locales/en.js index ecf686549..400602eea 100644 --- a/packages/cli/src/i18n/locales/en.js +++ b/packages/cli/src/i18n/locales/en.js @@ -713,6 +713,9 @@ export default { 'When a session is ending': 'When a session is ending', 'When a permission dialog is displayed': 'When a permission dialog is displayed', + 'When a new todo item is created': 'When a new todo item is created', + 'When a todo item is marked as completed': + 'When a todo item is marked as completed', // Hooks - Event Descriptions (detailed) 'Input to command is JSON of tool call arguments.': 'Input to command is JSON of tool call arguments.', @@ -736,6 +739,10 @@ export default { 'Input to command is JSON with compaction details.', 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.': 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.', + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.', + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.', // Hooks - Exit Code Descriptions 'stdout/stderr not shown': 'stdout/stderr not shown', 'show stderr to model and continue conversation': @@ -761,6 +768,12 @@ export default { 'show stderr to user only but continue with compaction': 'show stderr to user only but continue with compaction', 'use hook decision if provided': 'use hook decision if provided', + 'allow todo creation': 'allow todo creation', + 'block todo creation and show reason to model': + 'block todo creation and show reason to model', + 'allow todo completion': 'allow todo completion', + 'block todo completion and show reason to model': + 'block todo completion and show reason to model', // Hooks - Messages 'Config not loaded.': 'Config not loaded.', 'Hooks are not enabled. Enable hooks in settings to use this feature.': diff --git a/packages/cli/src/i18n/locales/fr.js b/packages/cli/src/i18n/locales/fr.js index 28b532ac7..90318ade9 100644 --- a/packages/cli/src/i18n/locales/fr.js +++ b/packages/cli/src/i18n/locales/fr.js @@ -692,6 +692,9 @@ export default { 'When a session is ending': 'Quand une session se termine', 'When a permission dialog is displayed': 'Quand un dialogue de permission est affiché', + 'When a new todo item is created': 'Quand un nouvel élément todo est créé', + 'When a todo item is marked as completed': + 'Quand un élément todo est marqué comme terminé', 'Input to command is JSON of tool call arguments.': "L'entrée de la commande est du JSON des arguments d'appel d'outil.", 'Input to command is JSON with fields "inputs" (tool call arguments) and "response" (tool call response).': @@ -714,6 +717,10 @@ export default { "L'entrée de la commande est du JSON avec les détails de compaction.", 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.': "L'entrée de la commande est du JSON avec tool_name, tool_input et tool_use_id. Sortie JSON avec hookSpecificOutput contenant la décision d'autoriser ou de refuser.", + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + "L'entrée de la commande est du JSON avec todo_id, todo_content, todo_status, all_todos et phase. Dans validation, sortie JSON avec decision (allow/block/deny) et reason. Dans postWrite, block/deny est ignoré.", + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + "L'entrée de la commande est du JSON avec todo_id, todo_content, previous_status, all_todos et phase. Dans validation, sortie JSON avec decision (allow/block/deny) et reason. Dans postWrite, block/deny est ignoré.", 'stdout/stderr not shown': 'stdout/stderr non affiché', 'show stderr to model and continue conversation': 'afficher stderr au modèle et continuer la conversation', @@ -738,6 +745,12 @@ export default { 'show stderr to user only but continue with compaction': "afficher stderr à l'utilisateur uniquement mais continuer la compaction", 'use hook decision if provided': 'utiliser la décision du hook si fournie', + 'allow todo creation': 'autoriser la création de todo', + 'block todo creation and show reason to model': + 'bloquer la création de todo et afficher la raison au modèle', + 'allow todo completion': 'autoriser la complétion de todo', + 'block todo completion and show reason to model': + 'bloquer la complétion de todo et afficher la raison au modèle', 'Config not loaded.': 'Configuration non chargée.', 'Hooks are not enabled. Enable hooks in settings to use this feature.': 'Les hooks ne sont pas activés. Activez les hooks dans les paramètres pour utiliser cette fonctionnalité.', diff --git a/packages/cli/src/i18n/locales/ja.js b/packages/cli/src/i18n/locales/ja.js index 93a005b29..d51424257 100644 --- a/packages/cli/src/i18n/locales/ja.js +++ b/packages/cli/src/i18n/locales/ja.js @@ -418,6 +418,8 @@ export default { 'Before conversation compaction': '会話圧縮前', 'When a session is ending': 'セッション終了時', 'When a permission dialog is displayed': '権限ダイアログ表示時', + 'When a new todo item is created': '新Todo項目作成時', + 'When a todo item is marked as completed': 'Todo項目完了時', // Hooks - Event Descriptions (detailed) 'Input to command is JSON of tool call arguments.': 'コマンドへの入力はツール呼び出し引数の JSON です。', @@ -441,6 +443,10 @@ export default { 'コマンドへの入力は圧縮詳細を持つ JSON です。', 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.': 'コマンドへの入力は tool_name、tool_input、tool_use_id を持つ JSON です。許可または拒否の決定を含む hookSpecificOutput を持つ JSON を出力します。', + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'コマンドへの入力は todo_id、todo_content、todo_status、all_todos、phase を持つ JSON です。validation では decision(allow/block/deny)と reason を持つ JSON を出力します。postWrite では block/deny は無視されます。', + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'コマンドへの入力は todo_id、todo_content、previous_status、all_todos、phase を持つ JSON です。validation では decision(allow/block/deny)と reason を持つ JSON を出力します。postWrite では block/deny は無視されます。', // Hooks - Exit Code Descriptions 'stdout/stderr not shown': 'stdout/stderr は表示されません', 'show stderr to model and continue conversation': @@ -466,6 +472,12 @@ export default { 'show stderr to user only but continue with compaction': 'stderr をユーザーのみに表示し、圧縮を続ける', 'use hook decision if provided': '提供されている場合はフックの決定を使用', + 'allow todo creation': 'Todo作成を許可', + 'block todo creation and show reason to model': + 'Todo作成をブロックし、理由をモデルに表示', + 'allow todo completion': 'Todo完了を許可', + 'block todo completion and show reason to model': + 'Todo完了をブロックし、理由をモデルに表示', // Hooks - Messages 'Config not loaded.': '設定が読み込まれていません。', 'Hooks are not enabled. Enable hooks in settings to use this feature.': diff --git a/packages/cli/src/i18n/locales/pt.js b/packages/cli/src/i18n/locales/pt.js index c7b333174..7dde5317d 100644 --- a/packages/cli/src/i18n/locales/pt.js +++ b/packages/cli/src/i18n/locales/pt.js @@ -632,6 +632,9 @@ export default { 'When a session is ending': 'Quando uma sessão está terminando', 'When a permission dialog is displayed': 'Quando um diálogo de permissão é exibido', + 'When a new todo item is created': 'Quando um novo item todo é criado', + 'When a todo item is marked as completed': + 'Quando um item todo é marcado como concluído', // Hooks - Event Descriptions (detailed) 'Input to command is JSON of tool call arguments.': 'A entrada para o comando é JSON dos argumentos da chamada da ferramenta.', @@ -655,6 +658,10 @@ export default { 'A entrada para o comando é JSON com detalhes da compactação.', 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.': 'A entrada para o comando é JSON com tool_name, tool_input e tool_use_id. Saída é JSON com hookSpecificOutput contendo decisão de permitir ou negar.', + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'A entrada para o comando é JSON com todo_id, todo_content, todo_status, all_todos e phase. Em validation, saída é JSON com decision (allow/block/deny) e reason. Em postWrite, block/deny é ignorado.', + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'A entrada para o comando é JSON com todo_id, todo_content, previous_status, all_todos e phase. Em validation, saída é JSON com decision (allow/block/deny) e reason. Em postWrite, block/deny é ignorado.', // Hooks - Exit Code Descriptions 'stdout/stderr not shown': 'stdout/stderr não exibido', 'show stderr to model and continue conversation': @@ -680,6 +687,12 @@ export default { 'show stderr to user only but continue with compaction': 'mostrar stderr apenas ao usuário mas continuar com compactação', 'use hook decision if provided': 'usar decisão do hook se fornecida', + 'allow todo creation': 'permitir criação de todo', + 'block todo creation and show reason to model': + 'bloquear criação de todo e mostrar motivo ao modelo', + 'allow todo completion': 'permitir conclusão de todo', + 'block todo completion and show reason to model': + 'bloquear conclusão de todo e mostrar motivo ao modelo', // Hooks - Messages 'Config not loaded.': 'Configuração não carregada.', 'Hooks are not enabled. Enable hooks in settings to use this feature.': diff --git a/packages/cli/src/i18n/locales/ru.js b/packages/cli/src/i18n/locales/ru.js index d4e4238c5..e1fd2a48e 100644 --- a/packages/cli/src/i18n/locales/ru.js +++ b/packages/cli/src/i18n/locales/ru.js @@ -640,6 +640,9 @@ export default { 'Before conversation compaction': 'Перед сжатием разговора', 'When a session is ending': 'При завершении сессии', 'When a permission dialog is displayed': 'При отображении диалога разрешений', + 'When a new todo item is created': 'При создании новой задачи', + 'When a todo item is marked as completed': + 'При отметке задачи как выполненной', // Hooks - Event Descriptions (detailed) 'Input to command is JSON of tool call arguments.': 'Ввод в команду — это JSON аргументов вызова инструмента.', @@ -663,6 +666,10 @@ export default { 'Ввод в команду — это JSON с деталями сжатия.', 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.': 'Ввод в команду — это JSON с tool_name, tool_input и tool_use_id. Вывод — JSON с hookSpecificOutput, содержащим решение о разрешении или отказе.', + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'Ввод в команду — это JSON с todo_id, todo_content, todo_status, all_todos и phase. В validation вывод — JSON с decision (allow/block/deny) и reason. В postWrite block/deny игнорируется.', + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + 'Ввод в команду — это JSON с todo_id, todo_content, previous_status, all_todos и phase. В validation вывод — JSON с decision (allow/block/deny) и reason. В postWrite block/deny игнорируется.', // Hooks - Exit Code Descriptions 'stdout/stderr not shown': 'stdout/stderr не отображаются', 'show stderr to model and continue conversation': @@ -689,6 +696,12 @@ export default { 'показать stderr только пользователю, но продолжить сжатие', 'use hook decision if provided': 'использовать решение хука, если предоставлено', + 'allow todo creation': 'разрешить создание задачи', + 'block todo creation and show reason to model': + 'заблокировать создание задачи и показать причину модели', + 'allow todo completion': 'разрешить выполнение задачи', + 'block todo completion and show reason to model': + 'заблокировать выполнение задачи и показать причину модели', // Hooks - Messages 'Config not loaded.': 'Конфигурация не загружена.', 'Hooks are not enabled. Enable hooks in settings to use this feature.': diff --git a/packages/cli/src/i18n/locales/zh-TW.js b/packages/cli/src/i18n/locales/zh-TW.js index e768db515..54c3462ab 100644 --- a/packages/cli/src/i18n/locales/zh-TW.js +++ b/packages/cli/src/i18n/locales/zh-TW.js @@ -1427,6 +1427,18 @@ export default { 'Not in plan mode. Use "/plan" to enter plan mode first.': '未處於計劃模式。請先使用 "/plan" 進入計劃模式。', "Set up Qwen Code's status line UI": '配置 Qwen Code 的狀態欄', + 'When a new todo item is created': '建立新待辦事項時', + 'When a todo item is marked as completed': '待辦事項標記為完成時', + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + '命令輸入為包含 todo_id、todo_content、todo_status、all_todos 和 phase 的 JSON。在 validation 中,輸出為包含 decision(allow/block/deny)和 reason 的 JSON。在 postWrite 中,block/deny 會被忽略。', + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + '命令輸入為包含 todo_id、todo_content、previous_status、all_todos 和 phase 的 JSON。在 validation 中,輸出為包含 decision(allow/block/deny)和 reason 的 JSON。在 postWrite 中,block/deny 會被忽略。', + 'allow todo creation': '允許建立待辦事項', + 'block todo creation and show reason to model': + '阻止建立待辦事項並向模型顯示原因', + 'allow todo completion': '允許完成待辦事項', + 'block todo completion and show reason to model': + '阻止完成待辦事項並向模型顯示原因', // === Core: added from PR #3328 === 'Open the memory manager.': '打開記憶管理器。', diff --git a/packages/cli/src/i18n/locales/zh.js b/packages/cli/src/i18n/locales/zh.js index fd430b8c5..eb4cb3e42 100644 --- a/packages/cli/src/i18n/locales/zh.js +++ b/packages/cli/src/i18n/locales/zh.js @@ -508,8 +508,8 @@ export default { '扩展 "{{name}}" 已是最新版本。', 'Updates all extensions or a named extension to the latest version.': '将所有扩展或指定扩展更新到最新版本。', - 'The name of the extension to update.': '要更新的扩展名称。', 'Update all extensions.': '更新所有扩展。', + 'The name of the extension to update.': '要更新的扩展名称。', 'Either an extension name or --all must be provided': '必须提供扩展名称或 --all', 'Lists installed extensions.': '列出已安装的扩展。', @@ -610,6 +610,7 @@ export default { '按 Escape、Ctrl+C 或 Ctrl+D 取消', 'Press Space, Enter, or Escape to dismiss': '按 Space、Enter 或 Escape 关闭', 'No hook selected': '未选择 Hook', + 'Session (temporary)': '会话(临时)', // Hooks - List Step 'No hook events found.': '未找到 Hook 事件。', '{{count}} hook configured': '{{count}} 个 Hook 已配置', @@ -655,7 +656,6 @@ export default { 'User Settings': '用户设置', 'System Settings': '系统设置', Extensions: '扩展', - 'Session (temporary)': '会话(临时)', // Hooks - Event Descriptions (short) 'Before tool execution': '工具执行前', 'After tool execution': '工具执行后', @@ -670,6 +670,8 @@ export default { 'Before conversation compaction': '对话压缩前', 'When a session is ending': '会话结束时', 'When a permission dialog is displayed': '显示权限对话框时', + 'When a new todo item is created': '创建新待办事项时', + 'When a todo item is marked as completed': '待办事项标记为完成时', // Hooks - Event Descriptions (detailed) 'Input to command is JSON of tool call arguments.': '命令输入为工具调用参数的 JSON。', @@ -693,6 +695,10 @@ export default { '命令输入为包含压缩详情的 JSON。', 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.': '命令输入为包含 tool_name、tool_input 和 tool_use_id 的 JSON。输出包含 hookSpecificOutput 的 JSON,其中包含允许或拒绝的决定。', + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + '命令输入为包含 todo_id、todo_content、todo_status、all_todos 和 phase 的 JSON。在 validation 中,输出包含 decision(allow/block/deny)和 reason 的 JSON。在 postWrite 中,block/deny 会被忽略。', + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.': + '命令输入为包含 todo_id、todo_content、previous_status、all_todos 和 phase 的 JSON。在 validation 中,输出包含 decision(allow/block/deny)和 reason 的 JSON。在 postWrite 中,block/deny 会被忽略。', // Hooks - Exit Code Descriptions 'stdout/stderr not shown': 'stdout/stderr 不显示', 'show stderr to model and continue conversation': @@ -717,6 +723,12 @@ export default { 'show stderr to user only but continue with compaction': '仅向用户显示 stderr 但继续压缩', 'use hook decision if provided': '如果提供则使用 Hook 决定', + 'allow todo creation': '允许创建待办事项', + 'block todo creation and show reason to model': + '阻止创建待办事项并向模型显示原因', + 'allow todo completion': '允许完成待办事项', + 'block todo completion and show reason to model': + '阻止完成待办事项并向模型显示原因', // Hooks - Messages 'Config not loaded.': '配置未加载。', 'Hooks are not enabled. Enable hooks in settings to use this feature.': @@ -1290,6 +1302,8 @@ export default { '上下文空间不足,用 /compress 释放空间。', 'Long conversation? /compress summarizes history to free context.': '对话太长?用 /compress 总结历史,释放上下文。', + 'Show context window usage breakdown. Use "/context detail" for per-item breakdown.': + '显示上下文窗口使用情况明细。使用 "/context detail" 查看逐项明细。', // ============================================================================ // Exit Screen / Stats @@ -1456,7 +1470,7 @@ export default { // ============================================================================ 'API key cannot be empty.': 'API Key 不能为空。', 'Invalid API key. Coding Plan API keys start with "sk-sp-". Please check.': - '无效的 API Key,Coding Plan API Key 均以 "sk-sp-" 开头,请检查', + '无效的 API Key。Coding Plan API Key 以 "sk-sp-" 开头,请检查。', 'You can get your Coding Plan API key here': '您可以在这里获取 Coding Plan API Key', 'You can get your Token Plan API key here': @@ -1522,8 +1536,6 @@ export default { '暂无 API 响应。发送消息以查看实际使用情况。', 'Run /context detail for per-item breakdown.': '运行 /context detail 查看详细分解。', - 'Show context window usage breakdown. Use "/context detail" for per-item breakdown.': - '显示上下文窗口使用情况分解。输入 "/context detail" 查看详细分解。', 'body loaded': '内容已加载', memory: '记忆', '{{region}} configuration updated successfully.': '{{region}} 配置更新成功。', diff --git a/packages/cli/src/ui/components/hooks/HooksManagementDialog.tsx b/packages/cli/src/ui/components/hooks/HooksManagementDialog.tsx index 392e91515..5331d7490 100644 --- a/packages/cli/src/ui/components/hooks/HooksManagementDialog.tsx +++ b/packages/cli/src/ui/components/hooks/HooksManagementDialog.tsx @@ -89,31 +89,54 @@ function isValidHookDefinition(def: unknown): def is HookDefinition { /** * Type guard to check if a value is a valid hooks record + * Note: This validates the structure but allows individual events to have + * invalid configs - those will be filtered out during processing. */ -function isValidHooksRecord( - hooks: unknown, -): hooks is Record { +function isValidHooksRecord(hooks: unknown): hooks is Record { if (typeof hooks !== 'object' || hooks === null) { return false; } + // Basic structure check - must be a record with array values for event keys const record = hooks as Record; for (const [key, value] of Object.entries(record)) { // Skip non-event configuration fields if (HOOKS_CONFIG_FIELDS.includes(key)) { continue; } + // Event values should be arrays (even if contents are invalid) if (!Array.isArray(value)) { return false; } - for (const def of value) { - if (!isValidHookDefinition(def)) { - return false; - } - } } return true; } +/** + * Safely extract hook definitions for a specific event + * Returns empty array if the definitions are invalid + */ +function getValidHookDefinitions( + hooksRecord: Record, + eventName: string, +): HookDefinition[] { + const value = hooksRecord[eventName]; + if (!Array.isArray(value)) { + return []; + } + const result: HookDefinition[] = []; + for (const def of value) { + if (isValidHookDefinition(def)) { + result.push(def); + } else { + debugLogger.warn( + `Skipping invalid hook definition for ${eventName}:`, + def, + ); + } + } + return result; +} + export function HooksManagementDialog({ onClose, }: HooksManagementDialogProps): React.JSX.Element { @@ -247,11 +270,12 @@ export function HooksManagementDialog({ for (const eventName of DISPLAY_HOOK_EVENTS) { const hookInfo = createEmptyHookEventInfo(eventName); - // Get hooks from user settings (with type validation) + // Get hooks from user settings (with per-event validation) const userSettingsRecord = userSettings as Record; const userHooksRaw = userSettingsRecord?.['hooks']; - if (isValidHooksRecord(userHooksRaw) && userHooksRaw[eventName]) { - for (const def of userHooksRaw[eventName]) { + if (isValidHooksRecord(userHooksRaw)) { + const userDefs = getValidHookDefinitions(userHooksRaw, eventName); + for (const def of userDefs) { for (const hookConfig of def.hooks) { hookInfo.configs.push({ config: hookConfig, @@ -263,17 +287,18 @@ export function HooksManagementDialog({ } } - // Get hooks from workspace settings (with type validation) + // Get hooks from workspace settings (with per-event validation) const workspaceSettingsRecord = workspaceSettings as Record< string, unknown >; const workspaceHooksRaw = workspaceSettingsRecord?.['hooks']; - if ( - isValidHooksRecord(workspaceHooksRaw) && - workspaceHooksRaw[eventName] - ) { - for (const def of workspaceHooksRaw[eventName]) { + if (isValidHooksRecord(workspaceHooksRaw)) { + const workspaceDefs = getValidHookDefinitions( + workspaceHooksRaw, + eventName, + ); + for (const def of workspaceDefs) { for (const hookConfig of def.hooks) { hookInfo.configs.push({ config: hookConfig, diff --git a/packages/cli/src/ui/components/hooks/constants.test.ts b/packages/cli/src/ui/components/hooks/constants.test.ts index 0b49c7136..b24fec3b6 100644 --- a/packages/cli/src/ui/components/hooks/constants.test.ts +++ b/packages/cli/src/ui/components/hooks/constants.test.ts @@ -176,10 +176,12 @@ describe('hooks constants', () => { expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.PreCompact); expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.PostCompact); expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.PermissionRequest); + expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.TodoCreated); + expect(DISPLAY_HOOK_EVENTS).toContain(HookEventName.TodoCompleted); }); - it('should have 14 events', () => { - expect(DISPLAY_HOOK_EVENTS).toHaveLength(14); + it('should have 16 events', () => { + expect(DISPLAY_HOOK_EVENTS).toHaveLength(16); }); }); @@ -217,5 +219,27 @@ describe('hooks constants', () => { expect(info.exitCodes).toEqual([]); expect(info.configs).toEqual([]); }); + + it('should create empty info for TodoCreated', () => { + const info = createEmptyHookEventInfo(HookEventName.TodoCreated); + + expect(info.event).toBe(HookEventName.TodoCreated); + expect(info.shortDescription).toBe('When a new todo item is created'); + expect(info.description).toContain('todo_id'); + expect(info.exitCodes).toHaveLength(3); + expect(info.configs).toEqual([]); + }); + + it('should create empty info for TodoCompleted', () => { + const info = createEmptyHookEventInfo(HookEventName.TodoCompleted); + + expect(info.event).toBe(HookEventName.TodoCompleted); + expect(info.shortDescription).toBe( + 'When a todo item is marked as completed', + ); + expect(info.description).toContain('previous_status'); + expect(info.exitCodes).toHaveLength(3); + expect(info.configs).toEqual([]); + }); }); }); diff --git a/packages/cli/src/ui/components/hooks/constants.ts b/packages/cli/src/ui/components/hooks/constants.ts index b91178554..a80ca9cc2 100644 --- a/packages/cli/src/ui/components/hooks/constants.ts +++ b/packages/cli/src/ui/components/hooks/constants.ts @@ -94,6 +94,22 @@ export function getHookExitCodes(eventName: string): HookExitCode[] { { code: 0, description: t('use hook decision if provided') }, { code: 'Other', description: t('show stderr to user only') }, ], + [HookEventName.TodoCreated]: [ + { code: 0, description: t('allow todo creation') }, + { + code: 2, + description: t('block todo creation and show reason to model'), + }, + { code: 'Other', description: t('show stderr to user only') }, + ], + [HookEventName.TodoCompleted]: [ + { code: 0, description: t('allow todo completion') }, + { + code: 2, + description: t('block todo completion and show reason to model'), + }, + { code: 'Other', description: t('show stderr to user only') }, + ], }; return exitCodesMap[eventName] || []; } @@ -121,6 +137,8 @@ export function getHookShortDescription(eventName: string): string { [HookEventName.PermissionRequest]: t( 'When a permission dialog is displayed', ), + [HookEventName.TodoCreated]: t('When a new todo item is created'), + [HookEventName.TodoCompleted]: t('When a todo item is marked as completed'), }; return descriptions[eventName] || ''; } @@ -164,6 +182,12 @@ export function getHookDescription(eventName: string): string { [HookEventName.PermissionRequest]: t( 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.', ), + [HookEventName.TodoCreated]: t( + 'Input to command is JSON with todo_id, todo_content, todo_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.', + ), + [HookEventName.TodoCompleted]: t( + 'Input to command is JSON with todo_id, todo_content, previous_status, all_todos, and phase. In validation, output JSON with decision (allow/block/deny) and reason. In postWrite, block/deny is ignored.', + ), }; return descriptions[eventName] || ''; } diff --git a/packages/core/src/hooks/detectTodoChanges.test.ts b/packages/core/src/hooks/detectTodoChanges.test.ts new file mode 100644 index 000000000..6dbcdf48d --- /dev/null +++ b/packages/core/src/hooks/detectTodoChanges.test.ts @@ -0,0 +1,193 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { detectTodoChanges } from './types.js'; +import type { TodoItem } from './types.js'; + +describe('detectTodoChanges', () => { + describe('empty inputs', () => { + it('should return empty changes when both lists are empty', () => { + const result = detectTodoChanges([], []); + expect(result).toEqual({ + created: [], + completed: [], + }); + }); + + it('should mark all todos as created when oldTodos is empty', () => { + const newTodos: TodoItem[] = [ + { id: '1', content: 'Task 1', status: 'pending' }, + { id: '2', content: 'Task 2', status: 'in_progress' }, + ]; + const result = detectTodoChanges([], newTodos); + expect(result.created).toEqual(newTodos); + expect(result.completed).toHaveLength(0); + }); + + it('should return empty changes when newTodos is empty', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'Task 1', status: 'pending' }, + ]; + const result = detectTodoChanges(oldTodos, []); + expect(result).toEqual({ + created: [], + completed: [], + }); + }); + }); + + describe('todo creation', () => { + it('should detect newly created todos', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'Existing Task', status: 'pending' }, + ]; + const newTodos: TodoItem[] = [ + { id: '1', content: 'Existing Task', status: 'pending' }, + { id: '2', content: 'New Task', status: 'pending' }, + ]; + const result = detectTodoChanges(oldTodos, newTodos); + expect(result.created).toEqual([ + { id: '2', content: 'New Task', status: 'pending' }, + ]); + expect(result.completed).toHaveLength(0); + }); + + it('should detect multiple newly created todos', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'A', status: 'pending' }, + ]; + const newTodos: TodoItem[] = [ + { id: '1', content: 'A', status: 'pending' }, + { id: '2', content: 'B', status: 'pending' }, + { id: '3', content: 'C', status: 'in_progress' }, + ]; + const result = detectTodoChanges(oldTodos, newTodos); + expect(result.created).toHaveLength(2); + expect(result.created).toContainEqual({ + id: '2', + content: 'B', + status: 'pending', + }); + expect(result.created).toContainEqual({ + id: '3', + content: 'C', + status: 'in_progress', + }); + }); + }); + + describe('todo completion', () => { + it('should detect todo completion from pending to completed', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'pending' }, + ]; + const newTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'completed' }, + ]; + const result = detectTodoChanges(oldTodos, newTodos); + expect(result.completed).toEqual([ + { id: '1', content: 'Task', status: 'completed' }, + ]); + expect(result.created).toHaveLength(0); + }); + + it('should detect todo completion from in_progress to completed', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'in_progress' }, + ]; + const newTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'completed' }, + ]; + const result = detectTodoChanges(oldTodos, newTodos); + expect(result.completed).toHaveLength(1); + }); + + it('should NOT detect completion when todo was already completed', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'completed' }, + ]; + const newTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'completed' }, + ]; + const result = detectTodoChanges(oldTodos, newTodos); + expect(result.completed).toHaveLength(0); + }); + }); + + describe('non-hooked changes', () => { + it('should ignore status change from pending to in_progress', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'pending' }, + ]; + const newTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'in_progress' }, + ]; + const result = detectTodoChanges(oldTodos, newTodos); + expect(result).toEqual({ + created: [], + completed: [], + }); + }); + + it('should ignore status change from completed back to pending', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'completed' }, + ]; + const newTodos: TodoItem[] = [ + { id: '1', content: 'Task', status: 'pending' }, + ]; + const result = detectTodoChanges(oldTodos, newTodos); + expect(result).toEqual({ + created: [], + completed: [], + }); + }); + + it('should handle content change without status change', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'Old content', status: 'pending' }, + ]; + const newTodos: TodoItem[] = [ + { id: '1', content: 'New content', status: 'pending' }, + ]; + const result = detectTodoChanges(oldTodos, newTodos); + expect(result).toEqual({ + created: [], + completed: [], + }); + }); + }); + + describe('mixed changes', () => { + it('should detect created and completed changes simultaneously', () => { + const oldTodos: TodoItem[] = [ + { id: '1', content: 'Existing pending', status: 'pending' }, + { id: '2', content: 'Existing in_progress', status: 'in_progress' }, + { id: '3', content: 'Existing completed', status: 'completed' }, + ]; + const newTodos: TodoItem[] = [ + { id: '1', content: 'Existing pending', status: 'completed' }, + { id: '2', content: 'Existing in_progress', status: 'pending' }, + { id: '4', content: 'New task', status: 'pending' }, + ]; + const result = detectTodoChanges(oldTodos, newTodos); + + expect(result.created).toHaveLength(1); + expect(result.completed).toHaveLength(1); + expect(result.created).toContainEqual({ + id: '4', + content: 'New task', + status: 'pending', + }); + expect(result.completed).toContainEqual({ + id: '1', + content: 'Existing pending', + status: 'completed', + }); + }); + }); +}); diff --git a/packages/core/src/hooks/hookAggregator.test.ts b/packages/core/src/hooks/hookAggregator.test.ts index f270a845b..7a4a6c169 100644 --- a/packages/core/src/hooks/hookAggregator.test.ts +++ b/packages/core/src/hooks/hookAggregator.test.ts @@ -795,6 +795,54 @@ describe('HookAggregator', () => { }); }); + describe('Todo events - mergeWithOrLogic', () => { + it('should block TodoCreated when any hook blocks', () => { + const outputs: HookOutput[] = [ + { reason: 'policy violation', decision: 'block' }, + { reason: 'looks fine', decision: 'allow' }, + ]; + + const results: HookExecutionResult[] = outputs.map((output) => ({ + hookConfig: { type: HookType.Command, command: 'echo test' }, + eventName: HookEventName.TodoCreated, + success: true, + output, + duration: 100, + })); + + const result = aggregator.aggregateResults( + results, + HookEventName.TodoCreated, + ); + expect(result.finalOutput?.decision).toBe('block'); + expect(result.finalOutput?.reason).toBe('policy violation\nlooks fine'); + }); + + it('should block TodoCompleted when a later hook allows', () => { + const outputs: HookOutput[] = [ + { reason: 'already completed elsewhere', decision: 'block' }, + { reason: 'completion approved', decision: 'allow' }, + ]; + + const results: HookExecutionResult[] = outputs.map((output) => ({ + hookConfig: { type: HookType.Command, command: 'echo test' }, + eventName: HookEventName.TodoCompleted, + success: true, + output, + duration: 100, + })); + + const result = aggregator.aggregateResults( + results, + HookEventName.TodoCompleted, + ); + expect(result.finalOutput?.decision).toBe('block'); + expect(result.finalOutput?.reason).toBe( + 'already completed elsewhere\ncompletion approved', + ); + }); + }); + describe('StopFailure - fire-and-forget special handling', () => { it('should always return success true for StopFailure', () => { const results: HookExecutionResult[] = [ diff --git a/packages/core/src/hooks/hookAggregator.ts b/packages/core/src/hooks/hookAggregator.ts index 544b65180..e5eef2225 100644 --- a/packages/core/src/hooks/hookAggregator.ts +++ b/packages/core/src/hooks/hookAggregator.ts @@ -107,6 +107,8 @@ export class HookAggregator { case HookEventName.Stop: case HookEventName.UserPromptSubmit: case HookEventName.SubagentStop: + case HookEventName.TodoCreated: + case HookEventName.TodoCompleted: merged = this.mergeWithOrLogic(outputs, eventName); break; case HookEventName.PermissionRequest: diff --git a/packages/core/src/hooks/hookEventHandler.test.ts b/packages/core/src/hooks/hookEventHandler.test.ts index b9f8aebfa..48b68d051 100644 --- a/packages/core/src/hooks/hookEventHandler.test.ts +++ b/packages/core/src/hooks/hookEventHandler.test.ts @@ -17,6 +17,7 @@ import { PreCompactTrigger, PostCompactTrigger, NotificationType, + HookPhase, } from './types.js'; import type { StopFailureErrorType } from './types.js'; import type { Config } from '../config/config.js'; @@ -941,6 +942,116 @@ describe('HookEventHandler', () => { expect(result.success).toBe(false); expect(result.errors).toHaveLength(1); expect(result.errors[0].message).toBe('PreToolUse planner error'); + expect(result.finalOutput).toBeUndefined(); + }); + }); + + describe('todo hook fail-closed behavior', () => { + it('should block TodoCreated when hook execution setup fails', async () => { + vi.mocked(mockHookPlanner.createExecutionPlan).mockImplementation(() => { + throw new Error('TodoCreated planner error'); + }); + + const result = await hookEventHandler.fireTodoCreatedEvent( + 'todo-1', + 'secret token: abc123', + 'pending', + [{ id: 'todo-1', content: 'secret token: abc123', status: 'pending' }], + HookPhase.Validation, + ); + + expect(result.success).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBe('TodoCreated planner error'); + expect(result.finalOutput).toEqual({ + decision: 'block', + reason: + 'Hook system failed while processing TodoCreated: TodoCreated planner error', + }); + }); + + it('should block TodoCompleted when hook execution setup fails', async () => { + vi.mocked(mockHookPlanner.createExecutionPlan).mockImplementation(() => { + throw new Error('TodoCompleted planner error'); + }); + + const result = await hookEventHandler.fireTodoCompletedEvent( + 'todo-1', + 'internal host: db.internal', + 'in_progress', + [ + { + id: 'todo-1', + content: 'internal host: db.internal', + status: 'completed', + }, + ], + HookPhase.Validation, + ); + + expect(result.success).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBe('TodoCompleted planner error'); + expect(result.finalOutput).toEqual({ + decision: 'block', + reason: + 'Hook system failed while processing TodoCompleted: TodoCompleted planner error', + }); + }); + + it('should redact sensitive todo fields from hook telemetry', async () => { + const mockPlan = createMockExecutionPlan([ + { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + ]); + vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(mockPlan); + vi.mocked(mockHookRunner.executeHooksParallel).mockResolvedValue([ + { + hookConfig: { + type: HookType.Command, + command: 'echo test', + source: HooksConfigSource.Project, + }, + eventName: HookEventName.TodoCreated, + success: true, + output: undefined, + duration: 12, + exitCode: 0, + stdout: '', + stderr: '', + }, + ]); + vi.mocked(mockHookAggregator.aggregateResults).mockReturnValue( + createMockAggregatedResult(true), + ); + + await hookEventHandler.fireTodoCreatedEvent( + 'todo-1', + 'api_key=super-secret', + 'pending', + [ + { + id: 'todo-1', + content: 'api_key=super-secret', + status: 'pending', + }, + ], + HookPhase.PostWrite, + ); + + expect(logHookCall).toHaveBeenCalledTimes(1); + const hookCallEvent = vi.mocked(logHookCall).mock.calls[0]?.[1]; + expect(hookCallEvent?.hook_input).toMatchObject({ + hook_event_name: HookEventName.TodoCreated, + todo_id: 'todo-1', + todo_status: 'pending', + phase: HookPhase.PostWrite, + }); + expect(hookCallEvent?.hook_input).not.toHaveProperty('todo_content'); + expect(hookCallEvent?.hook_input).not.toHaveProperty('all_todos'); }); }); diff --git a/packages/core/src/hooks/hookEventHandler.ts b/packages/core/src/hooks/hookEventHandler.ts index 567a367ed..48c8c8455 100644 --- a/packages/core/src/hooks/hookEventHandler.ts +++ b/packages/core/src/hooks/hookEventHandler.ts @@ -38,8 +38,12 @@ import type { FunctionHookContext, StopFailureInput, StopFailureErrorType, + TodoCreatedInput, + TodoCompletedInput, + TodoItem, + TodoStatus, } from './types.js'; -import { PermissionMode } from './types.js'; +import { HookPhase, PermissionMode } from './types.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { logHookCall } from '../telemetry/loggers.js'; import { HookCallEvent } from '../telemetry/types.js'; @@ -473,6 +477,66 @@ export class HookEventHandler { ); } + /** + * Fire a TodoCreated event + * Called when a new todo item is added to the list + */ + async fireTodoCreatedEvent( + todoId: string, + todoContent: string, + todoStatus: TodoStatus, + allTodos: TodoItem[], + phase: HookPhase, + signal?: AbortSignal, + ): Promise { + const input: TodoCreatedInput = { + ...this.createBaseInput(HookEventName.TodoCreated), + hook_event_name: 'TodoCreated', + todo_id: todoId, + todo_content: todoContent, + todo_status: todoStatus, + all_todos: allTodos, + phase, + }; + + return this.executeHooks( + HookEventName.TodoCreated, + input, + undefined, + signal, + ); + } + + /** + * Fire a TodoCompleted event + * Called when a todo item's status changes to 'completed' + */ + async fireTodoCompletedEvent( + todoId: string, + todoContent: string, + previousStatus: 'pending' | 'in_progress', + allTodos: TodoItem[], + phase: HookPhase, + signal?: AbortSignal, + ): Promise { + const input: TodoCompletedInput = { + ...this.createBaseInput(HookEventName.TodoCompleted), + hook_event_name: 'TodoCompleted', + todo_id: todoId, + todo_content: todoContent, + previous_status: previousStatus, + all_todos: allTodos, + phase, + }; + + return this.executeHooks( + HookEventName.TodoCompleted, + input, + undefined, + signal, + ); + } + /** * Execute hooks for a specific event (direct execution without MessageBus) * Used as fallback when MessageBus is not available @@ -483,6 +547,21 @@ export class HookEventHandler { context?: HookEventContext, signal?: AbortSignal, ): Promise { + const failClosedResult: AggregatedHookResult = { + success: false, + allOutputs: [], + errors: [], + totalDuration: 0, + finalOutput: + eventName === HookEventName.TodoCreated || + eventName === HookEventName.TodoCompleted + ? { + decision: 'block', + reason: `Hook system failed while processing ${eventName}`, + } + : undefined, + }; + try { // Create execution plan from registry hooks const plan = this.hookPlanner.createExecutionPlan(eventName, context); @@ -578,12 +657,13 @@ export class HookEventHandler { } catch (error) { debugLogger.error(`Hook event bus error for ${eventName}: ${error}`); - return { - success: false, - allOutputs: [], - errors: [error instanceof Error ? error : new Error(String(error))], - totalDuration: 0, - }; + const normalizedError = + error instanceof Error ? error : new Error(String(error)); + failClosedResult.errors = [normalizedError]; + if (failClosedResult.finalOutput) { + failClosedResult.finalOutput.reason = `${failClosedResult.finalOutput.reason}: ${normalizedError.message}`; + } + return failClosedResult; } } @@ -629,6 +709,37 @@ export class HookEventHandler { } } + private sanitizeHookInputForTelemetry( + eventName: HookEventName, + input: HookInput, + ): Record { + const telemetryInput: Record = { ...input }; + + if (eventName === HookEventName.TodoCreated) { + delete telemetryInput['todo_content']; + delete telemetryInput['all_todos']; + if ('phase' in telemetryInput) { + telemetryInput['phase'] = + telemetryInput['phase'] === HookPhase.PostWrite + ? HookPhase.PostWrite + : HookPhase.Validation; + } + } + + if (eventName === HookEventName.TodoCompleted) { + delete telemetryInput['todo_content']; + delete telemetryInput['all_todos']; + if ('phase' in telemetryInput) { + telemetryInput['phase'] = + telemetryInput['phase'] === HookPhase.PostWrite + ? HookPhase.PostWrite + : HookPhase.Validation; + } + } + + return telemetryInput; + } + /** * Log hook execution for observability */ @@ -657,6 +768,8 @@ export class HookEventHandler { ); } + const telemetryInput = this.sanitizeHookInputForTelemetry(eventName, input); + for (const result of results) { const hookName = this.getHookNameFromResult(result); const hookType = this.getHookTypeFromResult(result); @@ -665,7 +778,7 @@ export class HookEventHandler { eventName, hookType, hookName, - { ...input }, + telemetryInput, result.duration, result.success, result.output ? { ...result.output } : undefined, diff --git a/packages/core/src/hooks/hookSystem.test.ts b/packages/core/src/hooks/hookSystem.test.ts index ae087c5cb..ea1066682 100644 --- a/packages/core/src/hooks/hookSystem.test.ts +++ b/packages/core/src/hooks/hookSystem.test.ts @@ -24,6 +24,7 @@ import { PreCompactTrigger, NotificationType, type PermissionSuggestion, + HookPhase, } from './types.js'; import type { Config } from '../config/config.js'; import type { AggregatedHookResult } from './hookAggregator.js'; @@ -96,6 +97,8 @@ describe('HookSystem', () => { firePermissionRequestEvent: vi.fn(), fireSubagentStartEvent: vi.fn(), fireSubagentStopEvent: vi.fn(), + fireTodoCreatedEvent: vi.fn(), + fireTodoCompletedEvent: vi.fn(), setMessagesProvider: vi.fn(), } as unknown as HookEventHandler; @@ -1685,4 +1688,204 @@ describe('HookSystem', () => { expect(hookSystem.getMessagesProvider()).toBeUndefined(); }); }); + + describe('fireTodoCreatedEvent', () => { + it('should fire TodoCreated event and return AggregatedHookResult', async () => { + const mockResult: AggregatedHookResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 50, + finalOutput: { decision: 'allow' as HookDecision, reason: 'OK' }, + }; + + vi.mocked(mockHookEventHandler.fireTodoCreatedEvent).mockResolvedValue( + mockResult, + ); + + const allTodos = [ + { id: '1', content: 'Test Task', status: 'pending' as const }, + ]; + + const result = await hookSystem.fireTodoCreatedEvent( + '1', + 'Test Task', + 'pending', + allTodos, + HookPhase.Validation, + ); + + expect(result).toEqual(mockResult); + expect(mockHookEventHandler.fireTodoCreatedEvent).toHaveBeenCalledWith( + '1', + 'Test Task', + 'pending', + allTodos, + HookPhase.Validation, + undefined, + ); + }); + + it('should pass abort signal to event handler', async () => { + const mockResult = createMockAggregatedResult(true); + vi.mocked(mockHookEventHandler.fireTodoCreatedEvent).mockResolvedValue( + mockResult, + ); + + const abortController = new AbortController(); + const allTodos = [ + { id: '1', content: 'Task', status: 'pending' as const }, + ]; + + await hookSystem.fireTodoCreatedEvent( + '1', + 'Task', + 'pending', + allTodos, + HookPhase.Validation, + abortController.signal, + ); + + expect(mockHookEventHandler.fireTodoCreatedEvent).toHaveBeenCalledWith( + '1', + 'Task', + 'pending', + allTodos, + HookPhase.Validation, + abortController.signal, + ); + }); + + it('should return blocking result when hook blocks', async () => { + const mockResult: AggregatedHookResult = { + success: false, + allOutputs: [], + errors: [], + totalDuration: 50, + finalOutput: { + decision: 'block' as HookDecision, + reason: 'Invalid todo content', + }, + }; + + vi.mocked(mockHookEventHandler.fireTodoCreatedEvent).mockResolvedValue( + mockResult, + ); + + const allTodos = [ + { id: '1', content: 'test', status: 'pending' as const }, + ]; + + const result = await hookSystem.fireTodoCreatedEvent( + '1', + 'test', + 'pending', + allTodos, + HookPhase.Validation, + ); + + expect(result.finalOutput?.decision).toBe('block'); + expect(result.finalOutput?.reason).toBe('Invalid todo content'); + }); + }); + + describe('fireTodoCompletedEvent', () => { + it('should fire TodoCompleted event and return AggregatedHookResult', async () => { + const mockResult: AggregatedHookResult = { + success: true, + allOutputs: [], + errors: [], + totalDuration: 50, + finalOutput: { decision: 'allow' as HookDecision, reason: 'OK' }, + }; + + vi.mocked(mockHookEventHandler.fireTodoCompletedEvent).mockResolvedValue( + mockResult, + ); + + const allTodos = [ + { id: '1', content: 'Test Task', status: 'completed' as const }, + ]; + + const result = await hookSystem.fireTodoCompletedEvent( + '1', + 'Test Task', + 'pending', + allTodos, + HookPhase.Validation, + ); + + expect(result).toEqual(mockResult); + expect(mockHookEventHandler.fireTodoCompletedEvent).toHaveBeenCalledWith( + '1', + 'Test Task', + 'pending', + allTodos, + HookPhase.Validation, + undefined, + ); + }); + + it('should pass abort signal to event handler', async () => { + const mockResult = createMockAggregatedResult(true); + vi.mocked(mockHookEventHandler.fireTodoCompletedEvent).mockResolvedValue( + mockResult, + ); + + const abortController = new AbortController(); + const allTodos = [ + { id: '1', content: 'Task', status: 'completed' as const }, + ]; + + await hookSystem.fireTodoCompletedEvent( + '1', + 'Task', + 'in_progress', + allTodos, + HookPhase.Validation, + abortController.signal, + ); + + expect(mockHookEventHandler.fireTodoCompletedEvent).toHaveBeenCalledWith( + '1', + 'Task', + 'in_progress', + allTodos, + HookPhase.Validation, + abortController.signal, + ); + }); + + it('should return blocking result when hook blocks completion', async () => { + const mockResult: AggregatedHookResult = { + success: false, + allOutputs: [], + errors: [], + totalDuration: 50, + finalOutput: { + decision: 'block' as HookDecision, + reason: 'Task not ready for completion', + }, + }; + + vi.mocked(mockHookEventHandler.fireTodoCompletedEvent).mockResolvedValue( + mockResult, + ); + + const allTodos = [ + { id: '1', content: 'Task', status: 'completed' as const }, + ]; + + const result = await hookSystem.fireTodoCompletedEvent( + '1', + 'Task', + 'in_progress', + allTodos, + HookPhase.Validation, + ); + + expect(result.finalOutput?.decision).toBe('block'); + expect(result.finalOutput?.reason).toBe('Task not ready for completion'); + }); + }); }); diff --git a/packages/core/src/hooks/hookSystem.ts b/packages/core/src/hooks/hookSystem.ts index 3b5ab51c5..0072cbdee 100644 --- a/packages/core/src/hooks/hookSystem.ts +++ b/packages/core/src/hooks/hookSystem.ts @@ -12,7 +12,7 @@ import { HookPlanner } from './hookPlanner.js'; import { HookEventHandler } from './hookEventHandler.js'; import type { HookRegistryEntry } from './hookRegistry.js'; import { createDebugLogger } from '../utils/debugLogger.js'; -import type { DefaultHookOutput } from './types.js'; +import type { DefaultHookOutput, HookPhase } from './types.js'; import { createHookOutput } from './types.js'; import type { SessionStartSource, @@ -31,6 +31,8 @@ import type { PendingAsyncOutput, MessagesProvider, StopFailureErrorType, + TodoItem, + TodoStatus, } from './types.js'; import { SessionHooksManager } from './sessionHooksManager.js'; import type { AsyncHookRegistry } from './asyncHookRegistry.js'; @@ -407,6 +409,50 @@ export class HookSystem { : undefined; } + /** + * Fire a TodoCreated event + * Called when a new todo item is added to the list + */ + async fireTodoCreatedEvent( + todoId: string, + todoContent: string, + todoStatus: TodoStatus, + allTodos: TodoItem[], + phase: HookPhase, + signal?: AbortSignal, + ): Promise { + return this.hookEventHandler.fireTodoCreatedEvent( + todoId, + todoContent, + todoStatus, + allTodos, + phase, + signal, + ); + } + + /** + * Fire a TodoCompleted event + * Called when a todo item's status changes to 'completed' + */ + async fireTodoCompletedEvent( + todoId: string, + todoContent: string, + previousStatus: 'pending' | 'in_progress', + allTodos: TodoItem[], + phase: HookPhase, + signal?: AbortSignal, + ): Promise { + return this.hookEventHandler.fireTodoCompletedEvent( + todoId, + todoContent, + previousStatus, + allTodos, + phase, + signal, + ); + } + // ==================== Session Hooks API ==================== /** diff --git a/packages/core/src/hooks/types.ts b/packages/core/src/hooks/types.ts index f3715f1b7..d85c87901 100644 --- a/packages/core/src/hooks/types.ts +++ b/packages/core/src/hooks/types.ts @@ -48,6 +48,21 @@ export enum HookEventName { PermissionRequest = 'PermissionRequest', // StopFailure - When the turn ends due to an API error (instead of Stop) StopFailure = 'StopFailure', + // TodoCreated - When a new todo item is added to the list (Qwen Code specific) + TodoCreated = 'TodoCreated', + // TodoCompleted - When a todo item's status changes to 'completed' (Qwen Code specific) + TodoCompleted = 'TodoCompleted', +} + +/** + * Hook execution phase for todo events + * Used to split validation from side effects for atomic updates + */ +export enum HookPhase { + /** Validation phase - hooks should only check and return block/approve decisions, no side effects */ + Validation = 'validation', + /** PostWrite phase - hooks can perform side effects (logging, HTTP sync, etc.) after data is persisted */ + PostWrite = 'postWrite', } /** @@ -883,6 +898,111 @@ export interface StopFailureInput extends HookInput { */ export type StopFailureOutput = HookOutput; +/** + * Todo item status types + */ +export type TodoStatus = 'pending' | 'in_progress' | 'completed'; + +/** + * TodoCreated hook input + * Fired when a new todo item is added to the list + */ +export interface TodoCreatedInput extends HookInput { + hook_event_name: 'TodoCreated'; + todo_id: string; + todo_content: string; + todo_status: TodoStatus; + all_todos: TodoItem[]; + /** Execution phase: validation (no side effects) or postWrite (side effects allowed) */ + phase: HookPhase; +} + +/** + * TodoCreated hook output + */ +export interface TodoCreatedOutput extends HookOutput { + hookSpecificOutput?: { + hookEventName: 'TodoCreated'; + additionalContext?: string; + }; +} + +/** + * TodoCompleted hook input + * Fired when a todo item's status changes to 'completed' + */ +export interface TodoCompletedInput extends HookInput { + hook_event_name: 'TodoCompleted'; + todo_id: string; + todo_content: string; + previous_status: 'pending' | 'in_progress'; + all_todos: TodoItem[]; + /** Execution phase: validation (no side effects) or postWrite (side effects allowed) */ + phase: HookPhase; +} + +/** + * TodoCompleted hook output + */ +export interface TodoCompletedOutput extends HookOutput { + hookSpecificOutput?: { + hookEventName: 'TodoCompleted'; + additionalContext?: string; + }; +} + +/** + * Todo item structure (mirrors the one in todoWrite.ts) + */ +export interface TodoItem { + id: string; + content: string; + status: TodoStatus; +} + +/** + * Changes detected when comparing old and new todo lists + */ +export interface TodoChanges { + created: TodoItem[]; + completed: TodoItem[]; +} + +/** + * Compare old and new todo lists to detect changes + * @param oldTodos The previous todo list + * @param newTodos The new todo list + * @returns TodoChanges containing created and completed items + */ +export function detectTodoChanges( + oldTodos: TodoItem[], + newTodos: TodoItem[], +): TodoChanges { + const oldTodosMap = new Map(oldTodos.map((t) => [t.id, t])); + + const changes: TodoChanges = { + created: [], + completed: [], + }; + + for (const newTodo of newTodos) { + const oldTodo = oldTodosMap.get(newTodo.id); + + if (!oldTodo) { + // New todo created (ID not found in old todos) + changes.created.push(newTodo); + } else if ( + oldTodo.status !== 'completed' && + newTodo.status === 'completed' + ) { + // Todo completed (status changed to 'completed') + changes.completed.push(newTodo); + } + } + + return changes; +} + /** * Hook execution result */ diff --git a/packages/core/src/tools/todoWrite.test.ts b/packages/core/src/tools/todoWrite.test.ts index cb096a60c..ceebfc148 100644 --- a/packages/core/src/tools/todoWrite.test.ts +++ b/packages/core/src/tools/todoWrite.test.ts @@ -5,12 +5,14 @@ */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import type { TodoWriteParams, TodoItem } from './todoWrite.js'; +import type { TodoWriteParams } from './todoWrite.js'; import { TodoWriteTool, listTodoSessions } from './todoWrite.js'; +import { DefaultHookOutput, HookPhase, type TodoItem } from '../hooks/types.js'; import * as fs from 'fs/promises'; import * as fsSync from 'fs'; import * as path from 'node:path'; import type { Config } from '../config/config.js'; +import type { AggregatedHookResult } from '../hooks/hookAggregator.js'; import { Storage } from '../config/storage.js'; // Mock fs modules @@ -28,6 +30,7 @@ describe('TodoWriteTool', () => { beforeEach(() => { mockConfig = { getSessionId: () => 'test-session-123', + getHookSystem: () => undefined, } as Config; tool = new TodoWriteTool(mockConfig); mockAbortSignal = new AbortController().signal; @@ -135,8 +138,10 @@ describe('TodoWriteTool', () => { ], }; - // Mock file not existing - mockFs.readFile.mockRejectedValue({ code: 'ENOENT' }); + // Mock file not existing (use proper Error object) + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); mockFs.mkdir.mockResolvedValue(undefined); mockFs.writeFile.mockResolvedValue(undefined); @@ -149,7 +154,7 @@ describe('TodoWriteTool', () => { expect(result.llmContent).toContain(''); expect(result.llmContent).toContain('Your todo list has changed'); expect(result.llmContent).toContain(JSON.stringify(params.todos)); - expect(result.returnDisplay).toEqual({ + expect(result.returnDisplay).toMatchObject({ type: 'todo_list', todos: [ { id: '1', content: 'Task 1', status: 'pending' }, @@ -191,7 +196,7 @@ describe('TodoWriteTool', () => { expect(result.llmContent).toContain(''); expect(result.llmContent).toContain('Your todo list has changed'); expect(result.llmContent).toContain(JSON.stringify(params.todos)); - expect(result.returnDisplay).toEqual({ + expect(result.returnDisplay).toMatchObject({ type: 'todo_list', todos: [ { id: '1', content: 'Updated Task', status: 'completed' }, @@ -213,7 +218,10 @@ describe('TodoWriteTool', () => { ], }; - mockFs.readFile.mockRejectedValue({ code: 'ENOENT' }); + // Mock readTodosFromFile returning empty array (file not existing) + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); mockFs.mkdir.mockResolvedValue(undefined); mockFs.writeFile.mockRejectedValue(new Error('Write failed')); @@ -234,6 +242,12 @@ describe('TodoWriteTool', () => { mockFs.mkdir.mockResolvedValue(undefined); mockFs.writeFile.mockResolvedValue(undefined); + // Mock readTodosFromFile returning existing todos + mockFs.readFile.mockResolvedValue( + JSON.stringify({ + todos: [{ id: '1', content: 'Old Task', status: 'pending' }], + }), + ); const invocation = tool.build(params); const result = await invocation.execute(mockAbortSignal); @@ -242,7 +256,7 @@ describe('TodoWriteTool', () => { expect(result.llmContent).toContain(''); expect(result.llmContent).toContain('Your todo list is now empty'); expect(result.llmContent).toContain('no pending tasks'); - expect(result.returnDisplay).toEqual({ + expect(result.returnDisplay).toMatchObject({ type: 'todo_list', todos: [], }); @@ -252,6 +266,370 @@ describe('TodoWriteTool', () => { 'utf-8', ); }); + + it('should block todo creation when validation hook returns block', async () => { + const hookResult: AggregatedHookResult = { + success: true, + allOutputs: [ + new DefaultHookOutput({ + decision: 'block', + reason: 'Creation denied', + }), + ], + errors: [], + totalDuration: 10, + finalOutput: new DefaultHookOutput({ + decision: 'block', + reason: 'Creation denied', + }), + }; + const mockHookSystem = { + fireTodoCreatedEvent: vi.fn().mockResolvedValue(hookResult), + fireTodoCompletedEvent: vi.fn(), + }; + mockConfig = { + getSessionId: () => 'test-session-123', + getHookSystem: () => mockHookSystem, + } as unknown as Config; + tool = new TodoWriteTool(mockConfig); + + const params: TodoWriteParams = { + todos: [{ id: '1', content: 'Task 1', status: 'pending' }], + }; + + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); + + const invocation = tool.build(params); + const result = await invocation.execute(mockAbortSignal); + + expect(mockHookSystem.fireTodoCreatedEvent).toHaveBeenCalledWith( + '1', + 'Task 1', + 'pending', + params.todos, + HookPhase.Validation, + mockAbortSignal, + ); + expect(mockFs.writeFile).not.toHaveBeenCalled(); + expect(result.llmContent).toContain( + 'Todo creation blocked: Creation denied', + ); + expect(result.returnDisplay).toBe( + 'Todo creation blocked: Creation denied', + ); + }); + + it('should block todo completion when validation hook returns block', async () => { + const hookResult: AggregatedHookResult = { + success: true, + allOutputs: [ + new DefaultHookOutput({ + decision: 'block', + reason: 'Completion denied', + }), + ], + errors: [], + totalDuration: 10, + finalOutput: new DefaultHookOutput({ + decision: 'block', + reason: 'Completion denied', + }), + }; + const mockHookSystem = { + fireTodoCreatedEvent: vi.fn(), + fireTodoCompletedEvent: vi.fn().mockResolvedValue(hookResult), + }; + mockConfig = { + getSessionId: () => 'test-session-123', + getHookSystem: () => mockHookSystem, + } as unknown as Config; + tool = new TodoWriteTool(mockConfig); + + const existingTodos = [ + { id: '1', content: 'Task 1', status: 'in_progress' }, + ]; + const params: TodoWriteParams = { + todos: [{ id: '1', content: 'Task 1', status: 'completed' }], + }; + + mockFs.readFile.mockResolvedValue( + JSON.stringify({ todos: existingTodos }), + ); + + const invocation = tool.build(params); + const result = await invocation.execute(mockAbortSignal); + + expect(mockHookSystem.fireTodoCompletedEvent).toHaveBeenCalledWith( + '1', + 'Task 1', + 'in_progress', + params.todos, + HookPhase.Validation, + mockAbortSignal, + ); + expect(mockFs.writeFile).not.toHaveBeenCalled(); + expect(result.llmContent).toContain( + 'Todo completion blocked: Completion denied', + ); + expect(result.returnDisplay).toBe( + 'Todo completion blocked: Completion denied', + ); + }); + + it('should ignore postWrite block decisions after persistence', async () => { + const hookResult: AggregatedHookResult = { + success: true, + allOutputs: [ + new DefaultHookOutput({ + decision: 'block', + reason: 'Ignored after write', + }), + ], + errors: [], + totalDuration: 10, + finalOutput: new DefaultHookOutput({ + decision: 'block', + reason: 'Ignored after write', + }), + }; + const mockHookSystem = { + fireTodoCreatedEvent: vi + .fn() + .mockResolvedValueOnce({ + success: true, + allOutputs: [new DefaultHookOutput({ decision: 'allow' })], + errors: [], + totalDuration: 10, + finalOutput: new DefaultHookOutput({ decision: 'allow' }), + }) + .mockResolvedValueOnce(hookResult), + fireTodoCompletedEvent: vi.fn(), + }; + mockConfig = { + getSessionId: () => 'test-session-123', + getHookSystem: () => mockHookSystem, + } as unknown as Config; + tool = new TodoWriteTool(mockConfig); + + const params: TodoWriteParams = { + todos: [{ id: '1', content: 'Task 1', status: 'pending' }], + }; + + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); + mockFs.mkdir.mockResolvedValue(undefined); + mockFs.writeFile.mockResolvedValue(undefined); + + const invocation = tool.build(params); + const result = await invocation.execute(mockAbortSignal); + + expect(mockFs.writeFile).toHaveBeenCalled(); + expect(mockHookSystem.fireTodoCreatedEvent).toHaveBeenNthCalledWith( + 1, + '1', + 'Task 1', + 'pending', + params.todos, + HookPhase.Validation, + mockAbortSignal, + ); + expect(mockHookSystem.fireTodoCreatedEvent).toHaveBeenNthCalledWith( + 2, + '1', + 'Task 1', + 'pending', + params.todos, + HookPhase.PostWrite, + mockAbortSignal, + ); + expect(result.llmContent).toContain( + 'Todos have been modified successfully', + ); + }); + + it('should validate created todos concurrently and stop before writing when one blocks', async () => { + let releaseSlowHook: (() => void) | undefined; + const slowValidation = new Promise((resolve) => { + releaseSlowHook = () => + resolve({ + success: true, + allOutputs: [new DefaultHookOutput({ decision: 'allow' })], + errors: [], + totalDuration: 10, + finalOutput: new DefaultHookOutput({ decision: 'allow' }), + }); + }); + + const mockHookSystem = { + fireTodoCreatedEvent: vi + .fn() + .mockImplementationOnce(() => slowValidation) + .mockResolvedValueOnce({ + success: true, + allOutputs: [ + new DefaultHookOutput({ + decision: 'block', + reason: 'Second todo denied', + }), + ], + errors: [], + totalDuration: 10, + finalOutput: new DefaultHookOutput({ + decision: 'block', + reason: 'Second todo denied', + }), + }), + fireTodoCompletedEvent: vi.fn(), + }; + mockConfig = { + getSessionId: () => 'test-session-123', + getHookSystem: () => mockHookSystem, + } as unknown as Config; + tool = new TodoWriteTool(mockConfig); + + const params: TodoWriteParams = { + todos: [ + { id: '1', content: 'Task 1', status: 'pending' }, + { id: '2', content: 'Task 2', status: 'pending' }, + ], + }; + + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); + + const invocation = tool.build(params); + const executionPromise = invocation.execute(mockAbortSignal); + + await vi.waitFor(() => { + expect(mockHookSystem.fireTodoCreatedEvent).toHaveBeenCalledTimes(2); + }); + + releaseSlowHook?.(); + const result = await executionPromise; + + expect(mockFs.writeFile).not.toHaveBeenCalled(); + expect(result.llmContent).toContain( + 'Todo creation blocked: Second todo denied', + ); + }); + + it('should report success when postWrite hooks fail after persistence', async () => { + const postWriteError = new Error('Hook timeout'); + const validationAllow: AggregatedHookResult = { + success: true, + allOutputs: [new DefaultHookOutput({ decision: 'allow' })], + errors: [], + totalDuration: 10, + finalOutput: new DefaultHookOutput({ decision: 'allow' }), + }; + + const mockHookSystem = { + fireTodoCreatedEvent: vi + .fn() + .mockResolvedValueOnce(validationAllow) + .mockRejectedValueOnce(postWriteError), + fireTodoCompletedEvent: vi.fn(), + }; + mockConfig = { + getSessionId: () => 'test-session-123', + getHookSystem: () => mockHookSystem, + } as unknown as Config; + tool = new TodoWriteTool(mockConfig); + + const params: TodoWriteParams = { + todos: [{ id: '1', content: 'Task 1', status: 'pending' }], + }; + + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); + mockFs.mkdir.mockResolvedValue(undefined); + mockFs.writeFile.mockResolvedValue(undefined); + + const invocation = tool.build(params); + const result = await invocation.execute(mockAbortSignal); + + expect(mockFs.writeFile).toHaveBeenCalled(); + expect(result.llmContent).toContain( + 'Todos have been modified successfully', + ); + expect(result.llmContent).toContain( + 'Todos were persisted successfully, but post-write hooks failed with error: Hook timeout.', + ); + expect(result.returnDisplay).toMatchObject({ + type: 'todo_list', + todos: params.todos, + }); + }); + + it('should run postWrite hooks concurrently after persistence', async () => { + let postWriteReleaseCount = 0; + const postWriteStarted: string[] = []; + const validationAllow: AggregatedHookResult = { + success: true, + allOutputs: [new DefaultHookOutput({ decision: 'allow' })], + errors: [], + totalDuration: 10, + finalOutput: new DefaultHookOutput({ decision: 'allow' }), + }; + + const mockHookSystem = { + fireTodoCreatedEvent: vi + .fn() + .mockImplementation((id, _content, _status, _allTodos, phase) => { + if (phase === HookPhase.Validation) { + return Promise.resolve(validationAllow); + } + + postWriteStarted.push(id as string); + return new Promise((resolve) => { + setTimeout(() => { + postWriteReleaseCount += 1; + resolve({ + success: true, + allOutputs: [new DefaultHookOutput({ decision: 'allow' })], + errors: [], + totalDuration: 10, + finalOutput: new DefaultHookOutput({ decision: 'allow' }), + }); + }, 0); + }); + }), + fireTodoCompletedEvent: vi.fn(), + }; + mockConfig = { + getSessionId: () => 'test-session-123', + getHookSystem: () => mockHookSystem, + } as unknown as Config; + tool = new TodoWriteTool(mockConfig); + + const params: TodoWriteParams = { + todos: [ + { id: '1', content: 'Task 1', status: 'pending' }, + { id: '2', content: 'Task 2', status: 'pending' }, + ], + }; + + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); + mockFs.mkdir.mockResolvedValue(undefined); + mockFs.writeFile.mockResolvedValue(undefined); + + const invocation = tool.build(params); + const result = await invocation.execute(mockAbortSignal); + + expect(mockFs.writeFile).toHaveBeenCalled(); + expect(postWriteStarted).toEqual(['1', '2']); + expect(postWriteReleaseCount).toBe(2); + expect(result.llmContent).toContain( + 'Todos have been modified successfully', + ); + }); }); describe('tool properties', () => { @@ -314,6 +692,7 @@ describe('TodoWriteTool – runtime output directory', () => { beforeEach(() => { mockConfig = { getSessionId: () => 'runtime-session', + getHookSystem: () => undefined, } as Config; tool = new TodoWriteTool(mockConfig); mockAbortSignal = new AbortController().signal; @@ -340,7 +719,10 @@ describe('TodoWriteTool – runtime output directory', () => { todos: [{ id: '1', content: 'Task 1', status: 'pending' }], }; - mockFs.readFile.mockRejectedValue({ code: 'ENOENT' }); + // Use proper Error object for ENOENT + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); mockFs.mkdir.mockResolvedValue(undefined); mockFs.writeFile.mockResolvedValue(undefined); @@ -361,7 +743,10 @@ describe('TodoWriteTool – runtime output directory', () => { todos: [{ id: '1', content: 'Task 1', status: 'pending' }], }; - mockFs.readFile.mockRejectedValue({ code: 'ENOENT' }); + // Use proper Error object for ENOENT + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); mockFs.mkdir.mockResolvedValue(undefined); mockFs.writeFile.mockResolvedValue(undefined); @@ -377,7 +762,10 @@ describe('TodoWriteTool – runtime output directory', () => { todos: [{ id: '1', content: 'Task 1', status: 'pending' }], }; - mockFs.readFile.mockRejectedValue({ code: 'ENOENT' }); + // Use proper Error object for ENOENT + const enoentError = new Error('ENOENT') as Error & { code: string }; + enoentError.code = 'ENOENT'; + mockFs.readFile.mockRejectedValue(enoentError); mockFs.mkdir.mockResolvedValue(undefined); mockFs.writeFile.mockResolvedValue(undefined); diff --git a/packages/core/src/tools/todoWrite.ts b/packages/core/src/tools/todoWrite.ts index bd689316b..e9bdda433 100644 --- a/packages/core/src/tools/todoWrite.ts +++ b/packages/core/src/tools/todoWrite.ts @@ -15,15 +15,11 @@ import type { Config } from '../config/config.js'; import { Storage } from '../config/storage.js'; import { ToolDisplayNames, ToolNames } from './tool-names.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { detectTodoChanges, HookPhase, type TodoItem } from '../hooks/types.js'; +export type { TodoItem } from '../hooks/types.js'; const debugLogger = createDebugLogger('TODO_WRITE'); -export interface TodoItem { - id: string; - content: string; - status: 'pending' | 'in_progress' | 'completed'; -} - export interface TodoWriteParams { todos: TodoItem[]; modified_by_user?: boolean; @@ -291,6 +287,20 @@ async function writeTodosToFile( await fs.writeFile(todoFilePath, JSON.stringify(data, null, 2), 'utf-8'); } +function createBlockedTodoResult( + message: string, + systemMessage: string, +): ToolResult { + return { + llmContent: `${message} + + +${systemMessage} +`, + returnDisplay: message, + }; +} + class TodoWriteToolInvocation extends BaseToolInvocation< TodoWriteParams, ToolResult @@ -315,6 +325,9 @@ class TodoWriteToolInvocation extends BaseToolInvocation< const sessionId = this.config.getSessionId(); try { + // 1. Read current todos (for change detection) + const oldTodos = await readTodosFromFile(sessionId); + let finalTodos: TodoItem[]; if (modified_by_user && modified_content !== undefined) { @@ -326,17 +339,140 @@ class TodoWriteToolInvocation extends BaseToolInvocation< finalTodos = todos; } + // 2. Detect changes + const changes = detectTodoChanges(oldTodos, finalTodos); + const oldTodosMap = new Map(oldTodos.map((t) => [t.id, t])); + + // 3. VALIDATION PHASE: Execute all hooks with Validation phase + // Hooks should only check and return block/approve decisions, no side effects + const hookSystem = this.config.getHookSystem(); + + // Validate TodoCreated hooks + if (hookSystem && changes.created.length > 0) { + const createdResults = await Promise.all( + changes.created.map((todo) => + hookSystem.fireTodoCreatedEvent( + todo.id, + todo.content, + todo.status, + finalTodos, + HookPhase.Validation, + _signal, + ), + ), + ); + + const blockedCreatedResult = createdResults.find( + (result) => result.finalOutput?.decision === 'block', + ); + if (blockedCreatedResult?.finalOutput) { + const reason = + blockedCreatedResult.finalOutput.reason || + 'Hook blocked todo creation'; + return createBlockedTodoResult( + `Todo creation blocked: ${reason}`, + `Todo list was not modified because a TodoCreated hook blocked the operation: ${reason}`, + ); + } + } + + // Validate TodoCompleted hooks + if (hookSystem && changes.completed.length > 0) { + const completedResults = await Promise.all( + changes.completed.map((todo) => { + const oldTodo = oldTodosMap.get(todo.id); + const previousStatus = oldTodo?.status ?? 'pending'; + + return hookSystem.fireTodoCompletedEvent( + todo.id, + todo.content, + previousStatus as 'pending' | 'in_progress', + finalTodos, + HookPhase.Validation, + _signal, + ); + }), + ); + + const blockedCompletedResult = completedResults.find( + (result) => result.finalOutput?.decision === 'block', + ); + if (blockedCompletedResult?.finalOutput) { + const reason = + blockedCompletedResult.finalOutput.reason || + 'Hook blocked todo completion'; + return createBlockedTodoResult( + `Todo completion blocked: ${reason}`, + `Todo list was not modified because a TodoCompleted hook blocked the operation: ${reason}`, + ); + } + } + + // 4. Write new todos AFTER all validation passes await writeTodosToFile(finalTodos, sessionId); - // Create structured display object for rich UI rendering + // 5. POST-WRITE PHASE: Execute hooks for side effects (logging, HTTP sync, etc.) + // These hooks can now safely perform side effects knowing data is persisted + // We don't check for blocking here since validation already passed + let postWriteError: Error | undefined; + try { + if (hookSystem && changes.created.length > 0) { + await Promise.all( + changes.created.map((todo) => + hookSystem.fireTodoCreatedEvent( + todo.id, + todo.content, + todo.status, + finalTodos, + HookPhase.PostWrite, + _signal, + ), + ), + ); + } + + if (hookSystem && changes.completed.length > 0) { + await Promise.all( + changes.completed.map((todo) => { + const oldTodo = oldTodosMap.get(todo.id); + const previousStatus = oldTodo?.status ?? 'pending'; + + return hookSystem.fireTodoCompletedEvent( + todo.id, + todo.content, + previousStatus as 'pending' | 'in_progress', + finalTodos, + HookPhase.PostWrite, + _signal, + ); + }), + ); + } + } catch (error) { + postWriteError = + error instanceof Error ? error : new Error(String(error)); + debugLogger.error( + `[TodoWriteTool] Post-write hooks failed after todos were persisted: ${postWriteError.message}`, + ); + } + + // 6. Create structured display object for rich UI rendering const todoResultDisplay = { type: 'todo_list' as const, todos: finalTodos, + changes, }; // Create plain string format with system reminder const todosJson = JSON.stringify(finalTodos); let llmContent: string; + const postWriteReminder = postWriteError + ? ` + + +Todos were persisted successfully, but post-write hooks failed with error: ${postWriteError.message}. Do not tell the user the write failed; only handle any follow-up hook issues if needed. +` + : ''; if (finalTodos.length === 0) { // Special message for empty todos @@ -344,16 +480,16 @@ class TodoWriteToolInvocation extends BaseToolInvocation< Your todo list is now empty. DO NOT mention this explicitly to the user. You have no pending tasks in your todo list. -`; +${postWriteReminder}`; } else { // Normal message for todos with items llmContent = `Todos have been modified successfully. Ensure that you continue to use the todo list to track your progress. Please proceed with the current tasks if applicable -Your todo list has changed. DO NOT mention this explicitly to the user. Here are the latest contents of your todo list: +Your todo list has changed. DO NOT mention this explicitly to the user. Here are the latest contents of your todo list: ${todosJson}. Continue on with the tasks at hand if applicable. -`; +${postWriteReminder}`; } return {