Skip to content

fix(tui): remove blocking loading#394

Merged
jdx merged 3 commits into
jdx:mainfrom
gaojunran:fix-tui-loading
Apr 28, 2026
Merged

fix(tui): remove blocking loading#394
jdx merged 3 commits into
jdx:mainfrom
gaojunran:fix-tui-loading

Conversation

@gaojunran

Copy link
Copy Markdown
Contributor

No description provided.

@greptile-apps

greptile-apps Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refactors the TUI's loading mechanism from a blocking View::Loading state (which suspended all input) to a non-blocking overlay driven by loading_text. IPC operations (start/stop/restart/enable/disable) are now dispatched as tokio::spawn background tasks that return results via an mpsc channel, and an in_flight flag prevents overlapping operations. The clears_in_flight field on TaskResult::Refresh correctly addresses the race conditions called out in the two previous review threads.

Confidence Score: 5/5

Safe to merge — the refactor is well-structured and the two race conditions flagged in prior threads are correctly resolved by the clears_in_flight flag.

No P0 or P1 findings. All IPC completion handlers correctly call stop_loading() and clear in_flight before spawning a post-op refresh. The clears_in_flight: true path for Action::Refresh properly dismisses the overlay and resets in_flight. All branches of synchronous operations (SaveConfig, DeleteDaemon) call stop_loading() in every outcome including error paths.

No files require special attention.

Important Files Changed

Filename Overview
src/tui/mod.rs Core refactor: introduces TaskResult enum and mpsc channel, dispatches all IPC calls to tokio::spawn, adds in_flight guard, and uses clears_in_flight to safely dismiss the loading overlay — addressing both previous thread concerns.
src/tui/app.rs Removes View::Loading variant; splits refresh() into fetch_daemon_data() (async, IPC) and apply_refresh() (sync, state mutation); same split for network refresh — clean separation of concerns.
src/tui/ui.rs Loading overlay is now drawn unconditionally based on loading_text presence rather than View::Loading, enabling it to appear on top of any view.
src/tui/event.rs Removes View::Loading arm from key-event dispatch; minor cleanup only.
mise.lock Auto-generated lock file update: domain change (mise.jdx.dev → mise.en.dev) and provenance fields added for aube and communique tools.

Reviews (5): Last reviewed commit: "fix: misc" | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the blocking View::Loading pattern with a non-blocking async design: IPC operations now run in tokio::spawn tasks and report results back to the main loop via an mpsc channel, keeping the TUI responsive during long operations. The View::Loading variant is removed and the loading overlay is instead drawn whenever loading_text is set.

  • P1: TaskResult::Refresh (line 269 of src/tui/mod.rs) never calls app.stop_loading(). When the user manually triggers a refresh (r key), start_loading(\"Refreshing...\") is called but the loading overlay is never dismissed once the data arrives — it persists for the remainder of the session.

Confidence Score: 3/5

Not safe to merge as-is — manual refresh leaves a permanent loading overlay.

A P1 logic bug causes the 'Refreshing...' loading indicator to never clear after user-triggered refresh, which is a visible and reproducible regression introduced by this PR. The fix is a single line, but the bug affects normal TUI usage.

src/tui/mod.rs — TaskResult::Refresh handler at line 269

Important Files Changed

Filename Overview
src/tui/mod.rs Core event loop refactored to use async background tasks via mpsc channel; missing stop_loading() in TaskResult::Refresh causes a permanent loading overlay on manual refresh
src/tui/app.rs Cleanly splits refresh/network-refresh into fetch (async) + apply (sync) to support background task pattern; View::Loading variant removed correctly
src/tui/event.rs View::Loading match arm removed; remaining arms correctly consolidated
src/tui/ui.rs Loading overlay now drawn unconditionally when loading_text is set, rendered on top of any view — correct approach for the non-blocking design
mise.lock Auto-generated lockfile update: added provenance attestation fields and updated comment URL

Reviews (2): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile

Comment thread src/tui/mod.rs Outdated
@gaojunran

gaojunran commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

@greptile-apps

Comment thread src/tui/mod.rs
@gaojunran gaojunran marked this pull request as ready for review April 28, 2026 06:14
@jdx jdx merged commit 983d8d1 into jdx:main Apr 28, 2026
3 checks passed
jdx pushed a commit that referenced this pull request Apr 28, 2026
## 🤖 New release

* `pitchfork-cli`: 2.7.0 -> 2.8.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [2.8.0](v2.7.0...v2.8.0)
- 2026-04-28

### Added

- *(boot-start)* support system level register
([#397](#397))

### Fixed

- *(tui)* remove blocking loading
([#394](#394))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants