fix: capture ExitPlanMode and restore MultiEdit/NotebookEdit in Claude prompt hook#137
Open
joshazmy wants to merge 1 commit into
Open
fix: capture ExitPlanMode and restore MultiEdit/NotebookEdit in Claude prompt hook#137joshazmy wants to merge 1 commit into
joshazmy wants to merge 1 commit into
Conversation
…Claude prompt hook The Telegram channel never showed agent prompts (selection questions, permission approvals) because the PreToolUse hook matcher in settings-hook.json only covered Bash|Edit|Write|AskUserQuestion. Any tool call not matched by this regex never fires the hook, so it's never written to the agent-request store and never reaches the JS dispatch/formatting code (formatPromptMessage in channel-runner.ts, which already handles every tool name generically) — the drop happens entirely upstream, in Claude Code's own hook config. Two concrete gaps in that one matcher: - ExitPlanMode was never included, even in the original design doc — Plan Mode's approval prompt is one of the most common permission-approval prompts in normal usage. - MultiEdit|NotebookEdit were silently removed by an unexplained "chore" commit (61a6633), uncaught because the only existing test exercised a synthetic fixture instead of the real shipped asset. Fix: broaden the matcher to Bash|Edit|Write|MultiEdit|NotebookEdit|ExitPlanMode|AskUserQuestion. Add a regression test that reads the real settings-hook.json asset (not the synthetic fixture) so the matcher can't silently regress again; confirmed it fails before this fix and passes after. Fixes codeaholicguy#110
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #110 — agent prompts (selection questions, permission approvals) were never showing up in the Telegram chat.
Root cause: the
PreToolUsehookmatcherregex inpackages/cli/assets/claude/settings-hook.jsononly coveredBash|Edit|Write|AskUserQuestion. Any Claude Code tool call not matched by this regex never fires the hook, so it's never written to the agent-request store and never reaches the JS dispatch/formatting code (formatPromptMessageinchannel-runner.ts, which already handles every tool name generically once it receives one). The drop happens entirely upstream, in the hook config itself — not in any routing/dispatch logic.Two concrete gaps in that one matcher:
ExitPlanModewas never included, even in the original design doc — Plan Mode's approval prompt is one of the most common permission-approval prompts in normal Claude Code usage, and is very likely what the issue is describing.MultiEdit|NotebookEditwere silently dropped by an unexplained "chore" commit (61a6633), uncaught because the only existing test exercised a synthetic fixture rather than the real shipped asset.Fix: broaden the matcher to
Bash|Edit|Write|MultiEdit|NotebookEdit|ExitPlanMode|AskUserQuestion.A regression test was added that reads the real shipped
settings-hook.jsonasset (not the synthetic fixture used elsewhere in the same test file), so this matcher can't silently regress again the way it did before.Test plan
AssertionError: expected [...] to include 'MultiEdit')npx nx test cli— 872/872 passing after the fixnpx nx lint cli— 0 errors (5 pre-existing warnings, unrelated files)npm run test(via pre-commit hook) — all 6 projects green (memory 110/110, channel-connector 69/69, memory-dashboard 22/22, task-manager 112/112, agent-manager 479/479, cli 872/872)