feat(ui): restore task completion notification for desktop app#6424
Closed
blackgirlbytes wants to merge 3 commits intomainfrom
Closed
feat(ui): restore task completion notification for desktop app#6424blackgirlbytes wants to merge 3 commits intomainfrom
blackgirlbytes wants to merge 3 commits intomainfrom
Conversation
When Goose finishes a task, show a native OS notification if the window is not focused. This helps users who switch to other apps while waiting for Goose to complete. - Add notification trigger in BaseChat.tsx onStreamFinish callback - Only notify when document.hasFocus() returns false - Add test file with 5 tests covering notification behavior
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds native OS notifications when goose completes a task, but only if the application window is not focused. This allows users to be notified when they've switched to other applications while waiting for goose to finish.
Key Changes:
- Added notification trigger in BaseChat's
onStreamFinishcallback that checks window focus - Created test suite with focus state verification and notification content validation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ui/desktop/src/components/BaseChat.tsx | Implements task completion notification in onStreamFinish callback, checking document.hasFocus() before showing notification |
| ui/desktop/src/components/TaskCompletionNotification.test.tsx | New test file with tests for notification behavior based on window focus state |
- Move notification logic from inline callback to utils/taskCompletionNotification.ts - Update BaseChat.tsx to import and use the utility function - Move test file to utils/ and test the actual function (not duplicated logic) - Remove redundant tests, keep only 2 essential tests This addresses code review feedback: - Tests now import and test the actual function, not duplicated logic - Removed redundant tests that were already covered
Contributor
Author
|
blackgirlbytes
force-pushed
the
feat/task-completion-notification
branch
from
ce54844 to
e588219
Compare
blackgirlbytes
changed the title
angiejones
reviewed
Jan 9, 2026
| // Store original hasFocus | ||
| originalHasFocus = document.hasFocus; | ||
|
|
||
| // Mock window.electron.showNotification |
Collaborator
There was a problem hiding this comment.
Suggested change
| // Mock window.electron.showNotification |
angiejones
reviewed
Jan 9, 2026
| beforeEach(() => { | ||
| mockShowNotification = vi.fn(); | ||
|
|
||
| // Store original hasFocus |
Collaborator
There was a problem hiding this comment.
Suggested change
| // Store original hasFocus |
angiejones
reviewed
Jan 9, 2026
|
|
||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| // Restore original hasFocus |
Collaborator
There was a problem hiding this comment.
Suggested change
| // Restore original hasFocus |
angiejones
reviewed
Jan 9, 2026
| @@ -0,0 +1,8 @@ | |||
| export function notifyTaskCompletion(): void { | |||
| if (!document.hasFocus()) { | |||
Collaborator
There was a problem hiding this comment.
any reason why you didn't restore the original implementation?
const timeSinceLastInteraction = Date.now() - lastInteractionTime;
if (timeSinceLastInteraction > 60000) {
try {
window.electron.showNotification({
title: 'Goose finished the task',
body: 'Click here for details',
});
} catch (error) {
console.error('Failed to show notification:', error);
}
}
Contributor
Author
There was a problem hiding this comment.
hmm yeah i dont like that it didnt restore it easier. I think goose did too much. Closing this PR for a better one here: #6427
Contributor
Author
|
closing this PR for #6427 |
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
2 participants
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
When Goose finishes a task, show a native OS notification if the window is not focused. This helps users who switch to other apps while waiting for Goose to complete.
Changes
ui/desktop/src/components/BaseChat.tsx: Add notification trigger inonStreamFinishcallbackdocument.hasFocus()returnsfalse(user is in another app)window.electron.showNotificationinfrastructureui/desktop/src/components/TaskCompletionNotification.test.tsx: New test file with 5 testsTesting
How to manually test