Skip to content

Handle kFSEventStreamEventFlagMustScanSubDirs to prevent missed UI refreshes#2782

Open
delacrixmorgan wants to merge 1 commit into
git-up:masterfrom
delacrixmorgan:feature/refresh
Open

Handle kFSEventStreamEventFlagMustScanSubDirs to prevent missed UI refreshes#2782
delacrixmorgan wants to merge 1 commit into
git-up:masterfrom
delacrixmorgan:feature/refresh

Conversation

@delacrixmorgan
Copy link
Copy Markdown
Contributor

Summary

The UI occasionally fails to refresh when files change in the working directory or .git directory. This is caused by the FSEvents callback ignoring kFSEventStreamEventFlagMustScanSubDirs events, which the kernel sends when its event buffer overflows during rapid filesystem activity.

Problem

In GCLiveRepository.m, the _stream:didReceiveEvents:withPaths:flags: method previously discarded events flagged with kFSEventStreamEventFlagMustScanSubDirs:

if (eventFlags[i] & kFSEventStreamEventFlagMustScanSubDirs) {
    XLOG_WARNING(@"Ignoring event stream request to rescan \"%s\"", path);
}

Per Apple's FSEvents documentation, this flag means: "Your application must rescan not just the directory given in the event, but all its children, recursively." It fires when the kernel drops or coalesces events due to buffer overflow — typically during rapid multi-file changes (e.g., git checkout, git rebase, IDE refactoring, build tools).

By ignoring this flag, the app missed those change notifications entirely, leaving the UI stale.

Fix

When kFSEventStreamEventFlagMustScanSubDirs is received, we now mark the appropriate stream as changed and schedule the existing debounced update timer:

if (eventFlags[i] & kFSEventStreamEventFlagMustScanSubDirs) {
    XLOG_WARNING(@"Received event stream request to rescan \"%s\"", path);
    if (stream == _gitDirectoryStream) {
        _gitDirectoryChanged = YES;
    } else {
        _workingDirectoryChanged = YES;
    }
    CFRunLoopTimerSetNextFireDate(_updateTimer, CFAbsoluteTimeGetCurrent() + kUpdateLatency);
}

This follows the exact same code path as regular file change events — no new logic introduced.

Why This Is Safe

  1. Reuses existing infrastructure — sets the same boolean flags and reschedules the same timer used by all normal events.
  2. Debounced — the 0.5s kUpdateLatency timer coalesces multiple triggers; no duplicate processing.
  3. Idempotent downstream_updateStatus: compares the new diff against the current one (isEqualToDiff:) and only notifies the UI if something actually changed. No spurious redraws.
  4. Rare trigger — this flag only fires on kernel buffer overflow, not on every event. Normal single-file edits are unaffected.

Risk Assessment

Risk Likelihood Impact Mitigation
Extra status recomputation on buffer overflow events Low (flag is rare) Negligible (same cost as any file-save event) Existing 0.5s debounce timer coalesces bursts
UI flicker from unnecessary redraws Very Low Low _updateStatus: already diffs against current state and only notifies on actual changes
Regression in ignore-path filtering None N/A For MustScanSubDirs, we intentionally skip the ignore check — the kernel is telling us events were lost, so we must assume something changed. The downstream status computation will correctly reflect only non-ignored files.
Thread safety concerns None N/A All code runs on the main run loop (FSEventStream is scheduled on CFRunLoopGetMain()), same as before

Overall risk: Very Low. The change is minimal, follows established patterns in the codebase, and Apple's documentation explicitly mandates this behavior.

@Cykelero
Copy link
Copy Markdown
Contributor

Cykelero commented Jun 3, 2026

This is a change I've been thinking of making for a while, now, but I've always been bugged by the fact that I don't know why POL, the original author, wrote it this way to begin with; whether it's a mistake, or a deliberate choice.

The only issue I can imagine this fix causing, would be GitUp adding oil to the fire, when the system is already swamped (which it would be, to reach the point of coalescing file system updates like this). Not sure if that's a reasonable concern, or if e.g. the debouncing (kUpdateLatency) would be enough to avoid the issue.

Can you describe when you've hit issues, that you think this patch would resolve? Have you been able to confirm that this fix helps, or is it a best guess at a solution? Having a reliable repro would be a great first step to evaluate the change.

@delacrixmorgan
Copy link
Copy Markdown
Contributor Author

I have to admit that I can't consistently recreate this issue, but it happened twice this week for me where I will need to close the application in order to see the latest code changes, else it'll just at an outdated state.

When I added this fix, I didn't notice any breaking changes.

@lucasderraugh
Copy link
Copy Markdown
Collaborator

Thanks @delacrixmorgan. I've experienced this myself in the Tahoe beta build. I have been meaning to debug why the UI doesn't appear to redraw in these cases and it's the main reason I haven't made the build the stable build yet.

I'll review your changes tonight and see if I can track the refresh cycle with logs to see if I can better understand if this is resolving that issue or if there's something else going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants