Conversation
Reviewer's GuideReplaces the previous Python-based self-update script with a dedicated native installer binary (with optional GUI) that orchestrates uv-based updates, adds robust version handling and logging, and ensures the Windows packer includes the installer executable. Sequence diagram for the new update process using installer.exesequenceDiagram
actor User
participant Manager as GUI_manager
participant Installer as installer.exe
participant Uv as uv_exc
participant NewApp as restarted_CGS
User->>Manager: click_update_to_version(ver)
Manager->>Manager: compute_index_url_and_log_path
alt windows_and_installer_exists
Manager->>Installer: spawn_with_args(version, uv_exc, index_url, parent_pid, uv_tool_dirs)
Note over Installer: main
Installer->>Installer: parse_CliArgs
Installer->>Installer: InstallerConfig.from_args
Installer->>Installer: wait_for_parent_exit(parent_pid)
Installer->>Installer: cleanup_stale_dirs
alt no_gui_flag
Installer->>Installer: run_update_cli
Installer->>Uv: tool_install_ComicGUISpider
Uv-->>Installer: exit_code
else gui_mode
Installer->>Installer: run_gui
Installer->>Installer: spawn_update_worker
Installer->>Uv: tool_install_ComicGUISpider
Uv-->>Installer: progress_output_and_exit_code
end
alt install_success
Installer->>Installer: restart_cgs
Installer->>Uv: tool_run_cgs
Uv-->>NewApp: start_new_process
end
else non_windows_or_no_installer
Manager->>Uv: spawn_shell_with_uv_tool_install_command
Uv-->>Manager: install_logs_written
end
Manager->>Manager: close_gui
Class diagram for the new native installer componentsclassDiagram
class CliArgs {
+String uv_exc
+String version
+String index_url
+u32 parent_pid
+String uv_tool_dir
+String uv_tool_bin_dir
+bool no_gui
}
class InstallerConfig {
+PathBuf uv_exc
+String version
+String index_url
+PathBuf uv_tool_dir
+PathBuf uv_tool_bin_dir
+from_args(args~CliArgs~) InstallerConfig
+log_path() PathBuf
+install_args() Vec~String~
}
class InstallerEvent {
<<enum>>
Progress
Finished
Fatal
}
class UpdaterApp {
-String version
-String status
-u8 progress
-Receiver~InstallerEvent~ rx
-Arc~AtomicI32~ exit_code
-Option~Instant~ close_at
+new(version String, rx Receiver~InstallerEvent~, exit_code Arc~AtomicI32~) UpdaterApp
-drain_events() void
+update(ctx Context, frame Frame) void
}
class VersionModule {
+normalize_version(version String) (String, bool)
+build_install_args(version String, index_url String) Vec~String~
}
class ProcessModule {
+cleanup_stale_dirs(config InstallerConfig) Result~(),io::Error~
+wait_for_parent_exit(parent_pid u32, timeout_ms u32) void
+run_update_cli(config InstallerConfig) i32
+spawn_update_worker(config InstallerConfig, tx Sender~InstallerEvent~) JoinHandle~()~
+restart_cgs(config InstallerConfig) Result~(),io::Error~
}
CliArgs --> InstallerConfig : used_to_create
InstallerConfig --> VersionModule : uses
InstallerConfig --> ProcessModule : passed_into
InstallerEvent --> UpdaterApp : drives_state
VersionModule <.. ProcessModule : uses_functions
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
GUI/manager/__init__.py::to_update, usingshlex.quotewhen building PowerShell andcmdcommand strings on Windows is likely to produce incorrectly escaped paths/arguments sinceshlexis POSIX-oriented; consider usingsubprocess.list2cmdlineor manual quoting appropriate forcmd/PowerShell instead. - In
tools/installer/src/process.rs::open_log, the fallbackexpect("cannot open any log file")will hard-crash the installer if both log paths fail; if logging is non-critical, consider gracefully degrading (e.g., usingstderronly) instead of panicking.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `GUI/manager/__init__.py::to_update`, using `shlex.quote` when building PowerShell and `cmd` command strings on Windows is likely to produce incorrectly escaped paths/arguments since `shlex` is POSIX-oriented; consider using `subprocess.list2cmdline` or manual quoting appropriate for `cmd`/PowerShell instead.
- In `tools/installer/src/process.rs::open_log`, the fallback `expect("cannot open any log file")` will hard-crash the installer if both log paths fail; if logging is non-critical, consider gracefully degrading (e.g., using `stderr` only) instead of panicking.
## Individual Comments
### Comment 1
<location> `GUI/manager/__init__.py:194` </location>
<code_context>
subprocess.Popen(args, creationflags=subprocess.CREATE_NEW_CONSOLE, env=env)
else:
- subprocess.Popen(args, start_new_session=True, env=env)
+ cmd = f"{uv_exc} tool install ComicGUISpider=={ver} --force --index-url {index_url}"
+ if os.name == "nt":
+ subprocess.Popen(["cmd", "/c", "start", "", "powershell", "-NoProfile", "-Command",
</code_context>
<issue_to_address>
**issue (bug_risk):** Command construction and quoting are fragile on Windows and may misbehave with spaces/special characters.
`cmd` is a flat f-string combining `uv_exc`, `ver`, and `index_url` and then embedded into a PowerShell `-Command` string. This introduces several risks on Windows:
- Spaces or special characters in `uv_exc`, `index_url`, or `log` can break the command.
- `shlex.quote()` is POSIX-focused and doesn't match PowerShell quoting semantics.
- Passing everything as one `-Command` string means characters like `&` or `?` in `index_url` can be interpreted by PowerShell.
Please restructure this to avoid stringly-typed command building: either call `uv_exc` directly with an argument list (skipping PowerShell), or build a PowerShell `Start-Process` with `-ArgumentList` where each argument is properly escaped for PowerShell rather than using `shlex.quote()`.
</issue_to_address>
### Comment 2
<location> `tools/installer/Cargo.toml:4` </location>
<code_context>
+[package]
+name = "cgs-installer"
+version = "2.8.6"
+edition = "2024"
+
+[[bin]]
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `edition = "2024"` will currently fail to compile on stable Rust.
Currently only `2015`, `2018`, and `2021` are stable editions; `edition = "2024"` will make builds fail on standard stable toolchains. Unless you need unreleased 2024-only features (and are pinning to a matching nightly/preview toolchain), consider using `"2021"` instead, or clearly document and/or gate the requirement for a 2024-capable toolchain.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Deploying comicguispider-docs with
|
| Latest commit: |
db80701
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bb8eb79e.comicguispider-docs.pages.dev |
| Branch Preview URL: | https://2-8-dev.comicguispider-docs.pages.dev |
jsonmaki
approved these changes
Feb 15, 2026
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.
Description
Related Issues
Checklist:
Summary by Sourcery
Introduce a native cross-platform updater and integrate it into the application update flow.
New Features:
Enhancements:
Tests: