Make the PO-token provider work right after a fresh install - #133
Merged
Conversation
On a fresh machine without Node.js, the installer's provider setup installs Node.js via winget but the same console session could fail to see the new executable (only %ProgramFiles%\nodejs was probed). The setup then bailed out, the provider was never built, and the GUI's Queue button stayed on loading... until the user re-ran the installer or update script from a new terminal. Two independent fixes: * Installer: after installing Node.js, also probe the per-user install location and refresh PATH from the registry (machine + user) before re-checking, so the provider build proceeds in the same run. * GUI self-heal: when Node.js and git are available but the provider was never built, the GUI now runs the provider setup script itself on startup (one-time, in the existing background worker, output in .potoken/setup.log) instead of asking the user to reinstall. A new progress signal surfaces the one-time build in the GUI console so the wait is explained. The setup scripts accept ULTRASINGER_POTOKEN_SKIP_WARMUP=1 to skip the warm-up step, since the GUI starts the server itself right afterwards.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR adds self-healing Node provider bootstrapping, improves setup-script PATH handling, allows warm-up skipping, reports setup progress in the GUI, and adds tests for success, failure, cancellation, and fallback behavior. ChangesPO-token provider bootstrap
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MainWindow
participant _PotokenWorker
participant ensure_provider
participant _bootstrap_node_provider
participant SetupHelper
MainWindow->>_PotokenWorker: start provider setup
_PotokenWorker->>ensure_provider: pass progress callback
ensure_provider->>_bootstrap_node_provider: build missing Node provider
_bootstrap_node_provider->>SetupHelper: run setup helper
SetupHelper-->>_bootstrap_node_provider: create provider entry
_bootstrap_node_provider-->>ensure_provider: return bootstrap result
ensure_provider-->>_PotokenWorker: provider status and progress
_PotokenWorker-->>MainWindow: append progress to queue log
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Problem
On a fresh machine without Node.js, the first install could finish with the PO-token provider unbuilt:
%ProgramFiles%\nodejsafterwards. If the executable was not visible there yet (e.g. per-user install scope, or the session PATH not reflecting the registry update), the setup bailed out with "ACTION REQUIRED" and the provider was never cloned/built.install\update.batfrom a new terminal (fresh PATH, Node.js visible) fixed it - which is exactly the workaround a user had to discover by hand.Fixes (two independent layers)
Installer robustness (
setup_potoken_provider.bat): after installing Node.js, probe both the machine-wide and per-user install locations and, as a fallback, refreshPATHfrom the registry (machine + user) before re-checking. The provider build now proceeds in the same installer run.GUI self-heal (
potoken_provider.py): when Node.js and git are available but the provider server was never built, the GUI now runs the provider setup script itself on first launch - one-time, inside the existing background worker, never blocking the UI, output captured in.potoken/setup.log. Instead of "re-run the install script", the user sees:Supporting changes:
.bat/.sh) honorULTRASINGER_POTOKEN_SKIP_WARMUP=1to skip the installer-time warm-up step, since the GUI bootstrap starts (and thereby warms up) the server itself right afterwards._PotokenWorkergains aprogresssignal so long-running provider steps are visible in the GUI console.Testing
ensure_provider); full GUI suite passes (164 passed)..batend-to-end with the skip-warm-up flag: builds and exits 0 without the warm-up wait.nodejsstripped from the session PATH: node is found again via the install-location probe / registry refresh.Summary by CodeRabbit
New Features
Bug Fixes