Skip to content

fix(agent-hooks): detect Claude permission prompts#2216

Open
janburzinski wants to merge 2 commits into
mainfrom
emdash/claude-hook-uqb98
Open

fix(agent-hooks): detect Claude permission prompts#2216
janburzinski wants to merge 2 commits into
mainfrom
emdash/claude-hook-uqb98

Conversation

@janburzinski
Copy link
Copy Markdown
Collaborator

summary

  • detect claude permissions prompt
  • add claude pretooluse start hook

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 24, 2026

Greptile Summary

This PR teaches emdash to detect when Claude Code surfaces a permission prompt by adding a PreToolUse hook that fires a start event, and by classifying Claude Notification payloads as permission_prompt vs idle_prompt based on the message text. The conversation store is updated so only an explicit start signal (i.e., Claude actually resuming tool execution) can clear the permission-prompt awaiting state.

  • payload-normalizer.ts (extracted from event-enricher.ts): classifies Claude notification events using a /\\bpermission\\b/i regex heuristic since Claude Code's Notification hook does not include an explicit notification_type field.
  • hook-config.ts: adds a PreToolUse → 'start' entry to the Claude hook event map so that when Claude accepts a permission and resumes work, emdash receives a start signal.
  • conversation-manager.ts / ConversationStore: setWorking gains a clearPermissionPrompt option; the start event handler now passes { clearPermissionPrompt: true } to allow transitioning out of the permission-prompt state.

Confidence Score: 4/5

Safe to merge; the changes are additive, tests cover the new state transitions, and existing callers of setWorking are unaffected by the new option.

The permission-prompt detection relies on a single-word regex against Claude's notification message text, which may miss prompts phrased without the word "permission" or false-positive on unrelated messages that happen to include it. The rest of the implementation — the PreToolUse hook wiring, the clearPermissionPrompt guard, and the normalizer extraction — is clean and well-tested.

src/main/core/agent-hooks/payload-normalizer.ts — the /\bpermission\b/i heuristic is the most likely source of subtle misbehavior if Claude's prompt copy changes.

Important Files Changed

Filename Overview
src/main/core/agent-hooks/payload-normalizer.ts New module extracted from event-enricher.ts; adds heuristic permission-prompt detection via /\bpermission\b/i regex — fragile against Claude messages that don't use the exact word "permission".
src/main/core/agent-hooks/hook-config.ts Renames HOOK_EVENT_MAP to CLAUDE_HOOK_EVENT_MAP and adds a PreToolUse → 'start' entry so emdash receives a hook signal when Claude resumes work after a permission prompt.
src/renderer/features/tasks/conversations/conversation-manager.ts setWorking gains a clearPermissionPrompt option; the start event handler now passes it, enabling the PreToolUse hook to transition conversations out of permission_prompt state. Pre-existing callers (markConversationWorking, createConversation) still use the no-arg form and remain unable to override permission-prompt state.
src/main/core/agent-hooks/event-enricher.ts Small refactor to import normalizePayload and forward raw.type to it; no logic changes beyond what is in the new normalizer.
src/main/core/agent-hooks/payload-normalizer.test.ts New test file covering permission/idle heuristic, explicit notification_type passthrough, Codex idle_prompt, and start event passthrough — good coverage of the normalizer's branches.
src/renderer/features/tasks/conversations/conversation-store.test.ts New test verifying the clearPermissionPrompt flag guards the awaiting-input → working transition correctly.
src/main/core/agent-hooks/hook-config.test.ts Adds two tests: one verifying the PreToolUse hook is written with 'start' event type, and one confirming user-defined PreToolUse hooks are preserved when Emdash-managed entries are replaced.

Sequence Diagram

sequenceDiagram
    participant Claude as Claude Code
    participant Hook as Hook Server
    participant Normalizer as payload-normalizer
    participant Store as ConversationStore

    Claude->>Hook: Notification hook (message: "...permission...")
    Hook->>Normalizer: normalizePayload('claude', 'notification', body)
    Normalizer-->>Hook: "{ notificationType: 'permission_prompt' }"
    Hook->>Store: setAwaitingInput('permission_prompt')
    Note over Store: status = 'awaiting-input'<br/>lastNotificationType = 'permission_prompt'

    Note over Claude: User grants permission in Claude UI

    Claude->>Hook: PreToolUse hook
    Hook->>Normalizer: normalizePayload('claude', 'start', body)
    Normalizer-->>Hook: "{ notificationType: undefined }"
    Hook->>Store: "setWorking({ clearPermissionPrompt: true })"
    Note over Store: status = 'working'<br/>lastNotificationType = null

    Claude->>Hook: Stop hook
    Hook->>Store: setStatus('completed')
Loading

Comments Outside Diff (1)

  1. src/renderer/features/tasks/conversations/conversation-manager.ts, line 196-202 (link)

    P2 markConversationWorking silently no-ops for permission-prompt conversations

    markConversationWorking calls store.setWorking() without clearPermissionPrompt: true. With the guard introduced in this PR, if a conversation is in awaiting-input with lastNotificationType === 'permission_prompt', this call returns early and the conversation stays visually stuck in the awaiting-input state. Any external caller (e.g., createConversation with an initialPrompt on line 182) that legitimately wants to force a transition to working would be silently ignored. This was already true before the PR but the new clearPermissionPrompt option now makes it clearly addressable.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/renderer/features/tasks/conversations/conversation-manager.ts
    Line: 196-202
    
    Comment:
    **`markConversationWorking` silently no-ops for permission-prompt conversations**
    
    `markConversationWorking` calls `store.setWorking()` without `clearPermissionPrompt: true`. With the guard introduced in this PR, if a conversation is in `awaiting-input` with `lastNotificationType === 'permission_prompt'`, this call returns early and the conversation stays visually stuck in the awaiting-input state. Any external caller (e.g., `createConversation` with an `initialPrompt` on line 182) that legitimately wants to force a transition to `working` would be silently ignored. This was already true before the PR but the new `clearPermissionPrompt` option now makes it clearly addressable.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/main/core/agent-hooks/payload-normalizer.ts:24-27
**Fragile word-boundary heuristic for permission detection**

The regex `/\bpermission\b/i` is a single-word gate, so any Claude notification whose text does not include the literal word "permission" will be silently classified as `idle_prompt` instead. Claude's UI copy can read "Allow Bash to run?" or "Do you want to grant Bash access?" — neither matches the pattern, and the conversation store will never enter `permission_prompt` state for those messages. The inverse is also possible: a Claude idle notification that happens to mention "permission" in passing would incorrectly lock the UI into awaiting-input with a permission indicator.

### Issue 2 of 2
src/renderer/features/tasks/conversations/conversation-manager.ts:196-202
**`markConversationWorking` silently no-ops for permission-prompt conversations**

`markConversationWorking` calls `store.setWorking()` without `clearPermissionPrompt: true`. With the guard introduced in this PR, if a conversation is in `awaiting-input` with `lastNotificationType === 'permission_prompt'`, this call returns early and the conversation stays visually stuck in the awaiting-input state. Any external caller (e.g., `createConversation` with an `initialPrompt` on line 182) that legitimately wants to force a transition to `working` would be silently ignored. This was already true before the PR but the new `clearPermissionPrompt` option now makes it clearly addressable.

Reviews (1): Last reviewed commit: "fix(agent-hooks): detect Claude permissi..." | Re-trigger Greptile

Comment thread src/main/core/agent-hooks/payload-normalizer.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant