Skip to content

Conversation

@guillaumeboehm
Copy link
Contributor

I felt like having system notifications on certain opencode events would be really nice, so I implemented those two hooks available in the user config.

I had to add a "user_message_count" to the state to track the user initiated requests rather than all requests otherwise the hook would trigger even if the user didn't send anything.
I'm not sure that the mechanism to determine if a job is user initiated is correct, it works for me but there might be edge cases that I'm not aware of.

This is meant to track the number of user initiated messages, the
job_count available currently only counts the total job counts
Copy link
Owner

@sudo-tee sudo-tee left a comment

Choose a reason for hiding this comment

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

This is actually a really great idea.

I left a little comment about the user_message_count. I think it would be better to set it in the send_message
we already have a promise running for the duration of the message.
state.user_message_count +1 before the promise

and

state.user_message_count -1 in the and_then callback.
Image

@sudo-tee
Copy link
Owner

Can you add some test cases in the hooks_spec.lua file for the new hooks ?

Also can you add them to the README file with a short description / example. I just realised that the doc was missing for the other hooks and added them to the docs

@guillaumeboehm
Copy link
Contributor Author

Alright, after thinking that adding the session info as a hook param would be nice to access the session title etc, I realized that the hook would only be called for the currently active session.
But the idea was to be notified for longer sessions, so it's better to be notified even after switching session.
I'll let you see if you think that it's looking good enough and I'll do the same thing for the permission hook.

I had to change the two subscribed function in core into "private-ish" functions in order to use them in the unit tests, I'm not sure if that's fine or not.

@sudo-tee
Copy link
Owner

Thanks for the fixes.

It looks fine to me

@sudo-tee sudo-tee merged commit d2447ed into sudo-tee:main Nov 24, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants