Auto reload config #1743

bshowell0 · 2025-10-13T08:03:20Z

PR checklist

Explain your changes in the relevant commit messages rather than in the PR description. The PR description must not contain more information than the commit messages (except for images and other media).
Each commit must explain what/why/how and motivation in its description. https://cbea.ms/git-commit/
Don't forget to link the appropriate issues/discussions in commit messages (if applicable).
Each commit must be an atomic change (a PR may contain several commits). Don't introduce new functional changes together with refactorings in the same commit.
The GitHub Actions CI must pass (you can fix failures after submitting a PR).

Failure to follow the checklist with no apparent reasons will result in silent PR rejection.

My attempt at solving #163, my first PR to this repo so let me know if I've missed anything obvious 👍

Add support for automatically reloading the config file when it changes on disk. This feature is controlled by new `auto-reload-config` config option (defaults to false). - New ConfigFileWatcher class that uses FSEvents to monitor the config file's directory for changes - Debouncing logic to handle editors that save multiple times/atomic saves - Integration with existing reload-config mechanism via runSession - FSEvents monitors the config directory - On file change, checks modification date to confirm actual change - Debounces rapid changes and reloads within a refresh session - File watcher is started/restarted after each successful config reload - Does not watch the default embedded config (only user configs) Closes nikitabobko#163

- Updated `default-config.toml` to include `auto-reload-config` with a comment describing its effect - Didn't give it it's own section in the docs, since, similar to `start-at-login`, it's a config-only feature that is self-explanatory

Didn't know about ./format.sh, caught by build-debug (macos-14) CI

nikitabobko

Thanks, but I am sceptical about this PR.

nikitabobko · 2025-10-14T10:10:58Z

Sources/AppBundle/config/ConfigFileWatcher.swift

+        var context = FSEventStreamContext(
+            version: 0,
+            info: Unmanaged.passUnretained(self).toOpaque(),
+            retain: nil,
+            release: nil,
+            copyDescription: nil,
+        )
+
+        let pathsToWatch = [directoryPath] as CFArray
+        let callback: FSEventStreamCallback = { (
+            streamRef: ConstFSEventStreamRef,
+            clientCallBackInfo: UnsafeMutableRawPointer?,
+            numEvents: Int,
+            eventPaths: UnsafeMutableRawPointer,
+            eventFlags: UnsafePointer<FSEventStreamEventFlags>,
+            eventIds: UnsafePointer<FSEventStreamEventId>,
+        ) in
+            guard let info = clientCallBackInfo else { return }
+            let watcher = Unmanaged<ConfigFileWatcher>.fromOpaque(info).takeUnretainedValue()
+
+            // Filter events to only react to our specific file
+            let paths = unsafeBitCast(eventPaths, to: NSArray.self) as? [String] ?? []
+            guard let watchedPath = watcher.watchedFileUrl?.path else { return }
+
+            for i in 0 ..< numEvents {
+                let flags = eventFlags[i]
+                let isFile = (flags & FSEventStreamEventFlags(kFSEventStreamEventFlagItemIsFile)) != 0
+                let isRelevant = (flags & FSEventStreamEventFlags(
+                    kFSEventStreamEventFlagItemModified |
+                        kFSEventStreamEventFlagItemRenamed |
+                        kFSEventStreamEventFlagItemCreated |
+                        kFSEventStreamEventFlagItemChangeOwner,
+                )) != 0
+
+                if isFile && isRelevant && paths[i] == watchedPath {
+                    // Already on main queue
+                    watcher.handleFileChange()
+                    break
+                }
+            }
+        }
+
+        guard let stream = FSEventStreamCreate(
+            kCFAllocatorDefault,
+            callback,
+            &context,
+            pathsToWatch,
+            FSEventStreamEventId(kFSEventStreamEventIdSinceNow),
+            0.3, // latency in seconds, letting the system coalesce events
+            FSEventStreamCreateFlags(kFSEventStreamCreateFlagFileEvents),
+        ) else {
+            return
+        }
+
+        FSEventStreamSetDispatchQueue(stream, DispatchQueue.main)
+
+        if !FSEventStreamStart(stream) {
+            FSEventStreamInvalidate(stream)
+            FSEventStreamRelease(stream)
+            return
+        }
+
+        eventStream = stream


It would help me to review this PR if you could share how you wrote this code so that I could follow along.

Did you copy the code from somewhere?
Did you google the API and then followed the docs?
Did you generate it with LLM?
Something else?

Did you verify the code yourself by reading the docs after you wrote it?

nikitabobko · 2025-10-14T10:11:40Z

Sources/AppBundle/config/ConfigFileWatcher.swift

+}
+
+@MainActor
+func stopConfigFileWatcher() {


Unused function

nikitabobko · 2025-10-14T10:19:35Z

Sources/AppBundle/config/ConfigFileWatcher.swift

+    private var lastModificationDate: Date?
+    private var debounceTask: Task<Void, Never>?
+
+    func startWatching(configUrl: URL) {


You always pass the same parameter here

nikitabobko · 2025-10-14T10:19:51Z

Sources/AppBundle/config/ConfigFileWatcher.swift

+@MainActor private var _configFileWatcher: ConfigFileWatcher?
+
+@MainActor
+func startConfigFileWatcher() {
+    if _configFileWatcher == nil {
+        _configFileWatcher = ConfigFileWatcher()
+    }
+    _configFileWatcher?.startWatching(configUrl: configUrl)
+}


Suggested change

@MainActor private var _configFileWatcher: ConfigFileWatcher?

@MainActor

func startConfigFileWatcher() {

if _configFileWatcher == nil {

_configFileWatcher = ConfigFileWatcher()

}

_configFileWatcher?.startWatching(configUrl: configUrl)

}

@MainActor private var _configFileWatcher = ConfigFileWatcher()

@MainActor

func startConfigFileWatcher() {

_configFileWatcher.startWatching(configUrl: configUrl)

}

+ inline startConfigFileWatcher function

Given that you never assign nil to the variable, I don't understand what this initialization ceremony is for

nikitabobko · 2025-10-14T10:22:28Z

Sources/AppBundle/initAppBundle.swift

    if !reloadConfig() {
        check(reloadConfig(forceConfigUrl: defaultConfigUrl))
    }
+    startConfigFileWatcher()


Your just reloaded the config in the lines above. Config reloading starts the file watcher

Why do you need to start it here again?

nikitabobko · 2025-10-14T10:57:48Z

Sources/AppBundle/config/ConfigFileWatcher.swift

+        guard modDate > (lastModificationDate ?? .distantPast) else {
+            return
+        }


What is this guard for? Do we have reasons not to trust the file watcher API?

nikitabobko · 2025-10-14T10:58:35Z

Sources/AppBundle/config/ConfigFileWatcher.swift

+
+        debounceTask?.cancel()
+
+        // Debounce the reload to handle atomic save sequences (write → rename → swap)


to handle atomic save sequences (write → rename → swap)

I don't understand what you mean

What is renamed? What is swapped with what?

nikitabobko · 2025-10-14T10:59:25Z

Sources/AppBundle/config/ConfigFileWatcher.swift

+            try? await Task.sleep(for: .milliseconds(300))
+            guard !Task.isCancelled else { return }


Suggested change

try? await Task.sleep(for: .milliseconds(300))

guard !Task.isCancelled else { return }

try await Task.sleep(for: .milliseconds(300))

The try here is exactly to check for cancellation

nikitabobko · 2025-10-14T10:59:47Z

Sources/AppBundle/config/ConfigFileWatcher.swift

+        debounceTask = Task { @MainActor in
+            try? await Task.sleep(for: .milliseconds(300))
+            guard !Task.isCancelled else { return }
+            // Reload config within a session to ensure layout refresh happens


Useless comment

nikitabobko · 2025-10-14T11:02:05Z

Sources/AppBundle/config/ConfigFileWatcher.swift

+    private var eventStream: FSEventStreamRef?
+    private var watchedFileUrl: URL?
+    private var lastModificationDate: Date?
+    private var debounceTask: Task<Void, Never>?


Do we need our own debouncer? Isn't the 0.3 that you pass to FSEventStreamCreate already sufficient?

zenangst · 2025-10-14T12:32:24Z

@nikitabobko just thought I'd chime in here.
This is what I ended up with in my app Keyboard Cowboy to listen for when things change inside a folder.

Long story short, don't know the implications of differences between using CoreServices contra DispatchSource, but it does seem a whole lot leaner to go for the DispatchSource.

https://github.com/zenangst/KeyboardCowboy/blob/main/App/Sources/Core/FileWatcher.swift

nikitabobko · 2025-10-14T12:53:50Z

@zenangst thanks for the pointer. I am all in for using simpler and safer APIs.

bshowell0 · 2025-10-16T00:27:08Z

I actually tried implementing this with DispatchSource originally, but ran into problems handling editors with atomic writes. Many/most editors nowadays don't write directly to files, instead they write to a temp file and delete/rename the original. Since DispatchSource watches file descriptors, it would "lose" the config if edited with an editor that does this, unless you spawn a new watcher on the new file. This definitely could work, but introduces timing issues and race conditions which I thought would be better to avoid completely. FSEvents uses path-based monitoring instead of file descriptor so handles atomic writes automatically, which is why I switched to using it.

Looking at @zenangst's example, it seems like it uses DispatchSource for monitoring directories but doesn't have to monitor individual files so it doesn't run into the same stale fd issue for that use case. But if anyone notices me misunderstanding that code or any of this please chime in.

Don't have much time to work on this until hopefully this weekend, I will address review comments then, but depending on what you prefer, I can keep the FSEvents implementation (addressing your comments ofc), or switch to DispatchSource and make sure I handle atomic writes manually.

I wasn't really expecting all of these complexities when I started working on this issue so I appreciate the patience and detailed review comments @nikitabobko :)

nikitabobko · 2025-10-18T15:23:45Z

I actually tried implementing this with DispatchSource originally, but ran into problems handling editors with atomic writes. [...]

Thanks for the explanation. Please, make sure to mention it somewhere in the implementation as a comment.

I'm fine to use this ugly API if it provides the benefit of watching paths instead of file descriptors.

bshowell0 added 3 commits October 13, 2025 00:41

Add auto-reload-config to default config docs

e9ef41c

- Updated `default-config.toml` to include `auto-reload-config` with a comment describing its effect - Didn't give it it's own section in the docs, since, similar to `start-at-login`, it's a config-only feature that is self-explanatory

Fixed formatting issue

37d27eb

Didn't know about ./format.sh, caught by build-debug (macos-14) CI

nikitabobko reviewed Oct 14, 2025

View reviewed changes


		debounceTask?.cancel()

		// Debounce the reload to handle atomic save sequences (write → rename → swap)

		try? await Task.sleep(for: .milliseconds(300))
		guard !Task.isCancelled else { return }

	try? await Task.sleep(for: .milliseconds(300))
	guard !Task.isCancelled else { return }
	try await Task.sleep(for: .milliseconds(300))

Uh oh!

Auto reload config #1743

Are you sure you want to change the base?

Auto reload config #1743

Uh oh!

Conversation

bshowell0 commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

nikitabobko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zenangst commented Oct 14, 2025

Uh oh!

nikitabobko commented Oct 14, 2025

Uh oh!

bshowell0 commented Oct 16, 2025

Uh oh!

nikitabobko commented Oct 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bshowell0 commented Oct 13, 2025 •

edited

Loading