refactor(services): inject dependencies into AppSettingsManager and add four singletons to AppServices#1157
Merged
datlechin merged 1 commit intoMay 9, 2026
Conversation
…dd four singletons to AppServices
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AppSettingsManagernow takes its eight cross-singleton dependencies via init, matching the pattern PR #1153 established for storage classes. The 32 raw.sharedreads inside the class are converted to instance properties, including the fourTask { ... }capture closures indidSetblocks that previously dialedCopilotService.shared/MCPServerManager.sharedmid-write.AppServicesgains four fields:queryHistoryManager,dateFormattingService,copilotService,mcpServerManager. Theliveinitializer threads.shareddefaults.AppSettingsManagerinit signature:private init()is dropped and the parameters all default to.shared, soAppSettingsManager.shared = AppSettingsManager()(and any test harness that wants to substitute) both compile.Why individual params, not
services: AppServices:AppServices.liveinitializesappSettings: .shared. DefaultingAppSettingsManager.inittoservices: AppServices = .livewould recursively read.livemid-construction. Individual params avoid the cycle.The 49 caller-side
AppSettingsManager.sharedreads are out of scope here; they ride along when their owners adoptservicesin subsequent DI waves.Why this matters
Audit findings 3.1 + arch 5.1 ("complete the AppServices threading"). After PRs #1151, #1152, #1153, #1155, this is the next-biggest concentration: the central settings hub. Resolving it removes 32 raw
.sharedreads from a singleton that fires on every settings change, and keeps the door open for tests that want to driveAppSettingsManageragainst fakes (e.g. a no-opMCPServerManagersomcp.didSetdoesn't actually start a server).Test plan
applyHistorySettingsImmediatelystill fires.