[feature/git_helper-13] Fixed Update Check and Self-update#17
Merged
justinkumpe merged 1 commit intomainfrom Dec 31, 2025
Merged
[feature/git_helper-13] Fixed Update Check and Self-update#17justinkumpe merged 1 commit intomainfrom
justinkumpe merged 1 commit intomainfrom
Conversation
Reviewer's GuideRefines the git-helper script’s update checking and self-update behavior, improves robustness of UI interactions (especially in non-interactive/CI contexts), and adds utilities to safely locate and replace the running script while avoiding side effects when sourced. Sequence diagram for the updated check_updates and self-update flowsequenceDiagram
actor User
participant GitHelper as GitHelperScript
participant GitHubRaw as GitHubRawEndpoint
participant FS as Filesystem
User->>GitHelper: select_check_updates()
activate GitHelper
GitHelper->>GitHelper: cmd_check_updates()
GitHelper->>GitHelper: fetch_latest_version()
GitHelper->>GitHubRaw: HTTP_GET raw_git_helper.sh
alt fetch_success
GitHubRaw-->>GitHelper: script_contents
GitHelper->>GitHelper: extract_and_trim_SCRIPT_VERSION()
alt version_missing
GitHelper->>User: ui_message_no_exit("Git Helper vSCRIPT_VERSION", "Could not retrieve version information from GitHub.")
else version_found
GitHelper->>GitHelper: compare_versions(SCRIPT_VERSION, latest_version)
alt versions_equal
GitHelper->>User: ui_message_no_exit("Git Helper vSCRIPT_VERSION", "You are running the latest version (SCRIPT_VERSION)!")
else current_older
GitHelper->>User: ui_message_no_exit("Git Helper vSCRIPT_VERSION", "A new version is available! ...")
GitHelper->>User: ui_yesno("Git Helper vSCRIPT_VERSION", "Download and replace this script with the latest version now?")
alt user_confirms_update
GitHelper->>GitHelper: update_script_from_remote()
GitHelper->>GitHelper: script_self_path()
GitHelper->>GitHubRaw: wget raw_git_helper.sh
alt download_and_sanity_ok
GitHubRaw-->>GitHelper: new_script_file
GitHelper->>FS: chmod +x tmp_file
GitHelper->>FS: mv tmp_file script_self_path
GitHelper->>User: ui_message("Git Helper vSCRIPT_VERSION", "Update complete. Please rerun the script.")
GitHelper->>GitHelper: exit 0
else update_failed
GitHelper->>User: ui_message_no_exit("Git Helper vSCRIPT_VERSION", "Update failed. Please try manual update.")
end
else user_declines_update
GitHelper-->>User: return_without_changes()
end
else current_newer
GitHelper->>User: ui_message_no_exit("Git Helper vSCRIPT_VERSION", "You are running a development version ...")
end
end
else fetch_failed
GitHubRaw-->>GitHelper: error
GitHelper->>User: ui_message_no_exit("Git Helper vSCRIPT_VERSION", "Failed to check for updates. Please check your internet connection.")
end
deactivate GitHelper
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
cmd_check_updates, togglingset +e/set -earoundcompare_versionscan unexpectedly change the shell’s errexit behavior for callers; consider capturing the exit code viaif ! compare_versions ...; thenor using a subshell to avoid mutating global shell options. - The
update_script_from_remotefunction requireswgetwhilefetch_latest_versionusescurl; to avoid surprising runtime failures on systems with only one tool installed, consider using a single HTTP client (or a shared helper that tries both) for both version check and self-update. - The
SCRIPT_VERSIONconstant was changed from1.0.0to0.0.1, which will cause the update logic to treat this build as older than any existing1.xrelease; double-check that this version downgrade is intentional and aligns with how remote versions are tagged.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `cmd_check_updates`, toggling `set +e`/`set -e` around `compare_versions` can unexpectedly change the shell’s errexit behavior for callers; consider capturing the exit code via `if ! compare_versions ...; then` or using a subshell to avoid mutating global shell options.
- The `update_script_from_remote` function requires `wget` while `fetch_latest_version` uses `curl`; to avoid surprising runtime failures on systems with only one tool installed, consider using a single HTTP client (or a shared helper that tries both) for both version check and self-update.
- The `SCRIPT_VERSION` constant was changed from `1.0.0` to `0.0.1`, which will cause the update logic to treat this build as older than any existing `1.x` release; double-check that this version downgrade is intentional and aligns with how remote versions are tagged.
## Individual Comments
### Comment 1
<location> `git_helpers/git-helper.sh:441-443` </location>
<code_context>
return 0
fi
+
+ # Validate versions are in correct format
+ if ! [[ "$v1" =~ ^[0-9]+(\.[0-9]+)*$ ]] || ! [[ "$v2" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then
+ return 1
+ fi
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Returning `1` on invalid version format conflates "older" with "invalid"
The regex check now makes invalid version strings (e.g. `v1.2.3` or empty) return `1`, which the contract uses for `version1 < version2`. In `cmd_check_updates` this will look like "current version is older" and may spuriously prompt updates when the remote version is malformed. Please return a distinct status for invalid input and handle it explicitly at the call site, or reject malformed versions earlier in `fetch_latest_version`.
Suggested implementation:
```
v1="${v1#"${v1%%[![:space:]]*}"}"
v1="${v1%"${v1##*[![:space:]]}"}"
v2="${v2#"${v2%%[![:space:]]*}"}"
v2="${v2%"${v2##*[![:space:]]}"}"
# Validate versions are in correct format; return distinct status for invalid input
if ! [[ "$v1" =~ ^[0-9]+(\.[0-9]+)*$ ]] || ! [[ "$v2" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then
return 2
fi
if [[ "$v1" == "$v2" ]]; then
return 0
fi
```
To fully implement the change in behavior, the call sites of this version comparison function need to be updated:
1. In `cmd_check_updates` (or whichever function calls the version comparison), handle the new `2` status code explicitly as "invalid / malformed version" and **do not** treat it as "current version is older". For example:
- On status `0`: versions equal → "up to date".
- On status `1`: current version older → offer update.
- On status `2`: log a warning about malformed local/remote version and skip update suggestion (or surface an error).
2. If there is a `fetch_latest_version` function that may return malformed versions, optionally add validation there too and fail early (e.g., return an empty string or an error) so that the compare function is not called with obviously bad input.
3. Update any tests or scripts that depend on the previous contract (`1` meaning only "older") to account for the new distinct error status.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+441
to
+443
| # Validate versions are in correct format | ||
| if ! [[ "$v1" =~ ^[0-9]+(\.[0-9]+)*$ ]] || ! [[ "$v2" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then | ||
| return 1 |
There was a problem hiding this comment.
suggestion (bug_risk): Returning 1 on invalid version format conflates "older" with "invalid"
The regex check now makes invalid version strings (e.g. v1.2.3 or empty) return 1, which the contract uses for version1 < version2. In cmd_check_updates this will look like "current version is older" and may spuriously prompt updates when the remote version is malformed. Please return a distinct status for invalid input and handle it explicitly at the call site, or reject malformed versions earlier in fetch_latest_version.
Suggested implementation:
v1="${v1#"${v1%%[![:space:]]*}"}"
v1="${v1%"${v1##*[![:space:]]}"}"
v2="${v2#"${v2%%[![:space:]]*}"}"
v2="${v2%"${v2##*[![:space:]]}"}"
# Validate versions are in correct format; return distinct status for invalid input
if ! [[ "$v1" =~ ^[0-9]+(\.[0-9]+)*$ ]] || ! [[ "$v2" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then
return 2
fi
if [[ "$v1" == "$v2" ]]; then
return 0
fi
To fully implement the change in behavior, the call sites of this version comparison function need to be updated:
- In
cmd_check_updates(or whichever function calls the version comparison), handle the new2status code explicitly as "invalid / malformed version" and do not treat it as "current version is older". For example:- On status
0: versions equal → "up to date". - On status
1: current version older → offer update. - On status
2: log a warning about malformed local/remote version and skip update suggestion (or surface an error).
- On status
- If there is a
fetch_latest_versionfunction that may return malformed versions, optionally add validation there too and fail early (e.g., return an empty string or an error) so that the compare function is not called with obviously bad input. - Update any tests or scripts that depend on the previous contract (
1meaning only "older") to account for the new distinct error status.
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
1 participant
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 by Sourcery
Improve the git-helper script’s update experience, robustness, and non-interactive behavior.
New Features:
Bug Fixes:
Enhancements: