fix: watch providers.jsonc for external edits so UI refreshes without restart#3233
fix: watch providers.jsonc for external edits so UI refreshes without restart#3233Neppkun wants to merge 8 commits into
Conversation
…ut restart Manual edits to ~/.mux/providers.jsonc were silently ignored by the frontend because notifyConfigChanged() was only called after API mutations. Added watchProvidersFile() to Config (fs.watch on the mux home directory, debounced 300ms) and wired it into ProviderService's constructor so external file changes propagate to all onConfigChanged subscribers automatically. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
I apologize in advance for using Claude Code to write this instead of Mux. (I ran out of credits) |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 902766a042
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…oop handle Two CI failures caused by the providers.jsonc watcher: - Smoke test crashed with ENOENT because ~/.mux doesn't exist on a fresh install; now ensurePrivateDirSync is called before fs.watch. - Integration tests hung and tore down with "Jest environment has been torn down" errors because the persistent watcher kept the event loop alive; fixed by passing persistent: false to fs.watch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The persistent: false flag was sufficient on Linux/macOS but Windows Jest workers still flagged "failed to exit gracefully" after my second commit, because the watcher's FSWatcher handle was never closed when tests tore down their ServiceContainer. Store the stop-watching callback returned by Config.watchProvidersFile, expose ProviderService.dispose() that calls it and clears emitter listeners, and wire it into ServiceContainer.dispose() alongside the other service disposals. Verified locally: - providerService unit tests: 75/75 pass - Standalone repro confirms the watcher fires on edits and disposal removes all active handles - Typecheck and lint clean on changed files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a9efb3226
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two issues Codex flagged on the watcher: P1 — fs.watch can throw for reasons beyond ENOENT: unsupported network filesystems (NFS/SMB), virtualized mounts, or watch-limit exhaustion (ENOSPC on Linux). Wrap the call in try/catch, log a warning, and return a no-op cleanup. The app boots and provider config keeps working; users on those filesystems just lose live-refresh (which is the pre-PR behaviour). P2 — fs.watch may pass changedFilename=null on some platforms/kernels (notably older macOS FSEvents). The previous "changedFilename !== filename" guard silently dropped valid events in that case, defeating the feature for exactly the edge it's meant to fix. Treat null as "unknown — might be ours" and fire the debounced callback; the worst case is one extra re-fetch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50aed63577
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex flagged that ensurePrivateDirSync was outside the try/catch guarding the watcher, so a read-only filesystem or unwritable MUX_ROOT would still throw out of ProviderService construction and crash startup. Move directory creation inside the same try/catch so every failure path degrades to the documented "manual edits need restart" behavior instead of taking the app down. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02e2eec676
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…n-app saves Mutation methods already call notifyConfigChanged() immediately after writing providers.jsonc. The file-watcher callback would then fire a second time ~300 ms later for the same write, causing redundant backend→frontend refresh cycles and potential UI flicker. Add lastMutationNotifyTime and a private notifyFromMutation() helper. Each mutation that writes providers.jsonc calls notifyFromMutation() instead of notifyConfigChanged(); the watcher callback skips if the recorded time is within 1500 ms (debounce + generous OS latency slack). External manual edits remain unaffected — their watcher events arrive well after any in-app save timestamp. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Alright I connected my ChatGPT account to Github... lets see if I can use codex. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 169a989078
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… dedupe Address two Codex review findings: P1 — providers.jsonc watcher had no 'error' listener, so runtime failures (e.g. mux home directory removed/unmounted after startup) emitted on the global 'uncaughtException' path and could terminate the process. Attach a local 'error' handler that logs, clears any pending debounce, and closes the watcher — degrading to the pre-PR "manual edits require restart" behaviour instead of crashing. P2 — the previous 1500 ms time-window dedupe suppressed *all* watcher events for that window, so a genuine external edit landing inside it was silently dropped. Replace the window with a write-specific fingerprint: capture providers.jsonc's mtime immediately after each in-app save, and only suppress a watcher event when the file's current mtime still matches. An external edit always changes mtime and therefore is never accidentally suppressed. Adds Config.getProvidersFileMtimeMs() so ProviderService doesn't need to reach into the private providersFile path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 125d14af49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…f-write dedupe Filesystems with coarse timestamp granularity (FAT, some network mounts) can bucket two distinct writes into the same `mtimeMs`. With the previous mtime-equality check, an external edit that landed in the same bucket as an in-app save would share the same mtime and get silently suppressed. Replace the mtime fingerprint with a sha256 of the file contents. The watcher callback only suppresses a notification when the file's current hash still matches the hash captured immediately after the in-app save. A genuine external edit always changes the bytes, so its hash differs and the notification is forwarded. If two writes happen to produce byte-identical content, suppression is a no-op anyway. Rename Config.getProvidersFileMtimeMs() → getProvidersFileFingerprint() and ProviderService.lastSelfWriteMtimeMs → lastSelfWriteFingerprint accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Summary
~/.mux/providers.jsoncnever appeared in the UI becausenotifyConfigChanged()was only called after API mutations, not on external file changesConfig.watchProvidersFile()— watches the mux home directory withfs.watch, debounced 300 ms, returns a cleanup fnProviderServiceconstructor so any external edit toproviders.jsoncimmediately propagates to allonConfigChangedfrontend subscribersRoot cause
ProviderService.list()andgetConfig()already readproviders.jsoncfresh on every call, so the backend always had the right data. The missing piece was a signal to the frontend to re-fetch.notifyConfigChanged()emits that signal — but it was only called after in-app edits, leaving manual file edits invisible until restart.Test plan
~/.mux/providers.jsoncwhile the app is running — it should appear in Settings → Providers within ~300 ms, no restart neededMaxListenersExceededWarningnoise in dev logs🤖 Generated with Claude Code