Skip to content

Add PermissionRequest hook for tool approvals / 新增工具审批 PermissionRequest Hook#4319

Open
SivanCola wants to merge 1 commit into
esengine:main-v2from
SivanCola:feature/approval-flow-fixes
Open

Add PermissionRequest hook for tool approvals / 新增工具审批 PermissionRequest Hook#4319
SivanCola wants to merge 1 commit into
esengine:main-v2from
SivanCola:feature/approval-flow-fixes

Conversation

@SivanCola

@SivanCola SivanCola commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #3661.
Related: #3855.

This PR keeps the scope limited to the PermissionRequest hook from #3855. The separate move_file work remains in #4316 and is intentionally outside this PR.

Authorship note

@AresNing is the primary author of the PermissionRequest hook implementation originally proposed in #3855. @SivanCola contributed as a collaborator by rebasing it onto the current main-v2, resolving integration conflicts, adding explicit regression coverage for prompt-triggered and non-triggering paths, and verifying the scoped PermissionRequest state.

What changed

  • Add PermissionRequest to the hook event list, default timeout handling, tool-name matching, hook payload, runner, CLI hook view, and example config.
  • Fire PermissionRequest only when a real tool approval prompt is emitted; policy allow, session grants, auto/yolo approval, and plan approval do not trigger it.
  • Add regression coverage for hook matching, payload delivery, prompt-only triggering, non-triggering paths, and CLI hook rendering.

Validation

  • go test ./internal/boot ./internal/cli ./internal/control ./internal/hook
  • go test ./...
  • git diff --check origin/main-v2..HEAD

@github-actions github-actions Bot added desktop Wails desktop app (desktop/**) tui Terminal UI / CLI (internal/cli, internal/control) skills Skill system (internal/skill, internal/tool) agent Core agent loop (internal/agent, internal/control) config Configuration & setup (internal/config) v2 Go rewrite (1.x) — main-v2 branch, active development labels Jun 13, 2026
@SivanCola SivanCola force-pushed the feature/approval-flow-fixes branch from 3362c45 to e542da6 Compare June 13, 2026 18:01
@SivanCola SivanCola changed the title Improve approval flows with PermissionRequest and move_file / 改进审批流程并新增 move_file Add PermissionRequest hook for tool approvals / 新增工具审批 PermissionRequest Hook Jun 13, 2026
@SivanCola SivanCola marked this pull request as ready for review June 13, 2026 18:04
@SivanCola SivanCola requested a review from esengine as a code owner June 13, 2026 18:04
@SivanCola SivanCola enabled auto-merge June 13, 2026 18:09
@SivanCola

Copy link
Copy Markdown
Collaborator Author

@codex review

@SivanCola

Copy link
Copy Markdown
Collaborator Author

approve

Co-authored-by: SivanCola <32437197+SivanCola@users.noreply.github.com>
@SivanCola SivanCola force-pushed the feature/approval-flow-fixes branch from e542da6 to fcf37fc Compare June 13, 2026 18:14

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e542da6388

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


c.sink.Emit(event.Event{Kind: event.ApprovalRequest, Approval: event.Approval{ID: id, Tool: tool, Subject: subject}})
if tool != planApprovalTool {
go c.hooks.PermissionRequest(ctx, tool, subject, args)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Redact memory approval payloads before hooks

When the pending approval is for remember/forget, gateApprover.Approve reaches this line with the original JSON args and a memory-derived subject; remember args include the private body field, while the memory approval contract says external notification hooks receive only the tool name (docs/SESSION_MEMORY_RETRIEVAL.md). Because this guard only excludes plan approvals, a user/project PermissionRequest hook configured for notifications can receive private memory content before the user approves it; please skip these fresh memory approvals here or pass a redacted payload.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Core agent loop (internal/agent, internal/control) config Configuration & setup (internal/config) desktop Wails desktop app (desktop/**) skills Skill system (internal/skill, internal/tool) tui Terminal UI / CLI (internal/cli, internal/control) v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(hooks): add PermissionRequest hook event for permission approval notifications

2 participants