-
Notifications
You must be signed in to change notification settings - Fork 107
Add RFD for session info update notification #288
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
ignatov
commented
Nov 28, 2025
- Adds session_info_update variant to SessionUpdate
- Enables real-time session metadata updates (title, _meta)
- Follows existing notification pattern (available_commands_update, current_mode_update)
- All fields optional for partial updates
- Must stay aligned with SessionInfo structure
|
my personal apologies, it's very AI generated |
8185756 to
e8d18af
Compare
- Adds session_info_update variant to SessionUpdate - Enables real-time session metadata updates (title, _meta) - Follows existing notification pattern (available_commands_update, current_mode_update) - All fields optional for partial updates - Must stay aligned with SessionInfo structure
a4acf7e to
bab1c4d
Compare
benbrandt
left a comment
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.
Overall, I like it, have a few questions, but will likely do a cleanup pass and merge regardless and we can update the RFD as we work on the draft implementation.
|
|
||
| 5. **Enhanced use cases**: | ||
| - Session templates that auto-set titles and tags | ||
| - Progress indicators via `_meta` |
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.
This one is huge. i see people trying to send random empty content updates for things like this, and this would help a lot 👍🏻
| - **Merge semantics**: New `_meta` fields are merged with existing ones | ||
| - To clear a specific field: `"_meta": { "fieldName": null }` | ||
| - To clear all custom metadata: `"_meta": null` |
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.
This is a new one that we might have to handle carefully
Right now _meta has always been a black box, so if we want this behavior, we may need to apply this as a general recommendation for things that are updated multiple times (like messages once we have message ids, or tool calls)
We could sketch this behavior out for these updates, add a SHOULD in the other spots and put it on the list for v2, where we may also want to provide more guidance like MCP does on the _meta field names
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.
okay, let's omit _meta for now, my aim is to add session_info update and involve it further
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 guess there are two options here:
- omit _meta
- decide how/if we want to handle merging _meta
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 actually would say we shouldn't omit _meta because we always offer it. I just realized we don't have _meta well-defined enough to maybe adequately require merging... though this is maybe possible
| 1. Client sends a prompt asking to rename session | ||
| 2. Agent updates its internal state | ||
| 3. Agent sends `session_info_update` notification | ||
| 4. Client receives and displays the update |
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 sure this would always work, I might remove this and just state that client-updates would be handled in a future RFD
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.
okay
|
|
||
| ### What if multiple updates are sent in quick succession? | ||
|
|
||
| Clients should apply updates incrementally in order. Each notification represents a delta, not a full replacement (except for fields explicitly set to `null`). |
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.
We don't currently have this unset behavior in tool calls (probably should, but would be a different topic)
Just pointing out we would differ here, but could plan to align these in the future
|
|
||
| ### Should `updatedAt` be automatically set by the agent? | ||
|
|
||
| Yes, typically the agent should update this timestamp when any session activity occurs, not just when metadata changes. However, including it in `session_info_update` allows agents to explicitly control it if needed. |
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.
It is an interesting question from the client point of view, where likely updatedAt would be updated fairly constantly.
However, the agent could decide when to update that timestamp as you say, and the next time the list is refreshed it would also be updated. And it is likely less important when the session is loaded like the title, as you can see when the last message was potentially.
So it isn't required that the agent sends an update whenever the agent would update this time, but they can if they feel like it's important to communicate eagerly
| --- | ||
|
|
||
| - Author(s): [@ignatov](https://github.com/ignatov) | ||
| - Champion: [@benbrandt](https://github.com/benbrandt) |
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.
Lucky you, core maintainer, you don't need a champion :D though I am happy to help