Conversation
📝 WalkthroughWalkthroughUpdates project to Xcode 2630, version 1.2.3 with strict Swift concurrency enabled. Migrates AppDelegate from completion-based callbacks to async/await. Adds Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
TOASTER-iOS/Present/AddLink/AddLinkViewController.swift (1)
405-410: Consider handling task cancellation explicitly.Using
try?onTask.sleepsilently ignoresCancellationError. If the task is cancelled (e.g., user dismisses the view during loading), the save request will still be sent after the delay completes, which may not be the intended behavior.Additionally, this unstructured
Taskcannot be cancelled externally since it's not stored. If cancellation support is needed (e.g., view dismissal should abort the save), consider storing the task handle.♻️ Suggested approach to respect cancellation
task: { _ in Task { - try? await Task.sleep(for: .seconds(0.5)) - await MainActor.run { [weak self] in - self?.requestSaveLink.send() + do { + try await Task.sleep(for: .seconds(0.5)) + await MainActor.run { [weak self] in + self?.requestSaveLink.send() + } + } catch { + // Task was cancelled; do not proceed with save } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TOASTER-iOS/Present/AddLink/AddLinkViewController.swift` around lines 405 - 410, The Task sleeping before calling requestSaveLink currently swallows CancellationError and is untrackable; change it to store the Task in a property on AddLinkViewController (e.g., a saved Task handle like saveDelayTask), use try await Task.sleep(...) without `try?` and handle CancellationError by returning early (or check Task.isCancelled after sleep) before calling self?.requestSaveLink.send(), and cancel that stored task at appropriate lifecycle events (e.g., viewWillDisappear or deinit) so the delayed save can be aborted when the view is dismissed.TOASTER-iOS/Present/Remind/ViewModel/RemindViewModel.swift (1)
80-99: Inconsistent main thread handling infetchAlarmCheck.The
.notDeterminedcase (lines 84-87) wraps its state mutations inDispatchQueue.main.async, but.deniedand.authorizedcases (lines 88-91) setdeviceAlarmSettingdirectly on the callback's background thread. This inconsistency may lead to the thread-safety issue mentioned above.Consider either:
- Wrapping all cases in
DispatchQueue.main.asyncfor consistency, or- Annotating the class with
@MainActorand usingTask {@mainactorin ... }for the callback body.♻️ Suggested consistent approach
func fetchAlarmCheck() { UNUserNotificationCenter.current().getNotificationSettings { permission in - switch permission.authorizationStatus { - case .notDetermined: - DispatchQueue.main.async { + DispatchQueue.main.async { + switch permission.authorizationStatus { + case .notDetermined: self.remindViewType = .deviceOnAppOnNoneData self.bottomSheetAction?() + case .denied: + self.deviceAlarmSetting = false + case .authorized: + self.deviceAlarmSetting = true + default: + print("unknown Error") } - case .denied: - self.deviceAlarmSetting = false - case .authorized: - self.deviceAlarmSetting = true - default: - print("unknown Error") } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TOASTER-iOS/Present/Remind/ViewModel/RemindViewModel.swift` around lines 80 - 99, In fetchAlarmCheck, you're mutating UI/state from the UNUserNotificationCenter.getNotificationSettings callback inconsistently: only the .notDetermined branch uses DispatchQueue.main.async while .denied and .authorized update deviceAlarmSetting off the main thread; fix by ensuring all state/UI mutations happen on the main thread—either wrap the entire switch body in DispatchQueue.main.async (so .notDetermined, .denied, .authorized all set remindViewType/deviceAlarmSetting there) or mark the view model `@MainActor` and run the callback body on the main actor (e.g., Task { `@MainActor` in ... }) so deviceAlarmSetting and remindViewType updates are always main-thread safe.TOASTER-iOS/Global/Resources/Config.swift (1)
26-31: Replace[String: Any]cache with a typed[String: String]to eliminatenonisolated(unsafe).All 10 callers cast dictionary values to
Stringbefore use. Changing the cache type from[String: Any]to[String: String](line 26) removes the need for the unsafe annotation while maintaining identical behavior:nonisolated(unsafe) private static let infoDictionary: [String: String] = { let dict = Bundle.main.infoDictionary ?? [:] return dict.mapValues { String(describing: $0) } }()Alternatively, remove the cache entirely and use
Bundle.main.object(forInfoDictionaryKey:)directly in each accessor if one-time reads are acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TOASTER-iOS/Global/Resources/Config.swift` around lines 26 - 31, The cached static infoDictionary currently uses nonisolated(unsafe) with type [String: Any]; change it to a typed [String: String] so the unsafe annotation can be removed: initialize infoDictionary by reading Bundle.main.infoDictionary (or defaulting to [:]) and map all values to strings (e.g., via mapValues { String(describing: $0) }), keeping the identifier infoDictionary and callers that expect String values unchanged; alternatively, remove the cache and replace uses with Bundle.main.object(forInfoDictionaryKey:) if you prefer one-off reads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@TOASTER-iOS/Application/AppDelegate.swift`:
- Around line 53-58: The async Task updating isLogin can race with scene setup;
make isLogin an observable state and notify consumers instead of silently
setting it: change isLogin to a `@Published` (or `@MainActor` `@Published`) property
on AppDelegate, keep checkAppleLogin() async, but after awaiting its result
publish the value (or post a NotificationCenter notification like
.loginStateUpdated) from the Task, and update SceneDelegate (or other consumers)
to observe AppDelegate.shared.$isLogin or subscribe to the notification and
defer UI/setup until the observed value arrives; ensure SceneDelegate uses the
published subscription (or notification callback) to react to login state rather
than reading isLogin synchronously during didFinishLaunchingWithOptions.
- Around line 149-164: The notification handler currently calls
completionHandler() immediately after spawning a Task, which can return to the
system before the UI navigation (popToRootViewController and
tabBarController.selectedIndex assignment) finishes; fix it by moving the
completionHandler() into the Task so it runs after the UI work completes (e.g.,
perform the UIWindowScene lookup and calls to
ToasterNavigationController.popToRootViewController(...) and
TabBarController.selectedIndex = 0 on the `@MainActor` Task and then call
completionHandler() at the end), or alternatively await the Task's completion
before calling completionHandler() so the UI updates finish first.
In `@TOASTER-iOS/Network/Base/APIInterceptor.swift`:
- Line 19: Remove the nonisolated(unsafe) private var isTokenRefreshed and all
uses of that shared mutable flag in APIInterceptor; instead have adapt() always
read the current token from KeyChain (or the secure token store) at the start of
each request and apply it, and have retry() perform the refresh and persist the
new token back to KeyChain (without toggling any shared boolean). Update adapt()
and retry() so adapt() only reads KeyChain for the token, and retry() writes the
refreshed token to KeyChain (and triggers any necessary re-attempts) to
eliminate the race between adapt() and retry().
In `@TOASTER-iOS/Network/Base/NetworkService.swift`:
- Line 12: Replace the unsafe opt-out by making NetworkService actor-isolated or
by proving Sendable for all contained services; specifically remove
nonisolated(unsafe) static let shared = NetworkService() and instead (a) convert
NetworkService to an actor (actor NetworkService) and expose a single isolated
shared instance, or (b) ensure every concrete service (AuthAPIService,
UserAPIService, ToasterAPIService, ClipAPIService, SearchAPIService,
TimerAPIService, PopupAPIService and their BaseAPIService superclass) and their
MoyaProvider properties conform to Sendable and are implemented thread-safe,
then reintroduce a Sendable-safe shared; update callers to await or hop to the
actor as needed to preserve isolation.
In
`@TOASTER-iOS/Present/DetailClip/View/Component/ChangeClipBottomSheetView.swift`:
- Around line 112-113: The Task block in ChangeClipBottomSheetView.swift uses
Task.sleep(for: .seconds(0.5)) which requires iOS 16+, breaking compilation for
iOS 15 deployment; replace that call with Task.sleep(nanoseconds: 500_000_000)
or wrap the entire Task { ... } block in an `@available`(iOS 16.0, *) guard so the
code only runs on iOS 16+, updating the Task invocation inside the async context
where Task.sleep is called to use the nanoseconds variant if you want iOS 15
compatibility.
In `@TOASTER-iOS/Present/LinkWeb/ViewController/LinkWebViewController.swift`:
- Around line 141-147: The KVO callback for estimatedProgress (the observation
stored in progressObservation created via observe(\.estimatedProgress)) must not
call MainActor.assumeIsolated because the callback may run off the main actor;
replace it with an explicit main-actor hop: inside the observer closure, remove
MainActor.assumeIsolated and dispatch the UI update to the main actor (e.g.,
start a Task { `@MainActor` in ... } or DispatchQueue.main.async) then set
self?.progressView.progress using Float(object.estimatedProgress); keep the weak
self capture and the rest of the observation logic in LinkWebViewController
unchanged.
---
Nitpick comments:
In `@TOASTER-iOS/Global/Resources/Config.swift`:
- Around line 26-31: The cached static infoDictionary currently uses
nonisolated(unsafe) with type [String: Any]; change it to a typed [String:
String] so the unsafe annotation can be removed: initialize infoDictionary by
reading Bundle.main.infoDictionary (or defaulting to [:]) and map all values to
strings (e.g., via mapValues { String(describing: $0) }), keeping the identifier
infoDictionary and callers that expect String values unchanged; alternatively,
remove the cache and replace uses with Bundle.main.object(forInfoDictionaryKey:)
if you prefer one-off reads.
In `@TOASTER-iOS/Present/AddLink/AddLinkViewController.swift`:
- Around line 405-410: The Task sleeping before calling requestSaveLink
currently swallows CancellationError and is untrackable; change it to store the
Task in a property on AddLinkViewController (e.g., a saved Task handle like
saveDelayTask), use try await Task.sleep(...) without `try?` and handle
CancellationError by returning early (or check Task.isCancelled after sleep)
before calling self?.requestSaveLink.send(), and cancel that stored task at
appropriate lifecycle events (e.g., viewWillDisappear or deinit) so the delayed
save can be aborted when the view is dismissed.
In `@TOASTER-iOS/Present/Remind/ViewModel/RemindViewModel.swift`:
- Around line 80-99: In fetchAlarmCheck, you're mutating UI/state from the
UNUserNotificationCenter.getNotificationSettings callback inconsistently: only
the .notDetermined branch uses DispatchQueue.main.async while .denied and
.authorized update deviceAlarmSetting off the main thread; fix by ensuring all
state/UI mutations happen on the main thread—either wrap the entire switch body
in DispatchQueue.main.async (so .notDetermined, .denied, .authorized all set
remindViewType/deviceAlarmSetting there) or mark the view model `@MainActor` and
run the callback body on the main actor (e.g., Task { `@MainActor` in ... }) so
deviceAlarmSetting and remindViewType updates are always main-thread safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e17f2103-20c4-42e3-81a0-a52fb17b7b15
📒 Files selected for processing (27)
TOASTER-iOS.xcodeproj/project.pbxprojTOASTER-iOS.xcodeproj/xcshareddata/xcschemes/TOASTER-iOS.xcschemeTOASTER-iOS/Application/AppDelegate.swiftTOASTER-iOS/Application/Coordinator/Coordinator.swiftTOASTER-iOS/Application/Coordinator/CoordinatorFinishOutput.swiftTOASTER-iOS/Application/Coordinator/Coordinators/TabBarCoordinator.swiftTOASTER-iOS/Application/Coordinator/Factory/CoordinatorFactory.swiftTOASTER-iOS/Application/Coordinator/Factory/ViewControllerFactory.swiftTOASTER-iOS/Application/Coordinator/Router.swiftTOASTER-iOS/Global/Resources/Config.swiftTOASTER-iOS/Network/Base/APIInterceptor.swiftTOASTER-iOS/Network/Base/NetworkService.swiftTOASTER-iOS/Present/AddLink/AddLinkViewController.swiftTOASTER-iOS/Present/AddLink/View/SelectClipHeaderView.swiftTOASTER-iOS/Present/Clip/View/Component/AddClipBottomSheetView.swiftTOASTER-iOS/Present/Clip/View/Component/ClipCollectionHeaderView.swiftTOASTER-iOS/Present/DetailClip/View/Cell/DetailClipListCollectionViewCell.swiftTOASTER-iOS/Present/DetailClip/View/Component/ChangeClipBottomSheetView.swiftTOASTER-iOS/Present/Home/View/Cell/UserClipCollectionViewCell.swiftTOASTER-iOS/Present/Home/View/Component/CompositioinalFactory.swiftTOASTER-iOS/Present/LinkWeb/ViewController/LinkWebViewController.swiftTOASTER-iOS/Present/Login/Adapter/KakaoAuthenticateAdapter.swiftTOASTER-iOS/Present/Remind/View/Component/RemindAlarmOffBottomSheetView.swiftTOASTER-iOS/Present/Remind/View/Component/RemindTimerEditBottomSheetView.swiftTOASTER-iOS/Present/Remind/ViewModel/RemindViewModel.swiftTOASTER-iOS/Present/RemindAdd/TimerAdd/View/TimerRepeatBottomSheetView.swiftTOASTER-iOS/Present/UpdateAlert/UpdateAlertManager.swift
💤 Files with no reviewable changes (1)
- TOASTER-iOS/Present/Login/Adapter/KakaoAuthenticateAdapter.swift
| Task { [weak self] in | ||
| let result = await self?.checkAppleLogin() ?? false | ||
| await MainActor.run { [weak self] in | ||
| self?.isLogin = result | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential race condition with isLogin state.
The isLogin property is set asynchronously inside a Task, but didFinishLaunchingWithOptions returns true immediately. If SceneDelegate or other code reads isLogin before the Apple login check completes, it may see a stale value.
Consider using a more robust pattern such as:
- A
@Publishedproperty with async observation - A completion-based initialization flow
- Delaying scene setup until login state is determined
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TOASTER-iOS/Application/AppDelegate.swift` around lines 53 - 58, The async
Task updating isLogin can race with scene setup; make isLogin an observable
state and notify consumers instead of silently setting it: change isLogin to a
`@Published` (or `@MainActor` `@Published`) property on AppDelegate, keep
checkAppleLogin() async, but after awaiting its result publish the value (or
post a NotificationCenter notification like .loginStateUpdated) from the Task,
and update SceneDelegate (or other consumers) to observe
AppDelegate.shared.$isLogin or subscribe to the notification and defer UI/setup
until the observed value arrives; ensure SceneDelegate uses the published
subscription (or notification callback) to react to login state rather than
reading isLogin synchronously during didFinishLaunchingWithOptions.
| nonisolated func userNotificationCenter(_ center: UNUserNotificationCenter, | ||
| didReceive response: UNNotificationResponse, | ||
| withCompletionHandler completionHandler: @escaping () -> Void) { | ||
| Task { @MainActor in | ||
| if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene, | ||
| let navigationViewController = windowScene.windows | ||
| .first(where: { $0.isKeyWindow })? | ||
| .rootViewController as? ToasterNavigationController { | ||
| navigationViewController.popToRootViewController(animated: false) | ||
| if let tabBarController = navigationViewController.topViewController as? TabBarController { | ||
| tabBarController.selectedIndex = 0 | ||
| } | ||
| } | ||
| } | ||
| completionHandler() | ||
| } |
There was a problem hiding this comment.
completionHandler called before UI work completes.
The completionHandler() at line 163 is called immediately after spawning the Task, not after the UI navigation completes. This signals to the system that notification handling is finished while the navigation may still be in progress.
Consider awaiting the Task completion or moving the completion handler inside the Task:
🐛 Proposed fix
nonisolated func userNotificationCenter(_ center: UNUserNotificationCenter,
didReceive response: UNNotificationResponse,
withCompletionHandler completionHandler: `@escaping` () -> Void) {
Task { `@MainActor` in
if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene,
let navigationViewController = windowScene.windows
.first(where: { $0.isKeyWindow })?
.rootViewController as? ToasterNavigationController {
navigationViewController.popToRootViewController(animated: false)
if let tabBarController = navigationViewController.topViewController as? TabBarController {
tabBarController.selectedIndex = 0
}
}
+ completionHandler()
}
- completionHandler()
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TOASTER-iOS/Application/AppDelegate.swift` around lines 149 - 164, The
notification handler currently calls completionHandler() immediately after
spawning a Task, which can return to the system before the UI navigation
(popToRootViewController and tabBarController.selectedIndex assignment)
finishes; fix it by moving the completionHandler() into the Task so it runs
after the UI work completes (e.g., perform the UIWindowScene lookup and calls to
ToasterNavigationController.popToRootViewController(...) and
TabBarController.selectedIndex = 0 on the `@MainActor` Task and then call
completionHandler() at the end), or alternatively await the Task's completion
before calling completionHandler() so the UI updates finish first.
| /// 해당 프로퍼티가 true 일 경우 기존의 Requset 의 Header 를 변경 | ||
| /// 재발급을 받은 적이 없는 false 의 경우 TargetType 을 통해 Header 를 설정한 Requset 를 기본으로 사용 | ||
| private var isTokenRefreshed = false | ||
| nonisolated(unsafe) private var isTokenRefreshed = false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "APIInterceptor.swift" -type fRepository: Link-MIND/TOASTER-iOS
Length of output: 112
🏁 Script executed:
cat -n TOASTER-iOS/Network/Base/APIInterceptor.swiftRepository: Link-MIND/TOASTER-iOS
Length of output: 4203
Remove the nonisolated(unsafe) flag and the shared mutable refresh state entirely.
The singleton APIInterceptor has a genuine data race: adapt() reads and clears isTokenRefreshed at lines 31–36, while retry() writes it at line 72. When multiple requests retry concurrently after a 401, the first adapt() clears the flag, causing subsequent overlapping retries to skip the token refresh. The nonisolated(unsafe) annotation only silences the compiler warning—it doesn't prevent the race.
Suggested fix
Remove the flag and instead read the token from KeyChain on every request:
final class APIInterceptor: RequestInterceptor {
- nonisolated(unsafe) private var isTokenRefreshed = false
static let shared = APIInterceptor()
func adapt(_ urlRequest: URLRequest, for session: Session, completion: `@escaping` (Result<URLRequest, Error>) -> Void) {
print("adapt 진입")
var modifiedRequest = urlRequest
- if isTokenRefreshed {
- print("토큰 재발급 후 URLRequset 변경")
- modifiedRequest.setValue(KeyChainService.loadAccessToken(key: Config.accessTokenKey), forHTTPHeaderField: "accessToken")
-
- isTokenRefreshed = false
- }
+ modifiedRequest.setValue(KeyChainService.loadAccessToken(key: Config.accessTokenKey), forHTTPHeaderField: "accessToken")
completion(.success(modifiedRequest))
}
func retry(_ request: Request, for session: Session, dueTo error: Error, completion: `@escaping` (RetryResult) -> Void) {
NetworkService.shared.authService.postRefreshToken { [weak self] result in
switch result {
case .success(let result):
guard let serverAccessToken = result?.data.accessToken, let serverRefreshToken = result?.data.refreshToken else {
completion(.doNotRetry)
return
}
let keyChainResult = KeyChainService.saveTokens(accessKey: serverAccessToken, refreshKey: serverRefreshToken)
if keyChainResult.accessResult == true && keyChainResult.refreshResult == true {
- self?.isTokenRefreshed = true
-
guard request.request != nil else {
completion(.doNotRetry)
return
}
completion(.retry)
} else {
completion(.doNotRetry)
}
default:
completion(.doNotRetry)
}
}
}
}This eliminates the race by using KeyChain as the single source of truth for the token, ensuring all concurrent requests use the latest refreshed token.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| nonisolated(unsafe) private var isTokenRefreshed = false | |
| final class APIInterceptor: RequestInterceptor { | |
| static let shared = APIInterceptor() | |
| func adapt(_ urlRequest: URLRequest, for session: Session, completion: `@escaping` (Result<URLRequest, Error>) -> Void) { | |
| print("adapt 진입") | |
| var modifiedRequest = urlRequest | |
| modifiedRequest.setValue(KeyChainService.loadAccessToken(key: Config.accessTokenKey), forHTTPHeaderField: "accessToken") | |
| completion(.success(modifiedRequest)) | |
| } | |
| func retry(_ request: Request, for session: Session, dueTo error: Error, completion: `@escaping` (RetryResult) -> Void) { | |
| NetworkService.shared.authService.postRefreshToken { [weak self] result in | |
| switch result { | |
| case .success(let result): | |
| guard let serverAccessToken = result?.data.accessToken, let serverRefreshToken = result?.data.refreshToken else { | |
| completion(.doNotRetry) | |
| return | |
| } | |
| let keyChainResult = KeyChainService.saveTokens(accessKey: serverAccessToken, refreshKey: serverRefreshToken) | |
| if keyChainResult.accessResult == true && keyChainResult.refreshResult == true { | |
| guard request.request != nil else { | |
| completion(.doNotRetry) | |
| return | |
| } | |
| completion(.retry) | |
| } else { | |
| completion(.doNotRetry) | |
| } | |
| default: | |
| completion(.doNotRetry) | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TOASTER-iOS/Network/Base/APIInterceptor.swift` at line 19, Remove the
nonisolated(unsafe) private var isTokenRefreshed and all uses of that shared
mutable flag in APIInterceptor; instead have adapt() always read the current
token from KeyChain (or the secure token store) at the start of each request and
apply it, and have retry() perform the refresh and persist the new token back to
KeyChain (without toggling any shared boolean). Update adapt() and retry() so
adapt() only reads KeyChain for the token, and retry() writes the refreshed
token to KeyChain (and triggers any necessary re-attempts) to eliminate the race
between adapt() and retry().
| final class NetworkService { | ||
|
|
||
| static let shared = NetworkService() | ||
| nonisolated(unsafe) static let shared = NetworkService() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A40 -B2 --type swift \
'protocol\s+(AuthAPIServiceProtocol|UserAPIServiceProtocol|ToasterAPIServiceProtocol|ClipAPIServiceProtocol|SearchAPIServiceProtocol|TimerAPIServiceProtocol|PopupAPIServiceProtocol)\b|(?:final\s+)?(?:class|actor|struct)\s+(AuthAPIService|UserAPIService|ToasterAPIService|ClipAPIService|SearchAPIService|TimerAPIService|PopupAPIService)\b|@unchecked Sendable|Sendable' \
TOASTER-iOSRepository: Link-MIND/TOASTER-iOS
Length of output: 33752
🏁 Script executed:
#!/bin/bash
# Check BaseAPIService for any Sendable/actor annotations
rg -n -A20 "class BaseAPIService" --type swift TOASTER-iOSRepository: Link-MIND/TOASTER-iOS
Length of output: 1971
🏁 Script executed:
#!/bin/bash
# Check NetworkService.shared full context and declaration
rg -n -B5 -A5 "nonisolated.*shared.*NetworkService" --type swift TOASTER-iOSRepository: Link-MIND/TOASTER-iOS
Length of output: 869
nonisolated(unsafe) suppresses compile-time safety without proof of thread-safety.
The concrete services exposed through shared are reference types without Sendable conformance or actor isolation. Every service (AuthAPIService, UserAPIService, ToasterAPIService, ClipAPIService, SearchAPIService, TimerAPIService, PopupAPIService) is a final class inheriting from non-Sendable BaseAPIService, with a non-Sendable MoyaProvider property. This converts strict concurrency from a compiler guarantee into a manual audit burden. Prove the service graph is thread-safe or use actor isolation instead of opting out of safety checks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TOASTER-iOS/Network/Base/NetworkService.swift` at line 12, Replace the unsafe
opt-out by making NetworkService actor-isolated or by proving Sendable for all
contained services; specifically remove nonisolated(unsafe) static let shared =
NetworkService() and instead (a) convert NetworkService to an actor (actor
NetworkService) and expose a single isolated shared instance, or (b) ensure
every concrete service (AuthAPIService, UserAPIService, ToasterAPIService,
ClipAPIService, SearchAPIService, TimerAPIService, PopupAPIService and their
BaseAPIService superclass) and their MoyaProvider properties conform to Sendable
and are implemented thread-safe, then reintroduce a Sendable-safe shared; update
callers to await or hop to the actor as needed to preserve isolation.
| Task { | ||
| try? await Task.sleep(for: .seconds(0.5)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'IPHONEOS_DEPLOYMENT_TARGET' TOASTER-iOS.xcodeproj/project.pbxproj
rg -n 'Task\.sleep\(for:' TOASTER-iOS/Present/DetailClip/View/Component/ChangeClipBottomSheetView.swiftRepository: Link-MIND/TOASTER-iOS
Length of output: 394
Use Task.sleep(nanoseconds:) or add an @available guard for iOS 16.0+.
The project has deployment targets as low as iOS 15.0, but Task.sleep(for:) requires iOS 16.0+. This code will fail to compile for the iOS 15.0 target. Either replace with Task.sleep(nanoseconds: 500_000_000) (available since iOS 13.0) or wrap the entire Task block in an @available(iOS 16.0, *) guard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@TOASTER-iOS/Present/DetailClip/View/Component/ChangeClipBottomSheetView.swift`
around lines 112 - 113, The Task block in ChangeClipBottomSheetView.swift uses
Task.sleep(for: .seconds(0.5)) which requires iOS 16+, breaking compilation for
iOS 15 deployment; replace that call with Task.sleep(nanoseconds: 500_000_000)
or wrap the entire Task { ... } block in an `@available`(iOS 16.0, *) guard so the
code only runs on iOS 16+, updating the Task invocation inside the async context
where Task.sleep is called to use the nanoseconds variant if you want iOS 15
compatibility.
| progressObservation = $0.observe( | ||
| \.estimatedProgress, | ||
| options: [.new]) { [weak self] object, _ in | ||
| let progress = Float(object.estimatedProgress) | ||
| self?.progressView.progress = progress | ||
| MainActor.assumeIsolated { | ||
| let progress = Float(object.estimatedProgress) | ||
| self?.progressView.progress = progress | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Swift MainActor.assumeIsolated documentation semantics main actor
💡 Result:
MainActor.assumeIsolated is a synchronous “trust me + verify” escape hatch:
-
What it promises (to the compiler): inside the closure, code is treated as running with
@MainActorisolation, so you can make synchronous accesses that would normally require anawaithop. Apple’s docs summarize it as: assume the current task is executing on the main actor’s serial executor, or stop program execution.[1] -
What it checks (at runtime): the stdlib implementation fetches
MainActor.shared’s serial executor and checks that the current task is already executing on that executor. If not, it crashes withfatalError("Incorrect actor executor assumption; Expected same executor as MainActor...").[2] -
How it works (mechanically): after the executor check succeeds, it uses
withoutActuallyEscaping+unsafeBitCastto erase the@MainActorannotation from the closure type and then calls it immediately (no hop, no suspension).[2] -
Important nuance: the check is executor-based, not “main-thread”-based; if some other actor is configured to use the same serial executor as
MainActor, the check can still succeed (this is explicitly documented in the stdlib comments).[2] -
Intended use: only from synchronous code; the API is marked unavailable from async contexts with guidance to “await the call to the
@MainActorclosure directly”.[2]
Sources: Apple Developer Documentation entry for MainActor.assumeIsolated[1], Swift standard library implementation (stdlib/public/Concurrency/MainActor.swift).[2]
🌐 Web query:
NSKeyValueObservation callback main thread execution guarantee iOS
💡 Result:
NSKeyValueObservation (the Swift block-based KVO API) does not guarantee its callback runs on the main thread.
KVO notifications are delivered on the same thread where the observed property change happens—so if the property is mutated on a background thread, your changeHandler will run on that background thread as well. Apple explicitly states this for KVO: “If the change to the property occurs on a secondary thread, the observeValueForKeyPath:… code executes on that same thread.” [1]
If you need to update UI (main-thread-only work), you must hop to the main execution context yourself (e.g. DispatchQueue.main.async { … } / await MainActor.run { … }). Apple discusses using a “Receptionist” style approach to redirect work from a background execution context to the main one for UI updates. [2]
Sources
- Apple Docs – Cocoa Design Patterns (KVO threading note) [1]
- Apple Docs – Streamline Your App with Design Patterns (Receptionist pattern / execution context) [2]
Don't use MainActor.assumeIsolated in this observer.
MainActor.assumeIsolated asserts the callback is already on the main actor—it doesn't hop there. Since NSKeyValueObservation callbacks run on whatever thread the property mutation occurs, this will crash at runtime if estimatedProgress changes on a background thread. Use an explicit main-actor hop instead.
🔧 Safer main-actor hop
progressObservation = $0.observe(
\.estimatedProgress,
- options: [.new]) { [weak self] object, _ in
- MainActor.assumeIsolated {
- let progress = Float(object.estimatedProgress)
- self?.progressView.progress = progress
- }
- }
+ options: [.new]) { [weak self] object, _ in
+ let progress = Float(object.estimatedProgress)
+ Task { `@MainActor` in
+ self?.progressView.progress = progress
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TOASTER-iOS/Present/LinkWeb/ViewController/LinkWebViewController.swift`
around lines 141 - 147, The KVO callback for estimatedProgress (the observation
stored in progressObservation created via observe(\.estimatedProgress)) must not
call MainActor.assumeIsolated because the callback may run off the main actor;
replace it with an explicit main-actor hop: inside the observer closure, remove
MainActor.assumeIsolated and dispatch the UI update to the main actor (e.g.,
start a Task { `@MainActor` in ... } or DispatchQueue.main.async) then set
self?.progressView.progress using Float(object.estimatedProgress); keep the weak
self capture and the rest of the observation logic in LinkWebViewController
unchanged.
| private var remindViewType: RemindViewType = .deviceOnAppOnNoneData { | ||
| didSet { | ||
| DispatchQueue.main.async { | ||
| self.dataChangeAction?(self.remindViewType) | ||
| } | ||
| self.dataChangeAction?(self.remindViewType) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how dataChangeAction is used and whether the caller handles main thread dispatch
echo "=== Searching for setupDataChangeAction call sites ==="
rg -n -A 10 'setupDataChangeAction' --type swift
echo ""
echo "=== Checking if RemindViewModel or related View uses `@MainActor` ==="
rg -n '@MainActor' --type swift -g '*Remind*'Repository: Link-MIND/TOASTER-iOS
Length of output: 3051
🏁 Script executed:
#!/bin/bash
# Check RemindViewController class declaration and `@MainActor` annotation
echo "=== RemindViewController class declaration ==="
rg -n -B 5 -A 20 'class RemindViewController' TOASTER-iOS/Present/Remind/View/RemindViewController.swift | head -40
echo ""
echo "=== Check reloadCollectionViewWithView implementation ==="
rg -n -B 2 -A 10 'reloadCollectionViewWithView' TOASTER-iOS/Present/Remind/View/RemindViewController.swift
echo ""
echo "=== Check other closures passed to setupDataChangeAction ==="
rg -n -B 2 -A 10 'setupAlarmBottomSheet\|deleteAction\|unAuthorizedAction' TOASTER-iOS/Present/Remind/View/RemindViewController.swift | head -80Repository: Link-MIND/TOASTER-iOS
Length of output: 2444
🏁 Script executed:
#!/bin/bash
# Check network callback implementations and their thread contexts
echo "=== Check fetchTimerData, deleteTimerData, patchTimerData callbacks ==="
rg -n -B 5 -A 15 'fetchTimerData|deleteTimerData|patchTimerData' TOASTER-iOS/Present/Remind/ViewModel/RemindViewModel.swift | head -150
echo ""
echo "=== Check getNotificationSettings implementation ==="
rg -n -B 5 -A 20 'func getNotificationSettings' TOASTER-iOS/Present/Remind/ViewModel/RemindViewModel.swiftRepository: Link-MIND/TOASTER-iOS
Length of output: 3780
🏁 Script executed:
#!/bin/bash
# Search for getNotificationSettings more broadly
echo "=== Search for getNotificationSettings in all Swift files ==="
rg -n 'getNotificationSettings' --type swift
echo ""
echo "=== Look at the RemindViewModel file to find deviceAlarmSetting didSet ==="
rg -n -B 2 -A 10 'deviceAlarmSetting' TOASTER-iOS/Present/Remind/ViewModel/RemindViewModel.swift | head -50Repository: Link-MIND/TOASTER-iOS
Length of output: 1481
🏁 Script executed:
#!/bin/bash
# Verify the exact line 28 change and confirm the DispatchQueue was removed
echo "=== View the complete remindViewType property definition ==="
sed -n '26,30p' TOASTER-iOS/Present/Remind/ViewModel/RemindViewModel.swift
echo ""
echo "=== Confirm this is a didSet observer that previously had DispatchQueue ==="
git log -p --all -S 'DispatchQueue.main.async' -- TOASTER-iOS/Present/Remind/ViewModel/RemindViewModel.swift | head -100Repository: Link-MIND/TOASTER-iOS
Length of output: 365
Thread-safety issue: dataChangeAction will execute on background threads.
Removing DispatchQueue.main.async allows the closure to run on whatever thread triggers the remindViewType didSet. Since reloadCollectionViewWithView performs direct UI updates (timerCollectionView.reloadData()), this violates UIKit's requirement that all UI operations occur on the main thread.
The remindViewType property is mutated from background threads in two ways:
-
getNotificationSettingscallback (lines 88–91):UNUserNotificationCenter.current().getNotificationSettings()executes its completion on a background thread, setsdeviceAlarmSetting, which triggerssetupAlarm(), which directly modifiesremindViewType. -
Network callbacks (lines 102–151): Network service completion handlers execute on background threads and modify
timerData, whosedidSetcan updateremindViewType.
Restore the DispatchQueue.main.async wrapper at line 28, or annotate the class with @MainActor if completing a concurrency migration.
🤖 iOS Code Review — ⚖️ Balanced (균형 모드)✅ 리뷰할 이슈가 없습니다. 좋은 코드네요! 📋 심각도 기준 · 감도 변경
감도 변경: 워크플로우의 |
1 participant
✨ 해결한 이슈
🛠️ 작업내용
🖥️ 주요 코드 설명
HomeView📂 참고한 내용
✅ Checklist
Summary by CodeRabbit
Chores
Bug Fixes