Skip to content

feat(hooks): add on_request_permission hook for external notifications #246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ro0gr wants to merge 2 commits into
base: main
Choose a base branch
from
+123 −2
Open

feat(hooks): add on_request_permission hook for external notifications #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14eb996
feat(hooks): add on_request_permission hook for external notifications
ro0gr May 18, 2026
1e07af2
docs: add on_request_permission hook documentation
ro0gr May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,18 @@ integrating with other plugins.
vim.lsp.buf.format({ bufnr = data.bufnr, timeout_ms = 5000 })
end
end,

-- Called when the agent needs permission to execute a tool (e.g. shell command).
-- Fires for each pending permission request.
on_request_permission = function(data)
-- data.request: table - The ACP permission request object
-- data.request.toolCall: table - contains .kind, .title, etc.
-- data.session_id: string - The ACP session ID
-- data.tab_page_id: number - The Neovim tabpage ID
local tool = data.request.toolCall
local label = tool.title or tool.kind or "action"
vim.notify("Agent needs permission for: " .. label)
end,
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions doc/agentic.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,11 @@ Event hooks ~
-- data.tab_page_id: number
-- data.bufnr: number|nil (if file is loaded in a buffer)
end,
on_request_permission = function(data)
-- data.request: table (contains .toolCall)
-- data.session_id: string
-- data.tab_page_id: number
end,
},
}
<
Expand Down
8 changes: 8 additions & 0 deletions lua/agentic/config_default.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@
--- @field tab_page_id number The tabpage ID
--- @field bufnr? number Buffer number if the file is loaded in a buffer

--- Data passed to the on_request_permission hook
--- @class agentic.UserConfig.RequestPermissionData
--- @field request agentic.acp.RequestPermission The permission request object
--- @field session_id string The ACP session ID
--- @field tab_page_id number The tabpage ID

--- @class agentic.UserConfig.KeymapEntry
--- @field [1] string The key binding
--- @field mode string|string[] The mode(s) for this binding
Expand Down Expand Up @@ -200,6 +206,7 @@
--- @field on_response_complete? fun(data: agentic.UserConfig.ResponseCompleteData): nil
--- @field on_session_update? fun(data: agentic.UserConfig.SessionUpdateData): nil
--- @field on_file_edit? fun(data: agentic.UserConfig.FileEditData): nil
--- @field on_request_permission? fun(data: agentic.UserConfig.RequestPermissionData): nil

--- Control various behaviors and features of the plugin
--- @class agentic.UserConfig.Settings
Expand Down Expand Up @@ -555,6 +562,7 @@ local ConfigDefault = {
on_response_complete = nil,
on_session_update = nil,
on_file_edit = nil,
on_request_permission = nil,
},

headers = {},
Expand Down
10 changes: 8 additions & 2 deletions lua/agentic/session_manager.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ local FILE_MUTATING_KINDS = {
}

--- Safely invoke a user-configured hook
--- @param hook_name "on_create_session_response" | "on_prompt_submit" | "on_response_complete" | "on_session_update" | "on_file_edit"
--- @param data agentic.UserConfig.CreateSessionResponseData | agentic.UserConfig.PromptSubmitData | agentic.UserConfig.ResponseCompleteData | agentic.UserConfig.SessionUpdateData | agentic.UserConfig.FileEditData
--- @param hook_name "on_create_session_response" | "on_prompt_submit" | "on_response_complete" | "on_session_update" | "on_file_edit" | "on_request_permission"
--- @param data agentic.UserConfig.CreateSessionResponseData | agentic.UserConfig.PromptSubmitData | agentic.UserConfig.ResponseCompleteData | agentic.UserConfig.SessionUpdateData | agentic.UserConfig.FileEditData | agentic.UserConfig.RequestPermissionData
function P.invoke_hook(hook_name, data)
local hook = Config.hooks and Config.hooks[hook_name]

Expand Down Expand Up @@ -977,6 +977,12 @@ function SessionManager:_build_handlers()
end,

on_request_permission = function(request, callback)
P.invoke_hook("on_request_permission", {
request = request,
session_id = self.session_id,
tab_page_id = self.tab_page_id,
})
Comment on lines +980 to +984
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the request’s session ID in hook payload to avoid stale-session misreporting.

session_id = self.session_id can be incorrect if an older permission event arrives after a session switch. Use request.sessionId for payload accuracy.

Proposed fix 
             P.invoke_hook("on_request_permission", {
                 request = request,
-                session_id = self.session_id,
+                session_id = request.sessionId,
                 tab_page_id = self.tab_page_id,
             })
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lua/agentic/session_manager.lua` around lines 980 - 984, The hook payload
uses self.session_id which can be stale; update the P.invoke_hook call in the
on_request_permission flow so the session_id field comes from the incoming
request (use request.sessionId) instead of self.session_id, keeping tab_page_id
as is; ensure you reference request.sessionId in the payload when calling
P.invoke_hook("on_request_permission", {...}) to avoid misreporting after
session switches.

Copy link
Copy Markdown
Contributor Author

@ro0gr ro0gr May 20, 2026

Choose a reason for hiding this comment

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

I guess if the session is stale, there is no point to notify? And maybe the whole callback should be ignored(or even not invoked at all)?

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 20, 2026

Choose a reason for hiding this comment

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

@ro0gr — You're right, and that's a better approach. Just swapping the session ID in the payload would still show diffs, render inline permission buttons, and stop the status animation for an event that belongs to a dead session. All of that should be skipped.

The pattern the rest of this file uses (e.g., in _handle_model_change, _handle_thought_level_change) is to capture the session ID at closure-creation time and bail out early if it no longer matches. For _build_handlers that would look roughly like:

function SessionManager:_build_handlers()
    local session_id_at_build = self.session_id   -- capture now

    local handlers = {
        -- ...
        on_request_permission = function(request, callback)
            -- Drop stale permission events from a superseded session.
            -- Still call callback with a rejection so the agent doesn't stall.
            if self.session_id ~= session_id_at_build then
                callback("reject_once")
                return
            end

            P.invoke_hook("on_request_permission", {
                request = request,
                session_id = self.session_id,
                tab_page_id = self.tab_page_id,
            })

            -- ... rest of handler unchanged
        end,
    }
    return handlers
end

The key nuance is that silently dropping callback (not calling it at all) would leave the agent side waiting indefinitely for a response, so calling it with "reject_once" before returning ensures the protocol stays consistent. Would you like me to open a follow-up issue for this, or plan the full code change?

🧠 Learnings used 
Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 74
File: tests/helpers/spy.lua:21-61
Timestamp: 2026-01-11T12:49:03.529Z
Learning: Neovim uses Lua 5.1 (as of version 0.11.0). In this repository, avoid recommending Lua 5.2+ features (e.g., table.pack, table.unpack, goto, \z in string literals) when reviewing or writing Lua code. For tests/helpers/spy.lua and other Lua files, prefer Lua 5.1-compatible constructs; if newer features are necessary, ensure compatibility or document rationale and provide a shim.

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 98
File: lua/agentic/ui/window_decoration.lua:77-85
Timestamp: 2026-01-29T20:36:14.589Z
Learning: In lua/agentic/**/*.lua, when initializing per-tab at vim.t[tab_page_id], per-buffer at vim.b[bufnr], or per-window at vim.w[winid] from module constants, do not use vim.deepcopy(). Assigning the table directly creates an independent copy due to the serialized storage in vim.* tables, which is typically sufficient for per-instance state. Only use deepcopy if you truly need a shared, non-serializable reference behavior.

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 104
File: lua/agentic/session_manager.lua:245-254
Timestamp: 2026-02-03T21:12:22.183Z
Learning: In lua/agentic/session_manager.lua, within _handle_input_submit, ensure chat_history.title is set from input_text in these two scenarios: 1) when restoring a session on the first submit and _needs_history_send is true, update the title; 2) for a new session when title is empty, set the title. Use an elseif branch so the title is assigned only once per submit.

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 124
File: lua/agentic/acp/acp_client.lua:342-359
Timestamp: 2026-02-20T14:38:23.848Z
Learning: For Lua files (e.g., lua/agentic/acp/acp_client.lua and similar), prefer Selene as the static analysis/linting tool over Luacheck. Selene is better maintained. Ensure repository CI and editor tooling invoke Selene for linting/analysis and disable Luacheck where configured.

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 162
File: lua/agentic/ui/message_writer.lua:432-444
Timestamp: 2026-03-20T07:09:34.381Z
Learning: In this repository’s Lua code (e.g., files like lua/agentic/ui/message_writer.lua), follow the existing author style: prefer defining behavior as “private” methods on the module/table (e.g., MessageWriter:_build_header_line) rather than as module-level local functions, even if the method receiver (`self`) is unused. Do not recommend converting unused-`self` methods into module-level local functions in review feedback, since module-level locals require being defined before use (hoisting concern), while methods can appear anywhere in the file.

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 175
File: lua/agentic/utils/buf_helpers.lua:70-74
Timestamp: 2026-04-01T17:38:48.173Z
Learning: When reviewing Neovim Lua code that uses `vim.keymap.set`, ensure buffer-local mappings use the correct option key for the target Neovim version. For Neovim 0.12+, `opts.buf` is the correct buffer-local key; `opts.buffer` is deprecated and removed in Neovim 0.15. Use the conditional branching pattern instead of `opts.buffer` unconditionally:

```lua
if vim.fn.has("nvim-0.12") == 1 then
  opts.buf = bufnr
else
  opts.buffer = bufnr
end
```
Avoid suggesting or using `opts.buffer` by itself.

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 182
File: lua/agentic/utils/file_system.lua:123-124
Timestamp: 2026-04-06T12:02:59.060Z
Learning: When writing Neovim Lua (LuaLS-checked), prefer creating a new local variable to achieve type narrowing instead of narrowing via reassignment. LuaLS often will not narrow a variable’s type through reassignments (e.g., `local lines, err = fn(); lines = lines or {}` may keep `lines` as `string[]|nil`). Use a new local on the narrowed expression (e.g., `local result, err = fn(); local lines = result or {}`) so LuaLS infers the non-nil type (e.g., `string[]`). Apply the “new-local” pattern whenever you need LuaLS to infer a narrowed/non-nil type.

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 193
File: lua/agentic/ui/chat_widget.lua:289-295
Timestamp: 2026-04-14T18:10:13.081Z
Learning: In this Neovim Lua codebase (agentic.nvim), it’s expected that buffers created or looked up via `vim.fn.bufadd(filepath)` or `vim.fn.bufnr(filepath)` won’t incur disk I/O or read file contents until that buffer is actually displayed in a window. Therefore, during code review you should not flag or warn about intermediate buffer assignments within the same synchronous event-loop tick as a “redundant load,” I/O/performance issue, memory leak, or visual artifact. In particular, don’t suggest forwarding/altering the buffer just to avoid “redundant” intermediate buffer-loading—this is a non-issue here.

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 220
File: lua/agentic/ui/tool_call_fold.lua:37-43
Timestamp: 2026-05-03T16:04:10.818Z
Learning: When reviewing Lua code that calls `vim.api.nvim_win_text_height(winid, opts)`, expect the return value to be a table/dict (`vim.api.keyset.win_text_height_ret`), not a number. The table includes fields such as `all` (total screen lines occupied), `fill`, `end_row`, and `end_vcol`. Do not treat `type(result) == "table"` as incorrect if the code uses `result.all`. If you need to guard for unexpected return types, the correct check is `type(result) ~= "table"` (i.e., only treat non-table results as errors). Access the screen-line count via `result.all`. 

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 231
File: lua/agentic/ui/permission_manager.lua:476-488
Timestamp: 2026-05-13T17:17:44.714Z
Learning: In this codebase, all keymap deletions must use `BufHelpers.keymap_del` instead of calling `vim.keymap.del` directly. Review `lua/agentic/**/*.lua` for any direct `vim.keymap.del(...)` usage and replace it with `require('...buf_helpers').keymap_del` (from `lua/agentic/utils/buf_helpers.lua`). `BufHelpers.keymap_del` must apply Neovim version-aware option handling: pass `{ buf = bufnr }` on Neovim 0.12.1+, and `{ buffer = bufnr }` on earlier versions. Treat any direct `vim.keymap.del` calls that include `{ buffer = ... }` as a bug.

Learnt from: carlos-algms
Repo: carlos-algms/agentic.nvim PR: 231
File: lua/agentic/ui/permission_manager.lua:129-135
Timestamp: 2026-05-13T19:45:47.457Z
Learning: In agentic.nvim, for MessageWriter’s `_capture_scroll`/`_apply_scroll` scroll-guard pair, only require the guard around buffer mutations that change the buffer’s total line count. Insertions/removals (which change `total_lines`) should trigger the gating condition; row-level text rewrites that keep line count constant (e.g., repainting a status row/button label width on focus toggle) should not. During reviews, do not flag missing `_capture_scroll`/`_apply_scroll` for row writes whose edits do not grow/shrink the buffer (and therefore do not affect `total_lines - cursor_line`).


self.status_animation:stop()

local function wrapped_callback(option_id)
Expand Down
90 changes: 90 additions & 0 deletions lua/agentic/session_manager.test.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1552,4 +1552,94 @@ describe("agentic.SessionManager", function()
end
)
end)

describe("_build_handlers: on_request_permission", function()
local Config = require("agentic.config")
--- @type TestStub
local schedule_stub
--- @type TestSpy
local hook_spy
--- @type agentic.SessionManager
local session

before_each(function()
schedule_stub = spy.stub(vim, "schedule")
schedule_stub:invokes(function(fn)
fn()
end)
hook_spy = spy.new(function() end)
Config.hooks = Config.hooks or {}
Config.hooks.on_request_permission = nil

session = {
session_id = "test-session-123",
tab_page_id = 1,
status_animation = {
stop = function() end,
start = function() end,
},
permission_manager = {
has_pending = function()
return false
end,
add_request = function() end,
},
_show_diff_in_buffer = function() end,
_clear_diff_in_buffer = function() end,
_build_handlers = SessionManager._build_handlers,
} --[[@as agentic.SessionManager]]
end)

after_each(function()
schedule_stub:revert()
Config.hooks.on_request_permission = nil
end)

it("invokes on_request_permission hook with correct payload", function()
Config.hooks.on_request_permission = function(data)
hook_spy(data)
end

local handlers = session:_build_handlers()
local mock_request = {
sessionId = "test-session-123",
toolCall = {
toolCallId = "tool-1",
kind = "edit",
title = "Edit file",
},
options = {
{
optionId = "allow_once",
name = "Allow Once",
kind = "allow_once",
},
},
}
local mock_callback = function() end

handlers.on_request_permission(mock_request, mock_callback)

assert.spy(hook_spy).was.called(1)
local data = hook_spy.calls[1][1]
assert.equal("test-session-123", data.session_id)
assert.equal(1, data.tab_page_id)
assert.equal(mock_request, data.request)
end)

it("does not fail when hook is not configured", function()
Config.hooks.on_request_permission = nil

local handlers = session:_build_handlers()
local mock_request = {
sessionId = "test-session-123",
toolCall = { toolCallId = "tool-1", kind = "edit" },
options = {},
}
local mock_callback = function() end

-- Should not throw an error
handlers.on_request_permission(mock_request, mock_callback)
end)
end)
end)
Loading