-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Feat/automatic update installation #5345
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?
Feat/automatic update installation #5345
Conversation
| const downloadAndInstallUpdate = async () => { | ||
| setUpdateStatus('downloading'); | ||
| setProgress(0); | ||
|
|
||
| try { | ||
| const result = await window.electron.downloadUpdate(); | ||
|
|
||
| if (!result.success) { | ||
| throw new Error(result.error || 'Failed to download update'); | ||
| } | ||
|
|
||
| // The download progress and completion will be handled by updater events | ||
| } catch (error) { | ||
| console.error('Error downloading update:', error); | ||
| setUpdateInfo((prev) => ({ | ||
| ...prev, | ||
| error: error instanceof Error ? error.message : 'Failed to download update', | ||
| })); | ||
| setUpdateStatus('error'); | ||
| setTimeout(() => setUpdateStatus('idle'), 5000); | ||
| } | ||
| }; | ||
|
|
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.
removed as downloads are automatic now
| {updateInfo.isUpdateAvailable && updateStatus === 'idle' && ( | ||
| <div className="text-xs text-text-muted mt-4 space-y-1"> | ||
| <p>Update will be downloaded automatically in the background.</p> | ||
| <p className="text-xs text-green-600"> | ||
| The update will be installed automatically when you quit the app. | ||
| </p> | ||
| </div> | ||
| )} | ||
|
|
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.
expected behavior for signed build
| log.info(`GitHubUpdater: Available assets: ${release.assets.map((a) => a.name).join(', ')}`); | ||
|
|
||
| const asset = release.assets.find((a) => a.name === assetName); | ||
| const asset = release.assets.find((a) => a.name.toLowerCase() === assetName); // keeping comparisms to lower case becasue Goose vs goose |
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.
thought about either changing the hardcoded values in lines 94-103 or keeping this normalized comparison. Let me know if i should change the hardcoded values instead.
| log.warn(`GitHubUpdater: No matching asset found for ${assetName}`); | ||
| } | ||
|
|
||
| if (!downloadUrl) { |
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.
edge case - update is available but no asset match.
TLDR; App detects there is a new version to be downloaded, code block on line 94 assetName = 'goose.zip'; but the build for that platform has Goose.zip. Due to uppercase - lowercase 'G' no downloadUrl set and downloading a new version keeps failing cause no url to hit.
Signed-off-by: AdemolaAri <ademola.ari@gmail.com>
…logic Signed-off-by: AdemolaAri <ademola.ari@gmail.com>
Signed-off-by: AdemolaAri <ademola.ari@gmail.com>
bdab6cc to
ddccbb6
Compare
|
Thank you for your contribution! Let me add the tags, and tag teammates for review. @DOsinga @alexhancock @michaelneale @jamadeo @zanesq @angelahning As we are in the weekend in AU and in the states, expect a review come Monday, ET! @AdemolaAri |
|
Thanks @AdemolaAri! I tested locally and it seems to download the update but it didn't actually install it after restart does it work for you? Also a few minor things with the experience:
2. The prompt after downloading still shows the text to the user to extract the file and move to Applications. After clicking this it open downloads it closes the app. Can we improve this experience a bit?
|
|
@zanesq , auto-install did not work for me on mac. from my findings, it seems Mac does not allow auto-install of unsigned app which is what builds locally. to really test auto-install, we'll need a signed app - which I believe only the app owners can do. chatGPT breakdown and references to this (https://chatgpt.com/share/690015a1-9500-800a-96a9-830079e13087) I'll work on those other items. |
|
.bundle |
|
ok thanks looking into how we can test a signed version of this.. also if you want to look at #5424 while you are at it |
|
quick heads up timing wise! @zanesq - Hacktoberfest has a deadline where all PRs need to be merged by October 31st (Friday). To make sure contributor PRs like this one hit that deadline on time, aiming to have these PRs reviewed and approved (if we're able) by EOD tomorrow (October 30th)! Let me know if you think it's possible for this one 🙏 and thank you for working on this, @AdemolaAri ! |
|
Not sure if this will make it unfortunately since there are still changes requested and we need to test in an actual signed release. I'll try to get a signed release tested with the current changes so we can rule that out at least. |
|
after the requested changes are made and approved if this doesn't break anything we can merge to main to test on a signed build and revert if needed |
|
@taniandjerry , I'm sorry I've not been able to work on the other things I need for this - I do not have access to my computer until this Friday. It's fine if this doesn't count towards HacktoberFest. I plan to get the updates on this PR out sometime this weekend |
That is amazing. Since your submission would likely pass beyond hacktoberfest, but you've been working on this feature with @zanesq that will make an impact on users for goose, I'll definitely be sure we are able to reward you in some way for still working on this. 🤗 ❤️ looking forward to getting this across the finish line! |
|
@michaelneale made a signed build to test from this branch but I get the following error message
|
@zanesq interesting. I believe you're hitting this code block, I added https://github.com/block/goose/pull/5345/files#r2457676532 . I encountered this when the app detects that there's a new version available, but when the code scans the release version repository it couldn't find a build for that assetName. Yeah, I thought I resolved it by making the assetName search case insensitive but might need some cleanups there |
|
not latest version its 1.11.0 in the screenshot and latest is 1.12.1 |




Summary
This is a feature request (#4322) to have goose auto-download and install updates when available. This change adds that functionality.
Type of Change
Testing
Manual testing of the actual build. I added console.log statements and followed the follwing steps
Note: because these builds are unsigned, i wasn't able to test the auto-install part. I believe that should work as well.
Related Issues
Relates to #ISSUE_ID
Discussion: LINK (if any)
#4322
Screenshots/Demos (for UX changes)
Before:
After:



|
|
|
|
Email: