Advanced hooks #225
Advanced hooks #225
Conversation
Moves cache directory resolution and workspace hashing from `eca.db` to a new `eca.cache` namespace. Centralizes file path management and allows other components (hooks) to acess cache paths. Avoids circular dependencies on the database namespace.
zikajk
force-pushed
the
advanced-hooks
branch
2 times, most recently
from
e177621 to
af57b0a
Compare
|
@ericdallo I am not sure about the failing Windows test. Maybe you have an idea? |
| ["## Contexts" | ||
| "" | ||
| (contexts-str refined-contexts repo-map*)]) | ||
| (contexts-str refined-contexts repo-map* (get-in db [:chats chat-id :startup-context]))]) |
There was a problem hiding this comment.
I think this should go indeed to a refined-context, following others
| (logger/debug logger-tag "Hook JSON output must result in map"))) | ||
| (catch Exception e | ||
| (logger/debug logger-tag "Hook output is not valid JSON, treating as plain text" |
There was a problem hiding this comment.
I believe these should be at least a logger/warn?
There was a problem hiding this comment.
I am not sure. It is still valid to not return JSON. The question would be if this should be logged at all :-).
There was a problem hiding this comment.
yeah maybe the first one? I don't know too
There was a problem hiding this comment.
I think both are valid.
The first one will tell the user that his output is a valid json but not the one compatible with advanced hooks.
The second one might be useful to catch if the JSON is invalid but it will probably throw even with a string.
If i am not wrong, they will fire too often to have them on any other level than debug, no?
src/eca/features/chat.clj
Outdated
| (if (and @approved?* (not hook-rejected?)) | ||
| (when-not (#{:stopping :cleanup} (:status (get-tool-call-state @db* chat-id id))) | ||
| (assert-chat-not-stopped! chat-ctx) | ||
| (let [delayed-future (delay (future (let [result (f.tools/call-tool! name |
There was a problem hiding this comment.
A few enters + remove vertical alignment would make this way better to read
There was a problem hiding this comment.
I removed the vertical alignment, but where do you see newlines would help?
There was a problem hiding this comment.
I think one after delay and other after future maybe?
There was a problem hiding this comment.
Ah, i see, i did that :). Good.
ericdallo
left a comment
ericdallo
left a comment
There was a problem hiding this comment.
As a first review looks good! I will start test manually
The changes were all good and nice besides chat.clj which scared me a little bit the amount of unrelated changes TBH :)
Key changes: - New Hook Types: `sessionStart`, `sessionEnd`, `chatStart`, `chatEnd` for better lifecycle management. - Enhanced `preRequest`: Supports prompt rewriting, context injection, and request blocking. - Enhanced `preToolCall`: Allows argument modification, approval overrides (allow/ask/deny), and tool blocking via exit codes or JSON. - New `postToolCall`: Provides access to tool responses and errors, allowing for post-execution context injection. - JSON Protocol: Hooks now communicate via structured JSON on stdin/stdout for robust data exchange. - Integration: Updates chat loop and tool state machine to respect hook decisions.
- `docs/configuration.md`: Detailed reference for new hook types, JSON icnput/output schema, and execution options (timeout, runOnError). - `docs/examples.md`: Added practical examples for prompt rewriting, tool guarding, and context injection.
zikajk
force-pushed
the
advanced-hooks
branch
from
af57b0a to
ca6ad9d
Compare
zikajk
force-pushed
the
advanced-hooks
branch
2 times, most recently
from
922eb19 to
408ffd6
Compare
zikajk
force-pushed
the
advanced-hooks
branch
from
408ffd6 to
381e9c7
Compare
ericdallo
left a comment
ericdallo
left a comment
There was a problem hiding this comment.
Looks good to me! I didn't test all new hooks features, but the most important ones, feel free to merge
Implement advanced lifecycle hooks
Key changes:
New Hook Types:
sessionStart,sessionEnd,chatStart,chatEndfor better lifecycle management.Enhanced
preRequest: Supports prompt rewriting, context injection, and request blocking.Enhanced
preToolCall: Allows argument modification, approval overrides (allow/ask/deny), and tool blocking via exit codes or JSON.New
postToolCall: Provides access to tool responses and errors, allowing for post-execution context injection.JSON Protocol: Hooks now communicate via structured JSON on stdin/stdout for robust data exchange.
Integration: Updates chat loop and tool state machine to respect hook decisions.
I added a entry in changelog under unreleased section.