Skip to content

feat: introduce various hooks to support extension packages #120

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
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

feat: introduce various hooks to support extension packages #120

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
05ddc27
feat: add tool-call-update hook for extension support
cmacrae Nov 5, 2025
7c79d7a
feat: add basic state access functions for extensions
cmacrae Nov 5, 2025
d8ab252
feat: add tool-call state management for extensions
cmacrae Nov 5, 2025
95db57c
feat: add path resolution helper for extensions
cmacrae Nov 5, 2025
de9d8bd
feat: add permission-request hook for custom queueing
cmacrae Nov 5, 2025
a872b2a
feat: add permission response control for extensions
cmacrae Nov 5, 2025
e8de0b9
feat: add dialog-block cleanup control for extensions
cmacrae Nov 5, 2025
089cd16
feat: add file-write hook for post-write effects
cmacrae Nov 5, 2025
f1de30c
fix(extensions): invoke tool-call-update hook for initial tool_call e…
cmacrae Nov 5, 2025
a127684
feat(extensions): add permission response hook and View button control
cmacrae Nov 5, 2025
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
227 changes: 202 additions & 25 deletions agent-shell.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,68 @@ See https://github.com/xenodium/agent-shell/issues/119"
:type 'boolean
:group 'agent-shell)

(defcustom agent-shell-tool-call-update-functions nil
"Abnormal hook run when a tool call is updated.
Each function is called with STATE and UPDATE alist, where UPDATE contains:
- toolCallId: string
- status: string (pending, in_progress, completed, failed)
- content: tool call content array
- locations: array of location objects (path, line)
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

Could we please make the functions receive a single parameter event alist? This can expose the internal :state and :tool-call.

This makes supporting new features easier (exposing more details to abnormal hook) while keeping backwards compatibility with existing clients much more manageable.


Functions should not modify STATE or UPDATE directly.

This hook is called after tool call state is updated but before
the dialog block is updated in the UI."
:type 'hook
:group 'agent-shell)

(defcustom agent-shell-permission-request-functions nil
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

Same here please. Let's expose an event alist param with :state and :request.

"Abnormal hook run when a permission request is received.
Each function is called with STATE and REQUEST alist.
If any function returns non-nil, default permission handling is skipped.
This allows extensions to implement custom permission queueing.

Functions receive:
- STATE: agent shell state
- REQUEST: full request alist from session/request_permission

Functions can return:
- nil: Continue with default handling
- non-nil: Skip default handling (extension handles it)"
:type 'hook
:group 'agent-shell)

(defcustom agent-shell-file-write-functions nil
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

Ditto alist param please with :state and :tool-call

"Abnormal hook run after a file is successfully written.
Each function is called with STATE, PATH, CONTENT, and TOOL-CALL-ID.

Functions receive:
- STATE: agent shell state
- PATH: absolute file path that was written
- CONTENT: full file content that was written
- TOOL-CALL-ID: tool call ID that triggered the write (may be nil)

This hook is called after the file is written and saved, but before
the ACP response is sent."
:type 'hook
:group 'agent-shell)

(defcustom agent-shell-permission-response-functions nil
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

And here too please ;) :state :tool-call :request-id :option-id :cancelled

"Abnormal hook run after a permission response is sent.
Each function is called with STATE, REQUEST-ID, TOOL-CALL-ID, OPTION-ID, and CANCELLED.

Functions receive:
- STATE: agent shell state
- REQUEST-ID: the permission request ID
- TOOL-CALL-ID: tool call ID that required permission
- OPTION-ID: the selected option ID (nil if cancelled)
- CANCELLED: non-nil if permission was cancelled/rejected

This hook is called after the ACP response is sent and dialog is cleaned up.
Extensions can use this to clean up their own state (e.g., preview overlays)."
:type 'hook
:group 'agent-shell)

(cl-defun agent-shell--make-acp-client (&key command
command-params
environment-variables
Expand Down Expand Up @@ -155,6 +217,14 @@ See `acp-make-initialize-request' for details."
:type 'boolean
:group 'agent-shell)

(defcustom agent-shell-show-permission-diff-button t
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

I'm not super sure what to call this just yet. How about something a little more generic like agent-shell-offer-diffing.

"Whether to show the View button in permission dialogs for file edits.
When non-nil, displays a button allowing users to view diffs in a separate
buffer. Extensions that provide their own inline diff preview can set this
to nil."
:type 'boolean
:group 'agent-shell)

(defcustom agent-shell-display-action
'(display-buffer-same-window)
"Display action for agent shell buffers.
Expand Down Expand Up @@ -385,6 +455,88 @@ Returns an empty string if no icon should be displayed."
(interactive)
(message "agent-shell v%s" agent-shell--version))

;;; Extension API

(defun agent-shell-get-state (&optional buffer)
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

If it's an internal function, I'm generally ok with with &optional, but being a public function now it makes me a little more nervous to introduce additional params.

Mind switching to cl-defun and &key buffer? You'll still be able to invoke as (agent-shell-get-state) or (agent-shell-get-state :buffer my-buffer).

"Get agent shell state from BUFFER (defaults to current buffer).
Returns nil if buffer is not an agent-shell buffer.
Note: The returned state should be treated as read-only by extensions."
(with-current-buffer (or buffer (current-buffer))
(when (and (boundp 'agent-shell--state)
(derived-mode-p 'agent-shell-mode))
agent-shell--state)))

(defun agent-shell-get-client (&optional buffer)
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

cl-defun and &key buffer too please.

"Get ACP client from BUFFER's agent shell state.
BUFFER defaults to current buffer.
Returns nil if buffer is not an agent-shell buffer."
(map-elt (agent-shell-get-state buffer) :client))

(defun agent-shell-tool-call-get (tool-call-id &optional buffer)
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

cl-defun &key tool-call-id buffer

"Get tool call data for TOOL-CALL-ID from BUFFER's agent shell state.
BUFFER defaults to current buffer.
Returns the tool call alist or nil if not found."
(when-let ((state (agent-shell-get-state buffer)))
(map-nested-elt state (list :tool-calls tool-call-id))))

(defun agent-shell-tool-call-put (tool-call-id data &optional buffer)
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

cl-defun &key tool-call-id merge buffer

(unless tool-call-id
(error "tool-call-id is required"))

and same for merge param

"Store tool call DATA for TOOL-CALL-ID in BUFFER's agent shell state.
BUFFER defaults to current buffer.
DATA should be an alist that will be merged with existing data.
Returns non-nil on success."
(when-let ((state (agent-shell-get-state buffer)))
(agent-shell--save-tool-call state tool-call-id data)
t))

(defun agent-shell-resolve-path (path)
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

(cl-defun agent-shell-resolve (&key path)

(unless path
...)

Client code will be fairly explict:

(agent-shell-resolve :path "some/path/here")

"Resolve PATH using configured path resolver.
This applies any path transformations configured via
`agent-shell-path-resolver-function'.

Extensions should use this instead of `agent-shell--resolve-path'
to ensure consistent path handling across the system."
(agent-shell--resolve-path path))

(defun agent-shell-send-permission-response (request-id option-id &optional buffer)
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

cl-defun agent-shell-send-permission-response (&key request-id option-id buffer)

"Send permission response for REQUEST-ID with OPTION-ID.
BUFFER defaults to current buffer.
Returns non-nil on success.

This function is intended for use by extensions implementing custom
permission queueing via `agent-shell-permission-request-functions'.
It sends the ACP response and cleans up the permission dialog UI.

Extensions that intercept permissions are responsible for calling this
function to send responses at the appropriate time."
(when-let ((state (agent-shell-get-state buffer))
(client (map-elt state :client)))
(with-current-buffer (or buffer (current-buffer))
;; Find the tool-call-id for this request
(when-let ((tool-call-entry
(seq-find (lambda (entry)
(equal (map-elt (cdr entry) :permission-request-id)
request-id))
(map-elt state :tool-calls))))
(let ((tool-call-id (car tool-call-entry)))
(agent-shell--send-permission-response
:client client
:request-id request-id
:option-id option-id
:state state
:tool-call-id tool-call-id)
t)))))

(defun agent-shell-delete-dialog-block (block-id &optional buffer)
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

cl-defun agent-shell-delete-dialog-block (&key block-id buffer)

ps. I've been considering renaming dialog in most places to section. Just a heads-up. If you're keen to adopt section now, that works for me. Otherwise I can ping you when I rename it or send a PR to your package.

"Delete dialog block BLOCK-ID from BUFFER.
BUFFER defaults to current buffer.
Returns non-nil on success.

This function is intended for use by extensions that create custom
dialog blocks and need to clean them up later."
(when-let ((state (agent-shell-get-state buffer)))
(agent-shell--delete-dialog-block :state state :block-id block-id)
t))

(defun agent-shell-interrupt (&optional force)
"Interrupt in-progress request and reject all pending permissions.
When FORCE is non-nil, skip confirmation prompt."
Expand Down Expand Up @@ -517,9 +669,13 @@ Flow:
(cons :kind (map-elt update 'kind))
(cons :command (map-nested-elt update '(rawInput command)))
(cons :description (map-nested-elt update '(rawInput description)))
(cons :content (map-elt update 'content)))
(cons :content (map-elt update 'content))
(cons :locations (map-elt update 'locations))
(cons :rawInput (map-elt update 'rawInput)))
Copy link
Owner

@xenodium xenodium Nov 6, 2025

Choose a reason for hiding this comment

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

Heads-up: This mixes internal representation with protocol representation (I've been slowly moving away from mixing in favor of internal). There are pros and cons to using internal over external. I've found that using internal requires mapping (a bit more work), but longer term makes is easier to find out where data came from (you can search the code base and find the relevant :foo symbols). When using protocol structure, things become much more implicit and you have to go digging into the protocol documentation if you can't find anything relevant in the codebase.

In any case, I'm ok for this to go in as is, with the caveat that in the future I may restructure some of these fields to use internal representations (I'd of course ping you or send a PR).

Another idea I've been considering is to have an :acp field in some of these internal representations which can include a bigger chunk of the verbatim protocol. Maybe you want to trial this idea?

(when-let ((diff (agent-shell--make-diff-info (map-elt update 'content))))
(list (cons :diff diff)))))
;; Run extension hooks after state update but before UI update
(run-hook-with-args 'agent-shell-tool-call-update-functions state update)
(let ((tool-call-labels (agent-shell-make-tool-call-label
state (map-elt update 'toolCallId))))
(agent-shell--update-dialog-block
Expand Down Expand Up @@ -595,6 +751,8 @@ Flow:
(cons :content (map-elt update 'content)))
(when-let ((diff (agent-shell--make-diff-info (map-elt update 'content))))
(list (cons :diff diff)))))
;; Run extension hooks after state update but before UI update
(run-hook-with-args 'agent-shell-tool-call-update-functions state update)
(let* ((diff (map-nested-elt state `(:tool-calls ,.toolCallId :diff)))
(output (concat
"\n\n"
Expand Down Expand Up @@ -686,28 +844,31 @@ Flow:
"Handle incoming request using SHELL, STATE, and REQUEST."
(let-alist request
(cond ((equal .method "session/request_permission")
(agent-shell--save-tool-call
state .params.toolCall.toolCallId
(append (list (cons :title .params.toolCall.title)
(cons :status .params.toolCall.status)
(cons :kind .params.toolCall.kind)
(cons :permission-request-id .id))
(when-let ((diff (agent-shell--make-diff-info .params.toolCall.content)))
(list (cons :diff diff)))))
(agent-shell--update-dialog-block
:state state
;; block-id must be the same as the one used
;; in agent-shell--delete-dialog-block param.
:block-id (format "permission-%s" .params.toolCall.toolCallId)
:body (with-current-buffer (map-elt state :buffer)
(agent-shell--make-tool-call-permission-text
:request request
:client (map-elt state :client)
:state state))
:expanded t
:navigation 'never)
(agent-shell-jump-to-latest-permission-button-row)
(map-put! state :last-entry-type "session/request_permission"))
;; Run extension hooks first - if any return non-nil, skip default handling
(unless (run-hook-with-args-until-success 'agent-shell-permission-request-functions
state request)
(agent-shell--save-tool-call
state .params.toolCall.toolCallId
(append (list (cons :title .params.toolCall.title)
(cons :status .params.toolCall.status)
(cons :kind .params.toolCall.kind)
(cons :permission-request-id .id))
(when-let ((diff (agent-shell--make-diff-info .params.toolCall.content)))
(list (cons :diff diff)))))
(agent-shell--update-dialog-block
:state state
;; block-id must be the same as the one used
;; in agent-shell--delete-dialog-block param.
:block-id (format "permission-%s" .params.toolCall.toolCallId)
:body (with-current-buffer (map-elt state :buffer)
(agent-shell--make-tool-call-permission-text
:request request
:client (map-elt state :client)
:state state))
:expanded t
:navigation 'never)
(agent-shell-jump-to-latest-permission-button-row)
(map-put! state :last-entry-type "session/request_permission")))
((equal .method "fs/read_text_file")
(agent-shell--on-fs-read-text-file-request
:state state
Expand Down Expand Up @@ -810,6 +971,19 @@ If the buffer's file has changed, prompt the user to reload it."
;; No open buffer, write to file directly.
(with-temp-file path
(insert content)))
;; Find tool-call-id for this write operation
(let ((tool-call-id
(car (seq-find (lambda (entry)
(let* ((tc-data (cdr entry))
(tc-raw-input (map-elt tc-data :rawInput))
(tc-path (and tc-raw-input
(map-elt tc-raw-input 'file_path))))
(and tc-path
(string= (agent-shell--resolve-path tc-path) path))))
(map-elt state :tool-calls)))))
;; Run extension hooks after write completes but before response
(run-hook-with-args 'agent-shell-file-write-functions
state path content tool-call-id))
(acp-send-response
:client (map-elt state :client)
:response (acp-make-fs-write-text-file-response
Expand Down Expand Up @@ -2433,7 +2607,7 @@ For example:
;; May as well interrupt so you can course-correct.
(agent-shell-interrupt t))))))
;; Add diff keybinding if diff info is available
(when diff
(when (and diff agent-shell-show-permission-diff-button)
(define-key map "v" (agent-shell--make-diff-viewing-function
:diff diff
:actions actions
Expand All @@ -2444,7 +2618,7 @@ For example:
;; Add interrupt keybinding
(define-key map (kbd "C-c C-c") #'agent-shell-interrupt)
map))
(diff-button (when diff
(diff-button (when (and diff agent-shell-show-permission-diff-button)
(agent-shell--make-permission-button
:text "View (v)"
:help "Press v to view diff"
Expand Down Expand Up @@ -2529,6 +2703,9 @@ MESSAGE-TEXT: Optional message to display after sending the response."
;; block-id must be the same as the one used as
;; agent-shell--update-dialog-block param by "session/request_permission".
(agent-shell--delete-dialog-block :state state :block-id (format "permission-%s" tool-call-id))
;; Run extension hooks after response sent and dialog cleaned up
(run-hook-with-args 'agent-shell-permission-response-functions
state request-id tool-call-id option-id cancelled)
(let ((updated-tool-calls (map-copy (map-elt state :tool-calls))))
(map-delete updated-tool-calls tool-call-id)
(map-put! state :tool-calls updated-tool-calls))
Expand Down