fix(api): dynamically reload apps if modified on disk - #5506
Conversation
This adds a timestamp check in proc::refresh() and calls it from the /applist endpoint to ensure changes made to apps.json outside the Web UI are immediately reflected in Moonlight.
This adds a timestamp check in proc::refresh() and calls it from the /applist endpoint to ensure changes made to apps.json outside the Web UI are immediately reflected in Moonlight.
…/Sunshine into fix/applist-reload
e11532b to
3e8b40d
Compare
ReenigneArcher
left a comment
There was a problem hiding this comment.
I found two functional regressions in this version. The timestamp guard skips the initial apps.json parse on UCRT64, and reloading the list can discard a running app's process state. I confirmed the timestamp ordering with MSYS2 UCRT64 GCC 16.2; I did not run a full Sunshine build or streaming session.
…n refresh
The timestamp guard in proc::refresh() compared against a value-initialized
file_time_type{}, which on MSYS2 UCRT64 GCC 16.2 compares greater-or-equal
to file_time_type::clock::now(). This caused the very first refresh() call
at startup to skip parsing apps.json, leaving the application list empty.
Replace the bare file_time_type with std::optional<file_time_type> so that
nullopt unambiguously represents 'never parsed', regardless of platform
clock epoch.
Additionally, refresh() previously move-assigned the entire newly parsed
proc_t over the global proc::proc, which overwrites _app_id, _process,
_process_group, and cleanup iterators for an in-flight streaming session.
Refreshing the Moonlight app list during a session could therefore lose
process tracking, preventing /cancel from terminating the running app.
Add proc_t::update_apps_and_env() to replace only _apps and _env from the
newly parsed result, preserving all active process state.
Add regression tests covering both fixes.
This comment was marked as resolved.
This comment was marked as resolved.
…pass ci impersonation
… restrictions and secure get_env
ReenigneArcher
left a comment
There was a problem hiding this comment.
The original startup and active-session findings are addressed, and the new environment check covers the cleanup variable that was previously lost. One timestamp edge case remains in the new reload guard.
eduardomozart
left a comment
There was a problem hiding this comment.
Tests are now passing as expected, no Sonar issues.
ReenigneArcher
left a comment
There was a problem hiding this comment.
The earlier timestamp finding is fixed: the guard now compares for equality, and the new test covers a decreasing mtime. Focused UCRT64 syntax checks passed for the changed process source and test file. The remaining comments are style/documentation cleanup under AGENTS.md and .clang-format. git diff --check currently reports trailing whitespace at test_process.cpp:379 and an extra blank line at EOF; the targeted clang-format dry run also fails. Verification status: the GitHub Actions workflows are awaiting maintainer approval, and the GH Pages documentation job timed out waiting for its dependent workflow, so full CI test results are not available yet.
Bundle ReportBundle size has no change ✅ |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
Thanks for your patience with all the requested changes. I'm trying out a new process to review PRs with Codex in order to try to get through this backlog faster. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5506 +/- ##
==========================================
+ Coverage 39.40% 39.98% +0.57%
==========================================
Files 122 122
Lines 28323 28361 +38
Branches 12353 12356 +3
==========================================
+ Hits 11161 11339 +178
- Misses 15264 15911 +647
+ Partials 1898 1111 -787
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 39 files with indirect coverage changes Continue to review full report in Codecov by Harness.
|
Screenshot ComparisonPR #5506 screenshots vs Matrix:
|
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: AppImage-x86_64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: Windows-AMD64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: Windows-ARM64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: macOS-arm64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: macOS-x86_64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Description
When
apps.jsonis modified directly on the filesystem (e.g. by external scripts or manually in a text editor), the Moonlight client still receives the old app list because the API relies on an in-memory cache ofproc::proc. This pull request fixes the issue by securely tracking the file'slast_write_timeinproc::refresh()and callingrefreshdynamically on the/applistAPI endpoint, seamlessly reloading the list only if modifications have occurred.UpdateAppsAndEnv_UpdatesAppsListUpdateAppsAndEnv_PreservesRunningState_app_id/placebosurvive the update (placebo desktop app)UpdateAppsAndEnv_PreservesSessionEnvironmentSUNSHINE_APP_NAME) survive the update and are successfully passed toundo_cmdscriptsRefresh_ParsesFileOnFirstCallrefresh()always parses regardless offile_time_type{}semanticsRefresh_SkipsUnchangedFileRefresh_ReparseAfterFileModifiedRefresh_ReparseAfterFileMtimeDecreasedRefresh_PreservesRunningAppDuringReparserunning()preservedFileTimeType_DefaultValueComparisonfile_time_type{}comparison behaviorScreenshot
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage
See our AI usage policy.