Add PermissionRequest hook for tool approvals / 新增工具审批 PermissionRequest Hook#4319
Add PermissionRequest hook for tool approvals / 新增工具审批 PermissionRequest Hook#4319SivanCola wants to merge 1 commit into
Conversation
3362c45 to
e542da6
Compare
|
@codex review |
|
approve |
Co-authored-by: SivanCola <32437197+SivanCola@users.noreply.github.com>
e542da6 to
fcf37fc
Compare
There was a problem hiding this comment.
💡 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) |
There was a problem hiding this comment.
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 👍 / 👎.
Summary
Fixes #3661.
Related: #3855.
This PR keeps the scope limited to the
PermissionRequesthook from #3855. The separatemove_filework remains in #4316 and is intentionally outside this PR.Authorship note
@AresNing is the primary author of the
PermissionRequesthook implementation originally proposed in #3855. @SivanCola contributed as a collaborator by rebasing it onto the currentmain-v2, resolving integration conflicts, adding explicit regression coverage for prompt-triggered and non-triggering paths, and verifying the scoped PermissionRequest state.What changed
PermissionRequestto the hook event list, default timeout handling, tool-name matching, hook payload, runner, CLI hook view, and example config.PermissionRequestonly when a real tool approval prompt is emitted; policy allow, session grants, auto/yolo approval, and plan approval do not trigger it.Validation
go test ./internal/boot ./internal/cli ./internal/control ./internal/hookgo test ./...git diff --check origin/main-v2..HEAD