feat(notifications): name the session a notification is about - #2
Closed
Jonathan-Asher wants to merge 1 commit into
Closed
Jonathan-Asher wants to merge 1 commit into
Jonathan-Asher wants to merge 1 commit into
Conversation
An OS notification was titled with the window's active folder, not the
folder of the session that raised it, and its body named only the agent.
With several sessions running there was no telling which one had
finished, failed, asked a question or was waiting on a permission, and
the folder in the title could be the wrong one.
Every notification raised from an ACP event (turn finished or failed,
session error, permission request, question, background task settled)
now resolves its session from the connection's context key, which is its
tab id. The tab leads to the persisted conversation: its title becomes
the notification title and its own folder leads the body
("<folder> · <message>"). A draft with no conversation row yet uses its
tab label. A context key no tab owns (a canvas card, a delegated
sub-agent's connection) keeps the previous "<folder> - Codeg" title.
With "hide notification contents" on, the notification reads as before,
"<folder> - Codeg" over the message alone, but names the session's own
folder: a session title is the user's own words, which that setting
exists to keep out of the notification centre. NotifyPayload gains an
optional redactedTitle for this, applied like redactedBody.
Owner
Author
|
Opened upstream as xintaofei#834. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OS notifications raised by agent sessions (turn finished or failed, session error, permission request, question, background task settled) were titled
<folder> - Codegwith the window's active folder, not the folder of the session that raised them, and the body named only the agent. With several sessions running there was no way to tell which one needed you, and the folder in the title could be the wrong one.Each of these notifications now names its session: the session's title as the notification title, and the session's own folder at the start of the body.
Behavior
<folder> · <message>, where the folder is the session's own (its alias when one is set). A chat-mode conversation names its hidden "Chat" folder; a chat draft that has no folder yet names none rather than the active one.<folder> - Codegover the message alone, but names the session's folder. A session title is the user's own words (often the first line of their prompt), which that setting exists to keep out of the notification centre.<active folder> - Codegtitle.Implementation:
src/lib/notification-session.ts(new):sessionNotificationPayload(contextKey, activeFolderName, { body, redactedBody }). The connection's context key is its tab id; from the tab it finds the persisted conversation (through the runtime session's row id for a draft whose first send has not bound the tab yet) and the folder inallFolders.src/lib/desktop-notification.ts:NotifyPayloadgains an optionalredactedTitle, used when contents are hidden, the same way asredactedBody.src/contexts/acp-connections-context.tsx: the sixnotifyDesktopcall sites wrap their existing payloads insessionNotificationPayload. What they notify and when (failure-aware turn completion,acpErrorNotifiesDesktop,quiet) is unchanged.Verification
src/lib/notification-session.test.ts(new, 7 tests): the session title and the session's own folder win over a different active folder; folder alias and reference-link folding; draft tab label; a draft reached through its runtime session; a chat-mode conversation's hidden folder; a folderless chat draft names no folder; an unknown context key keeps the old title.src/lib/desktop-notification.test.ts: the redacted title replaces the title only when contents are hidden.pnpm lint .,pnpm test,pnpm build) and every Rust desktop/server cell on Ubuntu, macOS and Windows. There are no Rust changes.