fix: dev frontend readiness - #5828
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe CLI adds ChangesFrontend readiness startup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DevTask
participant FrontendDevServer
participant WailsCLI
participant NativeApplication
DevTask->>FrontendDevServer: Start frontend development server
DevTask->>WailsCLI: Run wails3 tool waitport
WailsCLI->>FrontendDevServer: Poll configured port
FrontendDevServer-->>WailsCLI: Accept connection
WailsCLI-->>DevTask: Complete readiness task
DevTask->>NativeApplication: Start native application
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@v3/go.mod`:
- Around line 5-6: Remove the local sibling replacement for
github.com/atterpac/refresh in go.mod and pin it to a compatible tracked module
revision instead. Ensure clean checkouts and CI can resolve the imported refresh
packages without requiring ../../refresh to exist.
In `@v3/internal/commands/tool_waitport.go`:
- Around line 46-60: Update the port-resolution logic in the wait-port command
to reject values outside 1..65535, including environment-derived ports, before
invoking waitForPort. Validate the command timeout is positive before
constructing the duration or polling, and return clear validation errors for
invalid values. Add command-level tests covering invalid flag and environment
ports, plus zero and negative timeouts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e22fe157-59f9-4675-901a-5db12f03bd0f
📒 Files selected for processing (12)
v3/UNRELEASED_CHANGELOG.mdv3/cmd/wails3/main.gov3/go.modv3/internal/commands/build_assets/Taskfile.tmpl.ymlv3/internal/commands/build_assets/config.ymlv3/internal/commands/dev_config_test.gov3/internal/commands/tool_waitport.gov3/internal/commands/tool_waitport_test.gov3/internal/commands/watcher_test.gov3/pkg/application/application.gov3/pkg/application/application_dev.gov3/pkg/application/application_production.go
50ccf52 to
897dc5b
Compare
aeaf5af to
1186937
Compare
20ec6fd to
18f4a17
Compare
18f4a17 to
6eee683
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves wails3 dev startup robustness by moving frontend-dev-server readiness checks out of application.App.Run and into the dev-process orchestration layer, so slow (but valid) frontend startups don’t cause the native app to terminate.
Changes:
- Remove runtime-level frontend readiness gating from
application.App.Runand dev-only app hooks. - Add
wails3 tool waitportand wire it into generated dev configs (build_assets/config.yml,Taskfile.tmpl.yml) to gate native startup on port readiness with a 60s default. - Add unit tests covering the port-wait retry logic and generated dev configuration ordering.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| v3/UNRELEASED_CHANGELOG.md | Documents the new dev startup ordering and alpha-project migration steps. |
| v3/pkg/application/application.go | Removes the pre-run readiness hook from App.Run. |
| v3/pkg/application/application_production.go | Simplifies production hooks; removes unused pre-run hook and adjusts postQuit. |
| v3/pkg/application/application_dev.go | Removes dev-mode readiness logic; retains watcher shutdown messaging when dev server URL is set. |
| v3/internal/commands/watcher_test.go | Updates tests to use require assertions. |
| v3/internal/commands/tool_waitport.go | Introduces waitport tool and retry/timing logic for port readiness. |
| v3/internal/commands/tool_waitport_test.go | Adds unit tests for waitForPort retry/timeout behavior. |
| v3/internal/commands/dev_config_test.go | Adds tests ensuring generated dev config gates app start on frontend readiness and timeout is task-local. |
| v3/internal/commands/build_assets/Taskfile.tmpl.yml | Adds dev:wait task that invokes wails3 tool waitport. |
| v3/internal/commands/build_assets/config.yml | Inserts a type: once readiness step between frontend (background) and app (primary). |
| v3/go.mod | Updates github.com/atterpac/refresh dependency version. |
| v3/go.sum | Updates github.com/atterpac/refresh checksums for the new version. |
| v3/cmd/wails3/main.go | Registers the new tool waitport subcommand. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| github.com/atotto/clipboard v0.1.4 | ||
| github.com/atterpac/refresh v1.0.0 | ||
| github.com/atterpac/refresh v1.1.2 | ||
| github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 |
| if options.Timeout <= 0 { | ||
| return fmt.Errorf("timeout must be greater than zero") | ||
| } | ||
|
|
||
| timeout := time.Duration(options.Timeout) * time.Second | ||
| if !waitForPort(func() bool { return isPortOpen(options.Host, options.Port) }, timeout) { | ||
| return fmt.Errorf("timed out after %s waiting for port %d to open on %s", timeout, options.Port, options.Host) | ||
| } | ||
| return nil |
|
I'm keen to get @atterpac's thoughts on this. It sounds like there isn't a default fix here and a need to manually upgrade taskfiles. |
|
Curious since we added this to support to refresh we could just make it a change to the config file upstream (in refresh config?) might be cleaner if we can just have someone opt in via the config.yaml and configure it somewhat similar to a k8s container syntax executes:
...
- cmd: wails3 task common:dev:frontend
type: background-readiness
readiness-check: http://localhost:9172/
...Background type readiness would be treated as blocking until readiness-check returns a 2XX then is treated as background Means no additional task files or changes to wails outside of a refresh version bump Thoughts? |
Yeah I think that's a good idea. |
…gic and test coverage
b9e51f7 to
90a52f9
Compare
Description
This PR prevents
wails3 devfrom terminating the native application when the frontend development server takes longer than expected to start.Previously, frontend readiness was checked inside
application.App.Runusing 10 HTTP attempts with 500 ms delays. This gave the frontend approximately five seconds to become available.Frontend startup can legitimately take longer when Vite needs to optimize or re-optimize dependencies. In that case, Wails terminated the native application with a fatal error even
though the frontend server was still starting normally:
This PR moves frontend readiness from the application runtime into the wails3 dev process orchestration:
This prevents valid but slow frontend startups from terminating the application and keeps development-process coordination out of the application runtime.
Migration for existing Wails v3 alpha projects
Existing projects must update their generated development configuration manually.
Add the following task to build/Taskfile.yml, next to the existing dev:frontend task:
Then update dev_mode.executes in build/config.yml by inserting the readiness step after the background frontend task and before the primary application task:
The important execution order is:
Projects using a custom wails3 dev -config file must update that configuration instead of build/config.yml.
Projects whose frontend server does not use WAILS_VITE_PORT may specify the host and port explicitly:
Newly generated projects include the updated configuration automatically.
Dependency
This PR temporarily depends on the
fix/background-startup-exitbranch of github.com/savely-krasovsky/refresh. That branch adds the background and one-shot process semantics required for startup orchestration.The current development replacement is:
replace github.com/atterpac/refresh => ../../refresh
Before this PR can be merged, the refresh changes must be published or merged and the local replacement must be changed to an upstream version or remote pseudo-version.
Fixes #?
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Unit tests were added for:
Manual verification procedure:
wails3 devIf you checked Linux, please specify the distro and version.
Tested on Fedora Linux 44 using a Toolbx container.
Test Configuration
Checklist:
Summary by CodeRabbit
waitportdevelopment tool that blocks until a specified host/port becomes reachable, with a configurable timeout (default: 60s).wails3 devstartup so the native app launches only after the frontend dev server is accepting connections, including alpha-project flow.