Skip to content

SNTConfigurator: skip override-file watcher when overrides disabled#936

Merged
mlw merged 1 commit into
mainfrom
mlw/fix-flaky-daemon-cfg-bundle-test
May 8, 2026
Merged

SNTConfigurator: skip override-file watcher when overrides disabled#936
mlw merged 1 commit into
mainfrom
mlw/fix-flaky-daemon-cfg-bundle-test

Conversation

@mlw
Copy link
Copy Markdown
Contributor

@mlw mlw commented May 8, 2026

The DEBUG-only override-file watcher was dispatched onto a global queue unconditionally and held self in a polling loop. Under xctest's --runs_per_test, the worker block survived bundle teardown and aborted on objc_msgSend to a now-unloaded class, manifesting as a 1/25 flake in DaemonConfigBundleTest.

Gate the dispatch on the same xctest+ENABLE_CONFIG_OVERRIDES check that already short-circuits applyOverrides:, lifted into a shared overridesEnabled helper so both call sites can't drift.

The DEBUG-only override-file watcher was dispatched onto a global queue
unconditionally and held `self` in a polling loop. Under xctest's
`--runs_per_test`, the worker block survived bundle teardown and aborted
on `objc_msgSend` to a now-unloaded class, manifesting as a 1/25 flake
in DaemonConfigBundleTest.

Gate the dispatch on the same xctest+ENABLE_CONFIG_OVERRIDES check that
already short-circuits applyOverrides:, lifted into a shared
overridesEnabled helper so both call sites can't drift.
@mlw mlw added this to the 2026.4 milestone May 8, 2026
@github-actions github-actions Bot added configurator Issues or PRs related to the configurator / Santa configuration lang/objc++ PRs modifying files in ObjC++ comp/common size/s Size: small labels May 8, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 00adb322-e446-480a-a043-c4f95a876e32

📥 Commits

Reviewing files that changed from the base of the PR and between 801d6c9 and 924cdff.

📒 Files selected for processing (1)
  • Source/common/SNTConfigurator.mm

📝 Walkthrough

Walkthrough

SNTConfigurator refactors debug-only config override gating by extracting gate logic into a new overridesEnabled helper method. This helper centralizes DEBUG/xctest/environment checks. Both applyOverrides: and startWatchingDefaults now use this helper instead of embedding gate logic inline.

Changes

Config Override Gating Refactor

Layer / File(s) Summary
Override Gate Helper
Source/common/SNTConfigurator.mm
New overridesEnabled instance method determines whether the config override mechanism is active in the current process, respecting DEBUG build flag and ENABLE_CONFIG_OVERRIDES environment variable under xctest.
ApplyOverrides Gate
Source/common/SNTConfigurator.mm
applyOverrides: replaces inline DEBUG/xctest conditional checks with an early-return guard that calls overridesEnabled.
Conditional Watcher Setup
Source/common/SNTConfigurator.mm
startWatchingDefaults conditionally initiates the overrides-file watcher asynchronously only when overridesEnabled returns true, instead of always starting the watcher under DEBUG.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: preventing the override-file watcher from starting when overrides are disabled, which is the core fix for the flaky test.
Description check ✅ Passed The description clearly explains the problem (flaky test under xctest), the root cause (unconditional watcher dispatch), and the solution (gating dispatch with shared helper).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mlw/fix-flaky-daemon-cfg-bundle-test

Comment @coderabbitai help to get the list of available commands and usage tips.

@mlw mlw marked this pull request as ready for review May 8, 2026 13:19
@mlw mlw requested a review from a team as a code owner May 8, 2026 13:19
@mlw mlw enabled auto-merge (squash) May 8, 2026 13:40
@mlw mlw merged commit 2cbce6a into main May 8, 2026
8 checks passed
@mlw mlw deleted the mlw/fix-flaky-daemon-cfg-bundle-test branch May 8, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/common configurator Issues or PRs related to the configurator / Santa configuration lang/objc++ PRs modifying files in ObjC++ size/s Size: small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants