-
-
Notifications
You must be signed in to change notification settings - Fork 32
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.
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
base: main
Are you sure you want to change the base?
Conversation
Add agent-shell-tool-call-update-functions hook to enable extensions to react to tool call status changes. This is called after the tool call state is updated but before UI changes, allowing extensions to: - Follow files when tool calls become active (status: in_progress) - Track tool call lifecycle for logging/monitoring - Add custom behavior for specific tool call types The hook receives STATE and UPDATE, where UPDATE contains toolCallId, status, content, and locations from the ACP protocol.
Add agent-shell-get-state and agent-shell-get-client to provide read-only access to agent shell state and ACP client. These functions allow extensions to: - Query current agent session state - Access the ACP client for protocol operations - Safely read state without internal dependencies Extensions should treat the returned state as read-only.
Add agent-shell-tool-call-get and agent-shell-tool-call-put to allow extensions to manage tool call specific state. These functions enable extensions to: - Retrieve tool call metadata (rawInput, status, locations, etc.) - Store extension-specific data per tool call (timers, overlays, etc.) - Track tool call lifecycle without modifying core state structure This is essential for extensions that need to maintain state across multiple tool call updates, such as debouncing timers for follow-edits or tracking preview overlays for permission queueing.
Add agent-shell-resolve-path as a public API wrapper around agent-shell--resolve-path. This allows extensions to: - Apply configured path transformations consistently - Handle devcontainer path mapping transparently - Avoid depending on internal implementation details Extensions implementing follow-edits need this to correctly resolve file paths from ACP tool call locations before opening buffers.
Add agent-shell-permission-request-functions hook to allow extensions to intercept and handle permission requests before default processing. This hook uses run-hook-with-args-until-success, meaning: - If any hook function returns non-nil, default handling is skipped - Extensions take full ownership of showing UI and sending responses - Multiple extensions can coexist (first to return non-nil wins) This enables extensions to: - Implement custom permission queueing (one at a time) - Add preview overlays before showing permission dialogs - Batch permissions or add custom approval workflows - Track permission patterns for analytics Extensions using this hook must call agent-shell-send-permission-response to send the ACP response when ready.
Add agent-shell-send-permission-response to allow extensions to send permission responses and clean up UI at their own timing. This function: - Finds the tool-call-id associated with a permission request-id - Sends the ACP permission response with the chosen option - Cleans up the permission dialog from the UI - Returns non-nil on success This is essential for extensions implementing permission queueing, which need to defer responses until the user has reviewed permissions sequentially. Extensions intercept requests via the permission-request hook and later call this function when ready to respond.
Add agent-shell-delete-dialog-block to allow extensions to remove dialog blocks they have created in the agent shell buffer. This function wraps agent-shell--delete-dialog-block and provides: - Safe buffer-local state access - Proper error handling - Return value indicating success Extensions implementing permission queueing may need this to clean up custom dialog blocks they create for progress indicators or previews.
Add agent-shell-file-write-functions hook to enable extensions to react to file writes after they complete. This hook is called after the file is written and saved but before the ACP response is sent, allowing extensions to: - Highlight changed regions in the buffer (follow-edits) - Track file modifications for analytics - Trigger post-write actions (linting, formatting, etc.) The hook receives STATE, PATH, CONTENT, and TOOL-CALL-ID. The tool-call-id is found by searching tool calls for a rawInput with matching file_path, enabling extensions to access edit metadata (old_string, new_string) for precise highlighting.
…vents The agent-shell-tool-call-update-functions hook was only being called for tool_call_update events, missing the initial tool_call notification when operations begin. This prevented extensions from reacting to the in_progress status change. Additionally, save locations and rawInput data from tool_call events to enable extensions to access file paths and edit details.
Add new extension capabilities: - Add `agent-shell-permission-response-functions` hook that fires after any permission response is sent (accept/reject/always). This allows extensions to clean up their state (e.g., preview overlays) at the correct time, immediately after the user makes a decision. - Add `agent-shell-show-permission-diff-button` defcustom to control whether the View button is shown in permission dialogs. Extensions that provide their own inline diff preview can set this to nil, making the approach generic and extension-agnostic. The permission response hook is called with STATE, REQUEST-ID, TOOL-CALL-ID, OPTION-ID, and CANCELLED parameters, giving extensions complete context about the permission resolution.
| - toolCallId: string | ||
| - status: string (pending, in_progress, completed, failed) | ||
| - content: tool call content array | ||
| - locations: array of location objects (path, line) |
There was a problem hiding this comment.
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.
| :type 'hook | ||
| :group 'agent-shell) | ||
|
|
||
| (defcustom agent-shell-permission-request-functions nil |
There was a problem hiding this comment.
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.
| :type 'hook | ||
| :group 'agent-shell) | ||
|
|
||
| (defcustom agent-shell-file-write-functions nil |
There was a problem hiding this comment.
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
| :type 'hook | ||
| :group 'agent-shell) | ||
|
|
||
| (defcustom agent-shell-permission-response-functions nil |
There was a problem hiding this comment.
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
| :type 'boolean | ||
| :group 'agent-shell) | ||
|
|
||
| (defcustom agent-shell-show-permission-diff-button t |
There was a problem hiding this comment.
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.
| (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) |
There was a problem hiding this comment.
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
| (agent-shell--save-tool-call state tool-call-id data) | ||
| t)) | ||
|
|
||
| (defun agent-shell-resolve-path (path) |
There was a problem hiding this comment.
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")
| 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) |
There was a problem hiding this comment.
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)
| :tool-call-id tool-call-id) | ||
| t))))) | ||
|
|
||
| (defun agent-shell-delete-dialog-block (block-id &optional buffer) |
There was a problem hiding this comment.
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.
| (cons :content (map-elt update 'content))) | ||
| (cons :content (map-elt update 'content)) | ||
| (cons :locations (map-elt update 'locations)) | ||
| (cons :rawInput (map-elt update 'rawInput))) |
There was a problem hiding this comment.
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?
|
Thank a lot for the PR! It's looking great. This will enable richer extensions from other packages. I've left some comments primarily around making the integration points a little more flexible and easier for me to maintain without breaking ya. Also, this being the first non-trivial integration, we may need to iterate over things even after this PR is submitted as we address teething issues or figure out better ways to integrate. Thanks a lot for the effort and the PR! |
|
Nice. Can you post a gif/demo of automatic follow mode btw? |
|
There's a demo in this thread comment #68 (comment) |
Hey @xenodium 👋 as discussed here, I've taken a stab at exposing an API for the pieces I needed to implement the "follow edits" feature I'd built previously as a separate package. With the changes in this PR, I had enough hooks to implement a standalone extension package: https://github.com/cmacrae/agent-shell-follow-edits
Let me know what you think, if you're happy supporting this API, or something close to it that you're happier with, that'd be great :)
Checklist
M-x checkdocandM-x byte-compile-file.