Conversation
feat: support manage limit time
feat: improve checked icon for active account and update icon app
WalkthroughAdds a new menu-bar UI (MenuBarLabelView) with per-account controls and real‑time countdowns powered by MenuBarState and Account.time_limit; extends AccountManager with timer, updateTimeLimit, switchAccountApplyingLimitToCurrent, and checkQuota APIs. Introduces TimeLimitFormatter, QuotaService with QuotaSnapshot, and quota UI flows. Adds Shell utilities, LocalhostInsecureURLSessionDelegate, ProgressPanel, ProcessManager. Modifies DB restore to clear absent keys, expands AppConfiguration backup keys and bundle IDs, extends AppError, adds build pre-clean and AppIcon plist entry, VS Code launch configs, a verification script, and a new terminate/cleanup routine. Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/icon.pngis excluded by!**/*.png
📒 Files selected for processing (4)
Sources/AntigravityMenuBar/AntigravityApp.swift(5 hunks)Sources/AntigravityMenuBar/Logic/AccountManager.swift(2 hunks)Sources/AntigravityMenuBar/Utils/TimeLimitFormatter.swift(1 hunks)build.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
Sources/AntigravityMenuBar/Logic/AccountManager.swift (2)
Sources/AntigravityMenuBar/Utils/DBManager.swift (1)
getCurrentAccountEmail(129-143)Sources/AntigravityMenuBar/Utils/TimeLimitFormatter.swift (1)
formatMenuBarCountdown(64-85)
Sources/AntigravityMenuBar/AntigravityApp.swift (2)
Sources/AntigravityMenuBar/Logic/AccountManager.swift (2)
updateTimeLimit(255-269)removeAccount(235-253)Sources/AntigravityMenuBar/Utils/TimeLimitFormatter.swift (4)
formatCountdown(22-43)defaultTimeString(55-59)defaultDateString(47-51)parse(11-17)
🪛 SwiftLint (0.57.0)
Sources/AntigravityMenuBar/Logic/AccountManager.swift
[Warning] 24-24: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (5)
build.sh (1)
38-39: The CFBundleIconFile entry is not present in the current build.sh file.Lines 38-39 contain
<key>LSUIElement</key>and<true/>, not theCFBundleIconFileentry shown in the review comment snippet. The Info.plist in the current build.sh does not include an icon reference. If addingCFBundleIconFileis intended, ensure it is properly added to the plist generation in build.sh (around lines 23-43).Likely an incorrect or invalid review comment.
Sources/AntigravityMenuBar/Logic/AccountManager.swift (2)
13-18: LGTM on MenuBarState separation.Good design choice to separate the countdown state into its own
ObservableObject. This prevents triggering full menu rebuilds on every countdown tick.
255-269: LGTM onupdateTimeLimitimplementation.The method correctly handles both setting and clearing the time limit, with proper persistence.
Sources/AntigravityMenuBar/AntigravityApp.swift (2)
3-15: LGTM on MenuBarLabelView.Clean implementation that observes the shared countdown state and displays it alongside the icon.
43-52: Good conditional UI for Reset vs Update Time Limit.The logic correctly shows "Reset" only when there's an active countdown and "Update Time Limit" otherwise. This provides clear user feedback.
| // if account.email == "Unknown" { | ||
| // Image(systemName: "person.circle") | ||
| // } else { | ||
| // Image(systemName: "person.fill") | ||
| // } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
Dead code should be removed to maintain code cleanliness. If this logic is needed later, it can be retrieved from version control.
Image(systemName: "checkmark")
.foregroundColor(.green)
}
- // if account.email == "Unknown" {
- // Image(systemName: "person.circle")
- // } else {
- // Image(systemName: "person.fill")
- // }
// Build display name with countdown if applicable
Text(accountDisplayName(account))📝 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.
| // if account.email == "Unknown" { | |
| // Image(systemName: "person.circle") | |
| // } else { | |
| // Image(systemName: "person.fill") | |
| // } | |
| Image(systemName: "checkmark") | |
| .foregroundColor(.green) | |
| } | |
| // Build display name with countdown if applicable | |
| Text(accountDisplayName(account)) |
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/AntigravityApp.swift around lines 66 to 70, remove
the commented-out conditional block that toggles Image(systemName:
"person.circle") vs "person.fill" as it is dead code; delete those commented
lines so the file contains only active code and rely on version control to
recover this logic if needed later.
| private func accountDisplayName(_ account: Account) -> String { | ||
| var displayName = account.name | ||
|
|
||
| // Append countdown if time_limit is set and not expired | ||
| if let timeLimitStr = account.time_limit, | ||
| let timeLimit = ISO8601DateFormatter().date(from: timeLimitStr), | ||
| let countdown = TimeLimitFormatter.formatCountdown(to: timeLimit) { | ||
| displayName += " \(countdown)" | ||
| } | ||
|
|
||
| return displayName | ||
| } | ||
|
|
||
| private func hasActiveCountdown(_ account: Account) -> Bool { | ||
| guard let timeLimitStr = account.time_limit, | ||
| let timeLimit = ISO8601DateFormatter().date(from: timeLimitStr) else { | ||
| return false | ||
| } | ||
| return timeLimit > Date() // Active if not expired | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Cache ISO8601DateFormatter and consider extracting to a shared helper.
Both accountDisplayName and hasActiveCountdown create new ISO8601DateFormatter() instances on each call. This duplicates logic already present in AccountManager.updateMenuBarCountdown. Consider:
- Caching the formatter as a static property
- Adding a helper method on
Accountor inTimeLimitFormatterto parse thetime_limitfield
You could add a computed property to Account:
extension Account {
var timeLimitDate: Date? {
guard let timeLimitStr = time_limit else { return nil }
return ISO8601DateFormatter().date(from: timeLimitStr)
}
}Or better, add a cached formatter in TimeLimitFormatter:
static func parseISO8601(_ string: String) -> Date? {
// Use a cached ISO8601DateFormatter
}🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/AntigravityApp.swift around lines 127 to 146, both
accountDisplayName(_) and hasActiveCountdown(_) create new ISO8601DateFormatter
instances on each call; change this to use a single cached formatter and a
parsing helper: add a shared ISO8601DateFormatter (preferably as a static on
TimeLimitFormatter) and a helper function (e.g.,
TimeLimitFormatter.parseISO8601(_:)) or a computed property on Account
(timeLimitDate) that uses the cached formatter, then replace the inline
ISO8601DateFormatter().date(from:) calls with the new helper so parsing is
centralized and allocations are avoided.
| // Validate and parse | ||
| guard let parsedDate = TimeLimitFormatter.parse(time: timeValue, date: dateValue) else { | ||
| let errorAlert = NSAlert() | ||
| errorAlert.messageText = "Invalid Format" | ||
| errorAlert.informativeText = "Please enter time as HH:mm (e.g., 14:30) and date as dd.MM.yyyy (e.g., 31.12.2026)" | ||
| errorAlert.alertStyle = .warning | ||
| errorAlert.runModal() | ||
| return | ||
| } |
There was a problem hiding this comment.
Consider validating that the time limit is in the future.
The code validates the format but doesn't check if the parsed date is in the future. Setting a time limit in the past would be immediately expired and have no effect.
// Validate and parse
guard let parsedDate = TimeLimitFormatter.parse(time: timeValue, date: dateValue) else {
let errorAlert = NSAlert()
errorAlert.messageText = "Invalid Format"
errorAlert.informativeText = "Please enter time as HH:mm (e.g., 14:30) and date as dd.MM.yyyy (e.g., 31.12.2026)"
errorAlert.alertStyle = .warning
errorAlert.runModal()
return
}
+ guard parsedDate > Date() else {
+ let errorAlert = NSAlert()
+ errorAlert.messageText = "Invalid Date"
+ errorAlert.informativeText = "The time limit must be in the future."
+ errorAlert.alertStyle = .warning
+ errorAlert.runModal()
+ return
+ }
+
accountManager.updateTimeLimit(id: account.id, date: parsedDate)🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/AntigravityApp.swift around lines 223 to 231,
after parsing the date/time you must verify the resulting parsedDate is in the
future; if parsedDate <= Date() show an NSAlert with a clear message (e.g.,
"Time limit must be in the future") and return. Implement the comparison using
Date() (or Calendar.current.compare) to avoid timezone issues and keep the
existing UI flow (alertStyle .warning and runModal()) before returning.
| static let shared = AccountManager() | ||
|
|
||
| @Published var accounts: [Account] = [] | ||
| @Published var currentEmail: String? = nil // Currently active account email |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant nil initialization.
Initializing an optional variable with nil is redundant in Swift.
- @Published var currentEmail: String? = nil // Currently active account email
+ @Published var currentEmail: String? // Currently active account email📝 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.
| @Published var currentEmail: String? = nil // Currently active account email | |
| @Published var currentEmail: String? // Currently active account email |
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 24-24: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Logic/AccountManager.swift at line 24, the
optional property is redundantly initialized to nil; remove the explicit "= nil"
so the declaration reads simply "@Published var currentEmail: String?" and keep
the rest of the code unchanged (verify no other code relies on an explicit
initializer).
| private func startRefreshTimer() { | ||
| // Create timer in common run loop mode so it runs even when menu is open | ||
| refreshTimer = Timer(timeInterval: 1.0, repeats: true) { [weak self] _ in | ||
| self?.updateMenuBarCountdown() | ||
| } | ||
| // Add to common run loop mode for background updates | ||
| RunLoop.main.add(refreshTimer!, forMode: .common) | ||
|
|
||
| // Initial update | ||
| updateMenuBarCountdown() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid force unwrap and consider timer cleanup.
While the force unwrap is technically safe here since the timer was just created, using optional chaining or guard let is more defensive. Also, the timer should be invalidated if AccountManager is ever deallocated (though as a singleton this may not occur in practice).
private func startRefreshTimer() {
- // Create timer in common run loop mode so it runs even when menu is open
- refreshTimer = Timer(timeInterval: 1.0, repeats: true) { [weak self] _ in
+ refreshTimer?.invalidate()
+ let timer = Timer(timeInterval: 1.0, repeats: true) { [weak self] _ in
self?.updateMenuBarCountdown()
}
- // Add to common run loop mode for background updates
- RunLoop.main.add(refreshTimer!, forMode: .common)
+ RunLoop.main.add(timer, forMode: .common)
+ refreshTimer = timer
// Initial update
updateMenuBarCountdown()
}🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Logic/AccountManager.swift around lines 38 to 48,
avoid force-unwrapping refreshTimer and ensure cleanup: after creating the
timer, use a safe binding (e.g., guard let timer = refreshTimer or optional
chaining) when adding it to RunLoop instead of refreshTimer!, and add a
stopRefreshTimer() method that invalidates and nils the timer; also call that
cleanup from deinit (or provide a public stop method) so the timer is
invalidated if the AccountManager is ever deallocated.
| private func updateMenuBarCountdown() { | ||
| // Find all accounts with active countdown | ||
| let activeAccounts = accounts.compactMap { account -> (Account, Date)? in | ||
| guard let timeLimitStr = account.time_limit, | ||
| let timeLimit = ISO8601DateFormatter().date(from: timeLimitStr), | ||
| timeLimit > Date() else { | ||
| return nil | ||
| } | ||
| return (account, timeLimit) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Cache ISO8601DateFormatter to avoid repeated instantiation.
ISO8601DateFormatter() is created on every iteration inside compactMap, which runs every second. This is inefficient.
+ private static let iso8601Formatter = ISO8601DateFormatter()
+
private func updateMenuBarCountdown() {
+ let formatter = Self.iso8601Formatter
// Find all accounts with active countdown
let activeAccounts = accounts.compactMap { account -> (Account, Date)? in
guard let timeLimitStr = account.time_limit,
- let timeLimit = ISO8601DateFormatter().date(from: timeLimitStr),
+ let timeLimit = formatter.date(from: timeLimitStr),
timeLimit > Date() else {
return nil
}
return (account, timeLimit)
}📝 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.
| private func updateMenuBarCountdown() { | |
| // Find all accounts with active countdown | |
| let activeAccounts = accounts.compactMap { account -> (Account, Date)? in | |
| guard let timeLimitStr = account.time_limit, | |
| let timeLimit = ISO8601DateFormatter().date(from: timeLimitStr), | |
| timeLimit > Date() else { | |
| return nil | |
| } | |
| return (account, timeLimit) | |
| } | |
| private static let iso8601Formatter = ISO8601DateFormatter() | |
| private func updateMenuBarCountdown() { | |
| let formatter = Self.iso8601Formatter | |
| // Find all accounts with active countdown | |
| let activeAccounts = accounts.compactMap { account -> (Account, Date)? in | |
| guard let timeLimitStr = account.time_limit, | |
| let timeLimit = formatter.date(from: timeLimitStr), | |
| timeLimit > Date() else { | |
| return nil | |
| } | |
| return (account, timeLimit) | |
| } |
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Logic/AccountManager.swift around lines 60 to 69,
the ISO8601DateFormatter() is being instantiated inside the compactMap for every
account (and every second), causing needless allocations; move the formatter
creation out of the loop and reuse a single instance — either create a local let
formatter = ISO8601DateFormatter() immediately before the compactMap or add a
stored/static property on AccountManager (e.g., private let isoFormatter =
ISO8601DateFormatter()) and use that formatter in the compactMap to avoid
repeated instantiation.
| static func parse(time: String, date: String) -> Date? { | ||
| let combined = "\(time) \(date)" | ||
| let formatter = DateFormatter() | ||
| formatter.dateFormat = "HH:mm dd.MM.yyyy" | ||
| formatter.locale = Locale(identifier: "en_US_POSIX") | ||
| return formatter.date(from: combined) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Cache DateFormatter instances for performance.
DateFormatter creation is expensive and these methods may be called frequently (e.g., every second for countdown updates). Consider caching the formatters as static properties.
Additionally, defaultDateString() and defaultTimeString() don't specify a locale, which may produce unexpected results depending on user's locale settings.
Apply this diff to cache formatters:
struct TimeLimitFormatter {
+
+ private static let parseFormatter: DateFormatter = {
+ let formatter = DateFormatter()
+ formatter.dateFormat = "HH:mm dd.MM.yyyy"
+ formatter.locale = Locale(identifier: "en_US_POSIX")
+ return formatter
+ }()
+
+ private static let dateStringFormatter: DateFormatter = {
+ let formatter = DateFormatter()
+ formatter.dateFormat = "dd.MM.yyyy"
+ formatter.locale = Locale(identifier: "en_US_POSIX")
+ return formatter
+ }()
+
+ private static let timeStringFormatter: DateFormatter = {
+ let formatter = DateFormatter()
+ formatter.dateFormat = "HH:mm"
+ formatter.locale = Locale(identifier: "en_US_POSIX")
+ return formatter
+ }()
static func parse(time: String, date: String) -> Date? {
let combined = "\(time) \(date)"
- let formatter = DateFormatter()
- formatter.dateFormat = "HH:mm dd.MM.yyyy"
- formatter.locale = Locale(identifier: "en_US_POSIX")
- return formatter.date(from: combined)
+ return parseFormatter.date(from: combined)
}Also applies to: 47-59
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Utils/TimeLimitFormatter.swift around lines 11-17
(and also apply same change to lines 47-59), the code creates new DateFormatter
instances on each call and omits an explicit locale; change this by adding
static (shared) DateFormatter properties configured once (thread-safe static
lets) with the correct dateFormat and locale = Locale(identifier:
"en_US_POSIX"), then use those cached formatters in parse(time:date:) and in
defaultDateString()/defaultTimeString() so formatting/parsing is fast and
locale-deterministic.
| static func formatCountdown(to date: Date) -> String? { | ||
| let now = Date() | ||
|
|
||
| if date <= now { | ||
| return nil // Don't show anything when expired | ||
| } | ||
|
|
||
| let calendar = Calendar.current | ||
| let components = calendar.dateComponents([.day, .hour, .minute, .second], from: now, to: date) | ||
|
|
||
| let days = components.day ?? 0 | ||
| let hours = components.hour ?? 0 | ||
| let minutes = components.minute ?? 0 | ||
| let seconds = components.second ?? 0 | ||
|
|
||
| // Format as HH:mm:ss with optional days prefix | ||
| if days > 0 { | ||
| return String(format: " ⏳ %dd %d:%02d:%02d", days, hours, minutes, seconds) | ||
| } else { | ||
| return String(format: " ⏳ %d:%02d:%02d", hours, minutes, seconds) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reduce code duplication between formatCountdown and formatMenuBarCountdown.
These two methods share nearly identical logic for calculating date components. Consider extracting the common logic into a private helper.
+ private static func countdownComponents(to date: Date) -> (days: Int, hours: Int, minutes: Int, seconds: Int)? {
+ let now = Date()
+ guard date > now else { return nil }
+
+ let calendar = Calendar.current
+ let components = calendar.dateComponents([.day, .hour, .minute, .second], from: now, to: date)
+ return (
+ components.day ?? 0,
+ components.hour ?? 0,
+ components.minute ?? 0,
+ components.second ?? 0
+ )
+ }
+
static func formatCountdown(to date: Date) -> String? {
- let now = Date()
-
- if date <= now {
- return nil
- }
-
- let calendar = Calendar.current
- let components = calendar.dateComponents([.day, .hour, .minute, .second], from: now, to: date)
-
- let days = components.day ?? 0
- let hours = components.hour ?? 0
- let minutes = components.minute ?? 0
- let seconds = components.second ?? 0
-
- if days > 0 {
+ guard let (days, hours, minutes, seconds) = countdownComponents(to: date) else {
+ return nil
+ }
+ if days > 0 {
return String(format: " ⏳ %dd %d:%02d:%02d", days, hours, minutes, seconds)
} else {
return String(format: " ⏳ %d:%02d:%02d", hours, minutes, seconds)
}
}Also applies to: 64-85
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Utils/TimeLimitFormatter.swift around lines 22-43
(and similarly lines 64-85), both formatCountdown and formatMenuBarCountdown
duplicate the same date-component computation and formatting; extract that
shared logic into a private helper function (e.g., a helper that takes a target
Date and returns either an optional formatted String or an optional struct/tuple
containing days/hours/minutes/seconds) that performs: now check -> return nil if
expired -> compute calendar dateComponents -> produce the same " ⏳ ..." string
(or components) so both public methods simply call the helper and return its
result, preserving existing formatting and nil behavior.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AntigravityMenuBar/Logic/AccountManager.swift (1)
3-11: Consider storingtime_limitasDate?(or centralize ISO8601 encode/decode) to avoid parse drift.
Right nowupdateTimeLimit()writes viadate.ISO8601Format(), while reads use a defaultISO8601DateFormatter(); these formats/options can diverge and silently break countdown parsing.
♻️ Duplicate comments (3)
Sources/AntigravityMenuBar/Logic/AccountManager.swift (3)
23-26: Remove redundant optionalnilinitialization (SwiftLint).
Line 24 can be@Published var currentEmail: String?(no= nil).
38-48: AvoidrefreshTimer!and add timer cleanup/invalidation.
Force unwrap at Line 44 is avoidable, andstartRefreshTimer()should invalidate any prior timer + provide a stop/deinit cleanup hook.
60-99: CacheISO8601DateFormatterand ensure main-actor consistency foraccounts.
ISO8601DateFormatter()is created inside the per-account loop every second (allocation churn).- Also,
AccountManagerisn’t@MainActor; if any caller invokesupdateTimeLimit/removeAccount/saveAccounts/loadAccountsoff-main while this timer readsaccounts, you can race/crash.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.vscode/launch.json(1 hunks)Sources/AntigravityMenuBar/Logic/AccountManager.swift(3 hunks)antigravity-switcher(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Sources/AntigravityMenuBar/Logic/AccountManager.swift (2)
Sources/AntigravityMenuBar/Utils/DBManager.swift (1)
getCurrentAccountEmail(129-143)Sources/AntigravityMenuBar/Utils/TimeLimitFormatter.swift (1)
formatMenuBarCountdown(64-85)
🪛 SwiftLint (0.57.0)
Sources/AntigravityMenuBar/Logic/AccountManager.swift
[Warning] 24-24: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (2)
.vscode/launch.json (1)
1-24: Configs look fine; verify referencedpreLaunchTasknames exist.
If.vscode/tasks.jsondoesn’t defineswift: Build Debug AntigravityMenuBar/swift: Build Release AntigravityMenuBar, these launch configs will fail to start.Sources/AntigravityMenuBar/Logic/AccountManager.swift (1)
222-227: Good: updatescurrentEmailright after restore to keep the checkmark accurate.
| @@ -0,0 +1 @@ | |||
| Subproject commit cfec68659d343e7ea2c94bb01e5d15ebeda45258 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if antigravity-switcher submodule exists and is initialized
git config --file .gitmodules --name-only --get-regexp path | grep antigravity-switcherRepository: nhonn/antigravity-switcher
Length of output: 52
🏁 Script executed:
# Check the current submodule reference in the file
cat antigravity-switcherRepository: nhonn/antigravity-switcher
Length of output: 111
🏁 Script executed:
# List all submodules and their configuration
git config --file .gitmodules --get-regexp '^submodule' | head -20Repository: nhonn/antigravity-switcher
Length of output: 52
🏁 Script executed:
# Check git status for submodules
git status --porcelain | grep antigravity-switcherRepository: nhonn/antigravity-switcher
Length of output: 52
🏁 Script executed:
# Check if .gitmodules exists and look for antigravity-switcher
test -f .gitmodules && cat .gitmodules || echo ".gitmodules not found"Repository: nhonn/antigravity-switcher
Length of output: 91
🏁 Script executed:
# Check git submodule status
git submodule status 2>&1 || echo "No submodules or error"Repository: nhonn/antigravity-switcher
Length of output: 173
🏁 Script executed:
# List files in root directory to understand structure
ls -la | head -30Repository: nhonn/antigravity-switcher
Length of output: 962
🏁 Script executed:
# Check if antigravity-switcher is a submodule and its commit
git ls-files --stage | grep antigravity-switcherRepository: nhonn/antigravity-switcher
Length of output: 140
🏁 Script executed:
# Check if antigravity-switcher directory has its own .git folder
ls -la antigravity-switcher/ | head -15Repository: nhonn/antigravity-switcher
Length of output: 178
🏁 Script executed:
# Check git log to see when this entry was added
git log --oneline --all -- antigravity-switcher | head -10Repository: nhonn/antigravity-switcher
Length of output: 109
🏁 Script executed:
# Check git diff to see the actual change in this file
git diff HEAD~1 antigravity-switcher 2>&1 || echo "Cannot diff submodule entry"Repository: nhonn/antigravity-switcher
Length of output: 284
🏁 Script executed:
# Verify the commit hash format and check if it's a valid git object
git cat-file -t cfec68659d343e7ea2c94bb01e5d15ebeda45258 2>&1 || echo "Commit object not found in this repository"Repository: nhonn/antigravity-switcher
Length of output: 284
Remove or properly configure the broken submodule entry for antigravity-switcher.
The submodule reference at antigravity-switcher is in an invalid state:
- The commit hash
cfec68659d343e7ea2c94bb01e5d15ebeda45258does not exist in the repository and cannot be fetched - The
antigravity-switcherdirectory is empty with no.gitfolder or configuration - No
.gitmodulesfile exists to define this as a proper submodule
Either remove this entry from the git index (git rm --cached antigravity-switcher) or properly initialize it as a submodule with valid configuration and a reachable commit reference.
🤖 Prompt for AI Agents
In antigravity-switcher around lines 1 to 1, the repository contains a broken
submodule reference (missing .gitmodules, empty directory, and an unreachable
commit cfec68659d343e7ea2c94bb01e5d15ebeda45258); either remove the entry from
the git index or properly configure it as a submodule: to remove, run git rm
--cached antigravity-switcher, delete the empty directory if present, commit the
removal and push; to fix as a submodule, add a .gitmodules entry with the
correct repository URL and path, run git submodule sync && git submodule init &&
git submodule update --remote (or add with git submodule add <url>
antigravity-switcher) ensuring the submodule points to a reachable commit, then
commit .gitmodules and the submodule update.
| /// Separate class for menu bar countdown to avoid triggering menu content refresh | ||
| class MenuBarState: ObservableObject { | ||
| static let shared = MenuBarState() | ||
| @Published var countdown: String = "" | ||
| private init() {} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make MenuBarState @MainActor (published UI state).
It’s updated from multiple call sites; marking it @MainActor prevents accidental off-main publishes as this evolves.
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Logic/AccountManager.swift around lines 13–18, the
MenuBarState ObservableObject is not annotated with @MainActor which can allow
off-main-thread publishes; fix by annotating the class declaration with
@MainActor (i.e. add @MainActor before `class MenuBarState`) so all property
updates and @Published publishes run on the main actor; keep the existing
`shared` singleton and private init as-is.
| func updateTimeLimit(id: String, date: Date?) { | ||
| guard let index = accounts.firstIndex(where: { $0.id == id }) else { | ||
| return | ||
| } | ||
|
|
||
| if let date = date { | ||
| accounts[index].time_limit = date.ISO8601Format() | ||
| print("⏱ Set time limit for \(accounts[index].name): \(date)") | ||
| } else { | ||
| accounts[index].time_limit = nil | ||
| print("⏱ Cleared time limit for \(accounts[index].name)") | ||
| } | ||
|
|
||
| saveAccounts() | ||
| } |
There was a problem hiding this comment.
Ensure updateTimeLimit runs on main (and consider removing account-name logs).
This mutates @Published accounts; if it’s ever called off-main, SwiftUI will warn and you can race the timer. Also print includes account name (minor privacy/PII concern in logs).
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Logic/AccountManager.swift around lines 265 to
279, updateTimeLimit mutates the @Published accounts array and currently logs
account names; ensure the mutation and saveAccounts call execute on the main
thread to avoid SwiftUI threading warnings (wrap mutation/save in
DispatchQueue.main.async if not on main, or assert/guard Thread.isMainThread at
entry), and remove or redact account.name from the print statements (use a
generic message or omit prints entirely) to avoid PII in logs.
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (8)
Sources/AntigravityMenuBar/AntigravityApp.swift (3)
74-78: Remove commented-out code.This dead code was flagged in a previous review. It should be removed to maintain code cleanliness.
147-166: Cache ISO8601DateFormatter to avoid repeated instantiation.Both
accountDisplayNameandhasActiveCountdowncreate newISO8601DateFormatter()instances on each call. This was flagged in a previous review.
374-382: Consider validating that the parsed date is in the future.Setting a time limit in the past would be immediately expired and have no effect. This was flagged in a previous review.
Sources/AntigravityMenuBar/Logic/AccountManager.swift (5)
24-24: Remove redundant nil initialization.This was flagged in a previous review.
45-55: Avoid force unwrap of timer.This was flagged in a previous review.
82-91: Cache ISO8601DateFormatter to avoid repeated instantiation.This was flagged in a previous review.
13-18: MakeMenuBarState@MainActor.This was flagged in a previous review.
310-324: EnsureupdateTimeLimitruns on main thread.This was flagged in a previous review regarding thread safety for
@Publishedproperty mutations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
Sources/AntigravityMenuBar/AntigravityApp.swift(6 hunks)Sources/AntigravityMenuBar/Logic/AccountManager.swift(5 hunks)Sources/AntigravityMenuBar/Logic/QuotaService.swift(1 hunks)Sources/AntigravityMenuBar/Utils/AppConfiguration.swift(1 hunks)Sources/AntigravityMenuBar/Utils/AppError.swift(2 hunks)Sources/AntigravityMenuBar/Utils/DBManager.swift(1 hunks)Sources/AntigravityMenuBar/Utils/LocalhostInsecureURLSessionDelegate.swift(1 hunks)Sources/AntigravityMenuBar/Utils/ProcessManager.swift(1 hunks)Sources/AntigravityMenuBar/Utils/ProgressPanel.swift(1 hunks)Sources/AntigravityMenuBar/Utils/Shell.swift(1 hunks)build.sh(2 hunks)scripts/verify_switch_quota.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
Sources/AntigravityMenuBar/Utils/ProcessManager.swift (1)
Sources/AntigravityMenuBar/Utils/Shell.swift (1)
run(21-88)
Sources/AntigravityMenuBar/Logic/AccountManager.swift (4)
Sources/AntigravityMenuBar/Utils/DBManager.swift (3)
getCurrentAccountEmail(138-152)backupData(21-64)restoreData(67-95)Sources/AntigravityMenuBar/Utils/TimeLimitFormatter.swift (1)
formatMenuBarCountdown(64-85)Sources/AntigravityMenuBar/Utils/ProcessManager.swift (1)
terminateLanguageServers(73-121)Sources/AntigravityMenuBar/Logic/QuotaService.swift (1)
fetchQuota(51-68)
Sources/AntigravityMenuBar/Logic/QuotaService.swift (1)
Sources/AntigravityMenuBar/Utils/Shell.swift (1)
run(21-88)
Sources/AntigravityMenuBar/AntigravityApp.swift (3)
Sources/AntigravityMenuBar/Logic/AccountManager.swift (2)
updateTimeLimit(310-324)checkQuota(333-435)Sources/AntigravityMenuBar/Utils/TimeLimitFormatter.swift (4)
formatCountdown(22-43)defaultTimeString(55-59)defaultDateString(47-51)parse(11-17)Sources/AntigravityMenuBar/Utils/ProgressPanel.swift (2)
show(53-56)close(58-60)
🪛 Ruff (0.14.8)
scripts/verify_switch_quota.py
9-9: typing.Dict is deprecated, use dict instead
(UP035)
9-9: typing.List is deprecated, use list instead
(UP035)
9-9: typing.Tuple is deprecated, use tuple instead
(UP035)
59-59: Do not catch blind exception: Exception
(BLE001)
107-107: Avoid specifying long messages outside the exception class
(TRY003)
114-114: Avoid specifying long messages outside the exception class
(TRY003)
118-118: subprocess call: check for execution of untrusted input
(S603)
147-147: subprocess call: check for execution of untrusted input
(S603)
178-178: subprocess call: check for execution of untrusted input
(S603)
195-195: Unnecessary mode argument
Remove mode argument
(UP015)
215-215: Unnecessary mode argument
Remove mode argument
(UP015)
🔇 Additional comments (16)
build.sh (3)
12-13: ✓ Pre-build cleanup is good practice.The
rm -rf .build/ensures a clean build state before compilation, which helps prevent stale artifacts and ensures reproducible builds. The subsequentswift build(line 13) will recreate the directory correctly.
39-40: ✓ Icon reference follows macOS conventions.The
CFBundleIconFilekey correctly references "AppIcon" (base name without extension), and line 55 properly copies the icon asset asAppIcon.icnsin the Resources directory. The conditional check (line 54) ensures graceful degradation if the icon asset is missing.
54-56: Ensureassets/icon.icnsexists in the repository.The icon asset is referenced in the plist (lines 39–40) and copied here. If the source file is missing, the app will still build and run, but without an icon. Verify that
assets/icon.icnsis committed to the repository to ensure the app displays the icon as intended.Sources/AntigravityMenuBar/Utils/AppConfiguration.swift (1)
12-29: LGTM!The expanded backup keys cover additional Antigravity state (user settings, onboarding, analytics, UI) that aligns with the account-switching feature. The new bundle ID addition is consistent with the existing pattern.
Sources/AntigravityMenuBar/Utils/ProcessManager.swift (1)
73-96: Implementation looks solid for the use case.The language server termination logic correctly identifies processes by command pattern and handles graceful/forced termination. The best-effort approach with error swallowing is appropriate here.
Sources/AntigravityMenuBar/Utils/AppError.swift (1)
10-36: LGTM!The new error cases are well-structured with clear, actionable messages. The associated values for
quotaFetchFailedandcommandFailedprovide useful context for debugging.Sources/AntigravityMenuBar/Utils/ProgressPanel.swift (1)
3-61: LGTM! Clean implementation of a progress panel utility.The
@MainActorannotation is appropriate for UI code, and the panel configuration with floating behavior and proper constraint setup is well done.Minor consideration: The spinner's animation is never explicitly stopped in
close(). WhileorderOuthides the panel (and the spinner is effectively invisible), callingspinner.stopAnimation(nil)inclose()would be slightly more correct for resource cleanup if the panel instance is reused.Sources/AntigravityMenuBar/Utils/Shell.swift (1)
3-18: LGTM!The
ShellErrorenum provides clear, descriptive error cases with properLocalizedErrorconformance.scripts/verify_switch_quota.py (1)
183-265: LGTM! Well-structured verification script.The main function has good error handling with distinct exit codes, proper state restoration, and reasonable timing for process coordination. The rollback logic ensures the original DB state is restored regardless of outcome.
Sources/AntigravityMenuBar/AntigravityApp.swift (1)
3-15: LGTM! Clean MenuBarLabelView implementation.The view correctly observes
MenuBarState.sharedand conditionally displays the countdown text alongside the icon.Sources/AntigravityMenuBar/Logic/AccountManager.swift (3)
33-70: LGTM! Quota cache implementation is thread-safe.The
quotaCacheLockproperly protects concurrent access to the cache dictionary, and the cache expiry logic is correct.
446-455:waitForAntigravityRunningchecks app running state, not language server readiness.The function checks
ProcessManager.shared.isRunning()which likely verifies the Antigravity app is running. However, the caller (checkQuota) needs the language server to be ready before fetching quota. Consider renaming for clarity or waiting for the language server specifically:- private func waitForAntigravityRunning(timeoutSeconds: TimeInterval) async -> Bool { + private func waitForAntigravityAppRunning(timeoutSeconds: TimeInterval) async -> Bool {Or, if
QuotaService.fetchQuota()already retries internally (which it does viadetectLanguageServer), this may be acceptable—but the 12-second timeout here plus the 15-second internal timeout inQuotaServicecould lead to long waits.
275-288: LGTM! Clean implementation of applying limit before switch.The logic correctly identifies the active account by email, applies the time limit, and then performs the switch.
Sources/AntigravityMenuBar/Logic/QuotaService.swift (3)
3-37: LGTM! Well-designed quota snapshot model.The nested structs with computed properties (
remainingPercentage,isExhausted) provide clean API for consumers.
102-175: LGTM! Robust language server detection with scoring.The scoring system prioritizes known process signatures while still being flexible enough to handle variations. The fallback from
etimestoetimehandles macOS BSD vs GNU ps differences well.
417-429: LGTM! Simple and effective regex helper.The helper correctly extracts the first capture group from a pattern match.
| import sqlite3 | ||
| import subprocess | ||
| import time | ||
| from typing import Dict, List, Optional, Tuple |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use modern type hint syntax (Python 3.9+).
Per static analysis, typing.Dict, typing.List, typing.Tuple, and typing.Optional are deprecated. Use built-in generics instead:
-from typing import Dict, List, Optional, Tuple
+from typing import OptionalThen update signatures:
Dict[str, str]→dict[str, str]List[str]→list[str]List[int]→list[int]Tuple[int, str, List[int]]→tuple[int, str, list[int]]
📝 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.
| from typing import Dict, List, Optional, Tuple | |
| from typing import Optional |
🧰 Tools
🪛 Ruff (0.14.8)
9-9: typing.Dict is deprecated, use dict instead
(UP035)
9-9: typing.List is deprecated, use list instead
(UP035)
9-9: typing.Tuple is deprecated, use tuple instead
(UP035)
🤖 Prompt for AI Agents
In scripts/verify_switch_quota.py around line 9, the file imports typing aliases
(Dict, List, Optional, Tuple) that are deprecated; replace uses with built-in
generics (dict, list, tuple, Optional -> optional built-in annotation using |
None or typing.Optional removal). Remove the typing import of
Dict/List/Tuple/Optional and update all type hints accordingly: Dict[str, str]
-> dict[str, str], List[str] -> list[str], List[int] -> list[int], Tuple[int,
str, List[int]] -> tuple[int, str, list[int]], and change Optional[X] to X |
None (or just X | None where appropriate) so all annotations use Python 3.9+
built-ins.
| def db_get_values(keys: List[str]) -> Dict[str, str]: | ||
| con = sqlite3.connect(DB_PATH) | ||
| cur = con.cursor() | ||
| out: Dict[str, str] = {} | ||
| for k in keys: | ||
| cur.execute("select value from ItemTable where key=?", (k,)) | ||
| row = cur.fetchone() | ||
| if row and row[0] is not None: | ||
| out[k] = row[0] | ||
| con.close() | ||
| return out |
There was a problem hiding this comment.
DB connection not closed on exception.
If cur.execute or cur.fetchone raises an exception, con.close() is never called, leaking the connection.
def db_get_values(keys: List[str]) -> Dict[str, str]:
- con = sqlite3.connect(DB_PATH)
- cur = con.cursor()
out: Dict[str, str] = {}
- for k in keys:
- cur.execute("select value from ItemTable where key=?", (k,))
- row = cur.fetchone()
- if row and row[0] is not None:
- out[k] = row[0]
- con.close()
+ with sqlite3.connect(DB_PATH) as con:
+ cur = con.cursor()
+ for k in keys:
+ cur.execute("select value from ItemTable where key=?", (k,))
+ row = cur.fetchone()
+ if row and row[0] is not None:
+ out[k] = row[0]
return out📝 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.
| def db_get_values(keys: List[str]) -> Dict[str, str]: | |
| con = sqlite3.connect(DB_PATH) | |
| cur = con.cursor() | |
| out: Dict[str, str] = {} | |
| for k in keys: | |
| cur.execute("select value from ItemTable where key=?", (k,)) | |
| row = cur.fetchone() | |
| if row and row[0] is not None: | |
| out[k] = row[0] | |
| con.close() | |
| return out | |
| def db_get_values(keys: List[str]) -> Dict[str, str]: | |
| out: Dict[str, str] = {} | |
| with sqlite3.connect(DB_PATH) as con: | |
| cur = con.cursor() | |
| for k in keys: | |
| cur.execute("select value from ItemTable where key=?", (k,)) | |
| row = cur.fetchone() | |
| if row and row[0] is not None: | |
| out[k] = row[0] | |
| return out |
🤖 Prompt for AI Agents
In scripts/verify_switch_quota.py around lines 30-40 the DB connection is opened
but not guaranteed to be closed if cur.execute or cur.fetchone raises; wrap the
sqlite3.connect call in a context that always closes the connection (use a
try/finally or use "with sqlite3.connect(DB_PATH) as con:" and acquire a cursor
inside that block) so con.close() is executed even on exceptions, and return or
re-raise the exception as appropriate; ensure the function still returns the
expected dict when no error occurs.
| def db_set_values(d: Dict[str, str]) -> None: | ||
| con = sqlite3.connect(DB_PATH) | ||
| cur = con.cursor() | ||
| for k, v in d.items(): | ||
| cur.execute( | ||
| "insert or replace into ItemTable(key,value) values (?,?)", | ||
| (k, v), | ||
| ) | ||
| con.commit() | ||
| con.close() |
There was a problem hiding this comment.
Same connection leak issue in db_set_values.
Use a context manager here as well:
def db_set_values(d: Dict[str, str]) -> None:
- con = sqlite3.connect(DB_PATH)
- cur = con.cursor()
- for k, v in d.items():
- cur.execute(
- "insert or replace into ItemTable(key,value) values (?,?)",
- (k, v),
- )
- con.commit()
- con.close()
+ with sqlite3.connect(DB_PATH) as con:
+ cur = con.cursor()
+ for k, v in d.items():
+ cur.execute(
+ "insert or replace into ItemTable(key,value) values (?,?)",
+ (k, v),
+ )
+ con.commit()📝 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.
| def db_set_values(d: Dict[str, str]) -> None: | |
| con = sqlite3.connect(DB_PATH) | |
| cur = con.cursor() | |
| for k, v in d.items(): | |
| cur.execute( | |
| "insert or replace into ItemTable(key,value) values (?,?)", | |
| (k, v), | |
| ) | |
| con.commit() | |
| con.close() | |
| def db_set_values(d: Dict[str, str]) -> None: | |
| with sqlite3.connect(DB_PATH) as con: | |
| cur = con.cursor() | |
| for k, v in d.items(): | |
| cur.execute( | |
| "insert or replace into ItemTable(key,value) values (?,?)", | |
| (k, v), | |
| ) | |
| con.commit() |
🤖 Prompt for AI Agents
In scripts/verify_switch_quota.py around lines 43 to 52, db_set_values opens a
sqlite3 connection and cursor without a context manager which can leak
connections on exceptions; change it to use a with sqlite3.connect(DB_PATH) as
con: block (and obtain the cursor from that connection) so the connection is
automatically closed and committed even on errors, then perform the same insert
or replace loop inside that block.
| with open(ACCOUNTS_PATH, "r") as f: | ||
| accounts = list(json.load(f).values()) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unnecessary "r" mode argument.
Per static analysis, the mode argument is redundant for open() when reading:
- with open(ACCOUNTS_PATH, "r") as f:
+ with open(ACCOUNTS_PATH) as f:- with open(backup_file, "r") as f:
+ with open(backup_file) as f:Also applies to: 215-216
🧰 Tools
🪛 Ruff (0.14.8)
195-195: Unnecessary mode argument
Remove mode argument
(UP015)
🤖 Prompt for AI Agents
In scripts/verify_switch_quota.py around lines 195-196 (and likewise at lines
215-216), the open() calls include an unnecessary "r" mode argument when only
reading; remove the "r" argument so the calls use open(ACCOUNTS_PATH) (or
open(ACCOUNTS_PATH, encoding='utf-8') if you want explicit encoding) and keep
the rest of the logic unchanged to read the JSON.
| defer { | ||
| Task { @MainActor in | ||
| panel.close() | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
defer may not execute as expected in async context.
The defer block captures panel and schedules a Task to close it. However, if an exception is thrown, the defer runs synchronously on the current actor context, but the Task { @MainActor in ... } schedules work asynchronously. This should work, but consider using a simpler pattern:
- defer {
- Task { @MainActor in
- panel.close()
- }
- }
-
- let snapshot = try await accountManager.checkQuota(id: account.id)
+ let snapshot: QuotaSnapshot
+ do {
+ snapshot = try await accountManager.checkQuota(id: account.id)
+ } catch {
+ await MainActor.run { panel.close() }
+ throw error
+ }
+ await MainActor.run { panel.close() }Or wrap the entire operation in a do-catch that ensures panel.close() is called synchronously on the main actor.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/AntigravityApp.swift around lines 196–200, the
defer currently schedules panel.close() inside Task { @MainActor in ... } which
runs asynchronously and can skip guaranteed synchronous cleanup; replace this
pattern by wrapping the async work in a do-catch (or try/await) and after the
operation (in the finally-equivalent path) call MainActor.run to invoke
panel.close() on the main actor (i.e., perform await MainActor.run {
panel.close() }) so the panel is closed deterministically on the main actor even
if errors occur.
| // Restore allowlisted keys; for keys missing from the backup, clear them to avoid | ||
| // carrying over stale account state from the previously active account. | ||
| for key in keysToBackup { | ||
| if let value = data[key] { | ||
| let query = "INSERT OR REPLACE INTO ItemTable (key, value) VALUES (?, ?)" | ||
| var statement: OpaquePointer? | ||
|
|
||
| if sqlite3_prepare_v2(db, query, -1, &statement, nil) == SQLITE_OK { | ||
| sqlite3_bind_text(statement, 1, (key as NSString).utf8String, -1, nil) | ||
| sqlite3_bind_text(statement, 2, (value as NSString).utf8String, -1, nil) | ||
|
|
||
| if sqlite3_step(statement) != SQLITE_DONE { | ||
| print("❌ Error writing key: \(key)") | ||
| } | ||
| } | ||
| sqlite3_finalize(statement) | ||
| } else { | ||
| let query = "DELETE FROM ItemTable WHERE key = ?" | ||
| var statement: OpaquePointer? | ||
| if sqlite3_prepare_v2(db, query, -1, &statement, nil) == SQLITE_OK { | ||
| sqlite3_bind_text(statement, 1, (key as NSString).utf8String, -1, nil) | ||
| _ = sqlite3_step(statement) | ||
| } | ||
| sqlite3_finalize(statement) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider wrapping restore in a transaction for atomicity.
If a failure occurs mid-restore, the database could be left in a partially-updated state with some keys restored and some deleted. Wrapping the loop in a BEGIN/COMMIT transaction ensures all-or-nothing semantics.
+ // Begin transaction for atomicity
+ sqlite3_exec(db, "BEGIN TRANSACTION", nil, nil, nil)
+
// Restore allowlisted keys; for keys missing from the backup, clear them to avoid
// carrying over stale account state from the previously active account.
for key in keysToBackup {
if let value = data[key] {
// ... existing INSERT OR REPLACE logic ...
} else {
// ... existing DELETE logic ...
}
}
+
+ sqlite3_exec(db, "COMMIT", nil, nil, nil)
return .success(())Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Utils/DBManager.swift around lines 107 to 132, the
restore loop should be wrapped in a SQLite transaction to ensure atomicity:
start a transaction (BEGIN) before the for-loop, run all INSERT/DELETE
operations, and if any sqlite3_step or prepare returns an error perform a
rollback (ROLLBACK) and surface the error; otherwise commit (COMMIT) after the
loop. Ensure you check return codes from
sqlite3_exec/sqlite3_step/sqlite3_prepare_v2, finalize statements in all paths,
and handle errors so the transaction is always either committed on success or
rolled back on failure.
| } else { | ||
| let query = "DELETE FROM ItemTable WHERE key = ?" | ||
| var statement: OpaquePointer? | ||
| if sqlite3_prepare_v2(db, query, -1, &statement, nil) == SQLITE_OK { | ||
| sqlite3_bind_text(statement, 1, (key as NSString).utf8String, -1, nil) | ||
| _ = sqlite3_step(statement) | ||
| } | ||
| sqlite3_finalize(statement) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider logging DELETE failures for consistency.
The INSERT branch logs errors (line 119), but DELETE failures are silently ignored. Adding similar logging would help debug restore issues.
} else {
let query = "DELETE FROM ItemTable WHERE key = ?"
var statement: OpaquePointer?
if sqlite3_prepare_v2(db, query, -1, &statement, nil) == SQLITE_OK {
sqlite3_bind_text(statement, 1, (key as NSString).utf8String, -1, nil)
- _ = sqlite3_step(statement)
+ if sqlite3_step(statement) != SQLITE_DONE {
+ print("❌ Error deleting key: \(key)")
+ }
}
sqlite3_finalize(statement)
}🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Utils/DBManager.swift around lines 123 to 131, the
DELETE branch silently ignores failures while the INSERT branch logs errors;
update the DELETE code to mirror the INSERT branch error handling by checking
sqlite3_prepare_v2 and sqlite3_step return values and logging failures using the
same logging mechanism used for INSERT, including the sqlite error message from
sqlite3_errmsg(db) (and ensure sqlite3_finalize(statement) still runs in all
cases).
| guard challenge.protectionSpace.authenticationMethod == NSURLAuthenticationMethodServerTrust, | ||
| let trust = challenge.protectionSpace.serverTrust, | ||
| let host = challenge.protectionSpace.host.lowercased() as String? else { | ||
| completionHandler(.performDefaultHandling, nil) | ||
| return | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant cast and missing IPv6 localhost.
Line 13 has a redundant as String? cast—host.lowercased() already returns String. Additionally, ::1 (IPv6 localhost) isn't handled.
- guard challenge.protectionSpace.authenticationMethod == NSURLAuthenticationMethodServerTrust,
- let trust = challenge.protectionSpace.serverTrust,
- let host = challenge.protectionSpace.host.lowercased() as String? else {
+ guard challenge.protectionSpace.authenticationMethod == NSURLAuthenticationMethodServerTrust,
+ let trust = challenge.protectionSpace.serverTrust else {
completionHandler(.performDefaultHandling, nil)
return
}
+ let host = challenge.protectionSpace.host.lowercased()And update the check:
- if host == "127.0.0.1" || host == "localhost" {
+ if host == "127.0.0.1" || host == "localhost" || host == "::1" {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Utils/LocalhostInsecureURLSessionDelegate.swift
around lines 11–16, remove the redundant `as String?` cast from
`host.lowercased()` (lowercased() already returns String) and extend the
localhost check to include IPv6 by treating `"::1"` as trusted (e.g., check host
== "localhost" || host == "127.0.0.1" || host == "::1" — or include any other
project-accepted localhost variants); keep the existing guard semantics and call
completionHandler(.performDefaultHandling, nil) if the host is not one of these
trusted values.
| for (pid, _) in candidates { | ||
| var alive = true | ||
| for _ in 0..<25 { | ||
| if (try? Shell.run("/bin/kill", ["-0", String(pid)], timeoutSeconds: 1)) == nil { | ||
| alive = false | ||
| break | ||
| } | ||
| Thread.sleep(forTimeInterval: 0.2) | ||
| } | ||
| if alive { | ||
| print("⚠️ Language server \(pid) still running, forcing...") | ||
| _ = try? Shell.run("/bin/kill", ["-KILL", String(pid)], timeoutSeconds: 2) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider clarifying the process-alive check logic.
The logic relies on Shell.run throwing for non-zero exit codes (when the process is dead), making the try? ... == nil check work correctly but counter-intuitively. A brief comment would help future maintainers.
for _ in 0..<25 {
+ // kill -0 returns 0 if process exists (Shell.run succeeds → non-nil)
+ // and non-zero if dead (Shell.run throws → try? yields nil)
if (try? Shell.run("/bin/kill", ["-0", String(pid)], timeoutSeconds: 1)) == nil {
alive = false
break
}
Thread.sleep(forTimeInterval: 0.2)
}🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Utils/ProcessManager.swift around lines 103–116,
the process-alive check is counter-intuitive because it relies on try? returning
nil when Shell.run throws for non-zero exit codes (kill -0 returns non‑zero if
the PID is gone); add a short inline comment above the inner loop explaining
that Shell.run("/bin/kill", ["-0", pid]) will throw/return nil via try? when the
process is not running and therefore alive is set to false, and optionally note
a clearer alternative (e.g., explicitly handling Shell.run errors or using a
dedicated process-liveness API) for future refactors.
| // Stop handlers and drain any remaining data. | ||
| stdoutPipe.fileHandleForReading.readabilityHandler = nil | ||
| stderrPipe.fileHandleForReading.readabilityHandler = nil | ||
|
|
||
| lock.lock() | ||
| stdoutData.append(stdoutPipe.fileHandleForReading.readDataToEndOfFile()) | ||
| stderrData.append(stderrPipe.fileHandleForReading.readDataToEndOfFile()) | ||
| lock.unlock() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potential race condition when draining pipe data after handler removal.
Setting readabilityHandler = nil stops the handler, but there's a brief window where the handler might be executing concurrently with the subsequent readDataToEndOfFile() call. While the lock protects the data append, the readDataToEndOfFile() call itself could block or miss data if the handler was mid-read.
Consider waiting briefly or synchronizing more explicitly:
// Stop handlers and drain any remaining data.
stdoutPipe.fileHandleForReading.readabilityHandler = nil
stderrPipe.fileHandleForReading.readabilityHandler = nil
+ // Brief pause to let any in-flight handler invocations complete.
+ Thread.sleep(forTimeInterval: 0.01)
+
lock.lock()
stdoutData.append(stdoutPipe.fileHandleForReading.readDataToEndOfFile())
stderrData.append(stderrPipe.fileHandleForReading.readDataToEndOfFile())
lock.unlock()Alternatively, use a serial queue for handlers and drain synchronously on that queue.
📝 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.
| // Stop handlers and drain any remaining data. | |
| stdoutPipe.fileHandleForReading.readabilityHandler = nil | |
| stderrPipe.fileHandleForReading.readabilityHandler = nil | |
| lock.lock() | |
| stdoutData.append(stdoutPipe.fileHandleForReading.readDataToEndOfFile()) | |
| stderrData.append(stderrPipe.fileHandleForReading.readDataToEndOfFile()) | |
| lock.unlock() | |
| // Stop handlers and drain any remaining data. | |
| stdoutPipe.fileHandleForReading.readabilityHandler = nil | |
| stderrPipe.fileHandleForReading.readabilityHandler = nil | |
| // Brief pause to let any in-flight handler invocations complete. | |
| Thread.sleep(forTimeInterval: 0.01) | |
| lock.lock() | |
| stdoutData.append(stdoutPipe.fileHandleForReading.readDataToEndOfFile()) | |
| stderrData.append(stderrPipe.fileHandleForReading.readDataToEndOfFile()) | |
| lock.unlock() |
🤖 Prompt for AI Agents
In Sources/AntigravityMenuBar/Utils/Shell.swift around lines 67–74, removing the
readabilityHandler and then immediately calling readDataToEndOfFile risks a race
where the handler is concurrently executing; synchronize handler work by
introducing a dedicated serial DispatchQueue used by the readabilityHandler and
perform the draining readDataToEndOfFile synchronously on that same queue (i.e.,
set the handler to nil only after dispatching a synchronous barrier or
dispatchSync to the handler queue so you know the handler has finished), or
alternatively keep the handler appending into stdoutData/stderrData on that
serial queue and call readDataToEndOfFile from that queue to ensure no
concurrent access and no missed data.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.