Skip to content

fix(macos): stop config tests counting unrelated notifications#103481

Merged
steipete merged 1 commit into
mainfrom
codex/fix-configstore-notification-flake
Jul 10, 2026
Merged

fix(macos): stop config tests counting unrelated notifications#103481
steipete merged 1 commit into
mainfrom
codex/fix-configstore-notification-flake

Conversation

@steipete

@steipete steipete commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #103477
Related: #102637

What Problem This Solves

The macOS Swift suite could fail ConfigStoreTests when another publisher emitted the same config-change notification during the focused save assertion. The failure was timing-dependent: the original exact-head macOS run observed two notifications on its first attempt, then passed on retry.

Why This Change Was Made

Production keeps its existing sender contract: ConfigStore still posts .openclawConfigDidChange with a nil object on NotificationCenter.default. The focused tests now inject a private notification center through the existing DEBUG-only test overrides and deliberately race a competing nil-sender notification on the default center. This proves the save success path announces exactly once, the failure path does not announce, and the production nil-sender contract remains unchanged. A shared override helper also guarantees cleanup on both successful and throwing test paths.

User Impact

There is no user-visible behavior change. This is a test-robustness change that removes cross-test notification contamination without weakening the assertions.

Evidence

  • Before fix: exact-head macOS CI run 29073655164, job 86300486692 failed attempt 1 at ConfigStoreTests.swift:60 because changeCount was 2 instead of 1; the automatic retry passed.
  • Contract audit: ConfigStore and AppState both publish .openclawConfigDidChange with a nil sender, while SettingsRootView intentionally subscribes without a sender filter. Foundation documents that a nil observer object accepts notifications from any sender.
  • Blacksmith Testbox pnpm check:changed passed on lease tbx_01kx5sc6mejkzymc4qvxaq7j3g: all five selected lanes passed (171 tests passed, 37 skipped).
  • swiftc -frontend -parse passed for both touched Swift files.
  • swiftformat --lint --config config/swiftformat passed for both touched Swift files.
  • git diff --check origin/main...HEAD passed.
  • Fresh branch autoreview against base 29d018f0af5e92ff1c131f08dd9308e6c9e38e59 was clean with no accepted/actionable findings.
  • Exact-head CI run 29086898086 passed on da2009f015437c759cd5bba3ab97fcce822d4fb6, including macos-swift job 86342723880: lint, release build, dependency trait check, and the first full Swift test attempt passed with 781 tests in 123 suites. macos-node, native-i18n, security-fast, and preflight also passed.

Exact-head PR macOS CI is the authoritative full Swift build/test proof.

AI-assisted: diagnosis, implementation, and validation were prepared with Codex; the linked issue and hosted failure provide the original evidence.

@openclaw-barnacle openclaw-barnacle Bot added app: macos App: macos size: XS maintainer Maintainer-authored PR labels Jul 10, 2026
@steipete

steipete commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Current-head verification for 12949abd04a35b1e82fcf961ecd7d8d6c3467816:

  • Exact-head CI run 29074344132 is green: preflight, security-fast, native-i18n, macos-node, and macos-swift all succeeded.
  • macos-swift job 86302578422 passed the release build, SwiftLint, SwiftFormat, and 778 tests across 123 suites on the first test attempt. The focused assertions both passed:
    • save routes to remote in remote mode
    • failed save does not announce config change
  • Blacksmith Testbox pnpm check:changed passed on lease tbx_01kx5bkx98sc1shprjy8f4f1cf with 167 relevant tests passing and 37 skipped; the lease was stopped after proof.
  • swiftc -frontend -parse passed for both touched Swift files.
  • swiftformat --lint --config config/swiftformat passed for both touched Swift files.
  • git diff --check origin/main...HEAD passed.
  • Fresh mandatory autoreview was clean with no accepted/actionable findings.

Root-cause proof: Foundation's NotificationCenter contract defines a nil observer object as accepting any sender. The same-name sender probe produced all=3 filtered=1, and current source has AppState plus ConfigStore publishers. A stable ConfigStore sender preserves app-wide nil-filter delivery while isolating these focused assertions.

Landing remains intentionally held behind #103393 and the remaining #103427 fix in #103457. After both land, update once onto current main and rerun exact-head macOS CI before merge.

@steipete steipete self-assigned this Jul 10, 2026
@steipete
steipete force-pushed the codex/fix-configstore-notification-flake branch from 12949ab to da2009f Compare July 10, 2026 10:37
@steipete

steipete commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Land-ready proof for exact head da2009f015437c759cd5bba3ab97fcce822d4fb6:

  • Production behavior is unchanged: ConfigStore still posts .openclawConfigDidChange on NotificationCenter.default with a nil object. The new injection is DEBUG-only and used by focused tests.
  • The regression tests deliberately overlap a competing nil-sender default-center publisher, then assert one ConfigStore success announcement, no failure announcement, and the preserved nil-sender contract.
  • Blacksmith Testbox pnpm check:changed passed on lease tbx_01kx5sc6mejkzymc4qvxaq7j3g across five lanes: 171 passed, 37 skipped.
  • swiftc -frontend -parse and SwiftFormat lint passed for both touched Swift files; git diff --check origin/main...HEAD passed.
  • Fresh branch autoreview was clean with no accepted/actionable findings. Repo-native review artifacts validated with recommendation READY FOR /prepare-pr and zero findings.
  • Exact-head CI run 29086898086 passed. macos-swift job 86342723880 passed lint, release build, dependency trait validation, and its first full test attempt: 781 tests in 123 suites, including ConfigStoreTests. macos-node, native-i18n, security-fast, and preflight passed too.
  • OPENCLAW_TESTBOX=1 scripts/pr prepare-run 103481 accepted the exact hosted gates at this head. Changelog correctly not required for this test-only change.

Known proof gaps: none.

@steipete
steipete merged commit 6eb9a5a into main Jul 10, 2026
63 of 66 checks passed
@steipete
steipete deleted the codex/fix-configstore-notification-flake branch July 10, 2026 10:47
@steipete

steipete commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@steipete steipete mentioned this pull request Jul 10, 2026
Closed
@github-actions github-actions Bot mentioned this pull request Jul 10, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 11, 2026
@steipete
test(macos): isolate config change notifications (openclaw#103481)
92296ca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

app: macos App: macos maintainer Maintainer-authored PR size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: macOS ConfigStore test counts unrelated notifications

1 participant

@steipete