fix: support configurable Codex usage endpoints#108
fix: support configurable Codex usage endpoints#108Daltonganger wants to merge 11 commits intoopgginc:mainfrom
Conversation
Integrate Nano-GPT as a quota-based provider with daily/monthly reset tracking, local icon assets, and auth parsing support. Add UI/CLI wiring and tests so Nano-GPT usage, balances, and the subscription preset are available end-to-end.
…ations Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
# Conflicts: # CopilotMonitor/CopilotMonitor.xcodeproj/project.pbxproj # CopilotMonitor/CopilotMonitor/App/StatusBarController.swift # CopilotMonitor/CopilotMonitor/Helpers/ProviderMenuBuilder.swift # CopilotMonitor/CopilotMonitor/Providers/NanoGptProvider.swift # CopilotMonitor/CopilotMonitorTests/NanoGptProviderTests.swift
|
This PR introduces configurable Codex usage endpoints, adds fallback account fetching for Antigravity, and enhances API key discovery for search providers by supporting The fallback mechanisms and centralized This is just a short preview. For a detailed, line-by-line code review (including bug detection and security checks), please write a comment like |
❌ AI Code Review CancelledNewer commit detected or workflow was manually cancelled. 📋 View Logs | 🤖 Model: |
|
@Daltonganger this looks like include some other changes not related to the PR |
✅ AI Code Review CompletedReview posted on PR #108 — 4 inline comments with 1 critical bug, 1 major issue, and 2 minor findings. Check the review thread for details. Reviewed with |
|
Done. I removed the unrelated changes and left only the Codex external URL / endpoint update in this PR. |
![op-gg-ai-devops[bot]](https://avatars.githubusercontent.com/in/2680015?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Solid overall design here — the endpoint configuration system with CodexEndpointConfiguration / CodexEndpointMode is clean, the OpenCode config resolution chain with explicit > derived > default priority is well thought out, and the test coverage for the endpoint selection logic is genuinely good. The Antigravity accounts fallback is also a nice resilience improvement.
However, there's a critical off-by-one bug in the SQLite column indexing that will silently break ALL codex-lb account loading. This needs to be fixed before merge.
What's Good
- Config-driven endpoint resolution with proper validation (scheme check, host check, malformed URL fallback)
resolveConfigValue()handles env var references and Bearer prefix stripping — nice touch- Test cases cover the important edge cases: default, derived from baseURL, explicit override, malformed fallback, all-malformed
- Antigravity fallback with cache → API graceful degradation
What Needs Fixing
- 🔴 Critical: SQLite column index off-by-one —
access_token_encryptedis read from the wrong column, causing silent failure for all codex-lb accounts - 🟡 Bug: Dead duplicate branch in
codexRequestAccountID - 🟡 Security: JSON injection via string interpolation in Antigravity fallback HTTP body
CI Status
- ✅ Lint passed
- ⏳ Build, Test, CI still running at time of review
Note
Build/typecheck smoke tests could not be run — this is a macOS-only project and the review runner lacks Xcode. CI results should be checked before merge.
Also — repo owner @kargnas noted this PR may include some changes not directly related to the title. Might be worth cleaning up the commit history or splitting out the Antigravity fallback and NanoGPT changes into separate PRs for cleaner review scope.
Push a new commit to this branch to trigger an automatic re-review. You can also mention @op-gg-ai-devops in a comment to request a new review — optionally with specific directions, e.g.:
@op-gg-ai-devops review the SQLite column indexing fix@op-gg-ai-devops check security implications@op-gg-ai-devops review typos
| chatGPTAccountId: sqliteColumnString(statement, index: 1), | ||
| email: sqliteColumnString(statement, index: 2), | ||
| planType: sqliteColumnString(statement, index: 3), | ||
| status: sqliteColumnString(statement, index: 4), |
There was a problem hiding this comment.
This is a critical off-by-one bug introduced by adding chatgpt_account_id at column index 1.
The SQL query column order is now:
0: id
1: chatgpt_account_id ← NEW
2: email
3: plan_type
4: status
5: access_token_encrypted
6: refresh_token_encrypted
7: id_token_encrypted
8: last_refresh
But line 1140 reads index: 4 for accessTokenEncrypted — that's the status column, not the encrypted token. Then line 1149 also reads index: 4 for status, which is correct but means accessTokenEncrypted contains the status string.
The result: decryptCodexLBFernetToken() receives a status string (like "active") instead of actual encrypted token data, Fernet decryption fails, and every codex-lb account silently fails to load.
Fix:
| status: sqliteColumnString(statement, index: 4), | |
| guard let accessTokenEncrypted = sqliteColumnData(statement, index: 5), !accessTokenEncrypted.isEmpty else { | |
| continue | |
| } | |
| let account = CodexLBEncryptedAccount( | |
| accountId: sqliteColumnString(statement, index: 0), | |
| chatGPTAccountId: sqliteColumnString(statement, index: 1), | |
| email: sqliteColumnString(statement, index: 2), | |
| planType: sqliteColumnString(statement, index: 3), | |
| status: sqliteColumnString(statement, index: 4), | |
| accessTokenEncrypted: accessTokenEncrypted, | |
| refreshTokenEncrypted: sqliteColumnData(statement, index: 6), | |
| idTokenEncrypted: sqliteColumnData(statement, index: 7), | |
| lastRefresh: sqliteColumnString(statement, index: 8) | |
| ) |
Wait — index: 5 for accessTokenEncrypted is now correct, but the guard on line 1140 still says index: 4. The assignment on line 1150 uses the accessTokenEncrypted variable (from the guard), so the guard line is the only one that needs changing:
guard let accessTokenEncrypted = sqliteColumnData(statement, index: 5), ...
| func codexRequestAccountID(for account: OpenAIAuthAccount, endpointMode: CodexEndpointMode) -> String? { | ||
| switch endpointMode { | ||
| case .directChatGPT: | ||
| return account.accountId | ||
| case .external: | ||
| if account.source == .codexLB { | ||
| return account.externalUsageAccountId ?? account.accountId | ||
| } | ||
| return account.externalUsageAccountId ?? account.accountId | ||
| } |
There was a problem hiding this comment.
Both branches of the .external case return the exact same expression:
case .external:
if account.source == .codexLB {
return account.externalUsageAccountId ?? account.accountId // branch A
}
return account.externalUsageAccountId ?? account.accountId // branch B (identical)Either the codexLB check was supposed to do something different (like return account.externalUsageAccountId without the ?? account.accountId fallback), or the if-statement is dead code and should be removed.
If the intent is "codex-lb uses external ID, other sources use regular accountId":
case .external:
if account.source == .codexLB {
return account.externalUsageAccountId ?? account.accountId
}
return account.accountIdOtherwise just collapse to one line:
case .external:
return account.externalUsageAccountId ?? account.accountId| request.httpMethod = "POST" | ||
| request.setValue("Bearer \(accessToken)", forHTTPHeaderField: "Authorization") | ||
| request.setValue("application/json", forHTTPHeaderField: "Content-Type") | ||
| request.httpBody = "{\"project\":\"\(account.projectId)\"}".data(using: .utf8) |
There was a problem hiding this comment.
String interpolation directly into a JSON literal is a classic injection vector. If account.projectId contains a double quote or backslash (unlikely but possible with user-sourced config), the JSON becomes malformed and the request fails silently or — worse — sends unexpected payload structure.
Use proper JSON serialization:
let payload: [String: String] = ["project": account.projectId]
request.httpBody = try JSONSerialization.data(withJSONObject: payload)or even:
request.httpBody = try JSONEncoder().encode(["project": account.projectId])| OpenAIAuthAccount( | ||
| accessToken: access, | ||
| accountId: auth.openai?.accountId, | ||
| externalUsageAccountId: auth.openai?.accountId, |
There was a problem hiding this comment.
For opencodeAuth and codexAuth sources, externalUsageAccountId is set to the same value as accountId. This works for now since these sources don't have a separate ChatGPT account ID concept, but it means the external endpoint will send the OpenCode internal account ID as the ChatGPT-Account-Id header — which may or may not be valid for the external service.
Consider setting externalUsageAccountId: nil for non-codex-lb sources and letting the fallback in codexRequestAccountID handle it. This makes the intent clearer and avoids sending potentially wrong IDs to external endpoints.
externalUsageAccountId: nil, // only codex-lb has a separate ChatGPT account ID…ix/codex-openai-endpoint-config # Conflicts: # CopilotMonitor/CopilotMonitor/Services/TokenManager.swift
✅ AI Code Review CompletedReview posted successfully for PR #108. Reviewed with |
There was a problem hiding this comment.
Review Summary
The critical SQLite column off-by-one bug from the previous review is fixed — column indices are now correct with chatgpt_account_id at index 1 and all subsequent fields properly shifted. The unrelated changes (Antigravity fallback, NanoGPT, search config) have been cleaned out per the repo owner's request, leaving a focused and clean PR.
What's Good
- Endpoint configuration system is genuinely well-designed. The
CodexEndpointConfiguration/CodexEndpointModetypes with the explicit > derived > default resolution chain is clean. URL validation (scheme + host check) with proper fallback on malformed config is solid defensive coding. - SQL query fallback cascade is clever. Four query variants handle both old schemas (without
chatgpt_account_id) and new schemas gracefully. The DB can evolve without breaking existing installs — nice forward/backward compat. - Test coverage is excellent. 10 new test cases covering defaults, derived URLs, explicit overrides, malformed input fallbacks, account ID routing for both codexLB and non-codexLB sources, and the mapping function. The edge cases that matter are all covered.
makeCodexLBOpenAIAccountextraction makes the code more testable and thereadCodexLBOpenAIAccounts()function cleaner.- Crash → graceful error in
codexUsageURLreplacing the old assertion with a properthrow— exactly the right call for a menu bar app that shouldn't crash on bad config.
CI Status
- ✅ Lint passed
- ⏳ Build, Test, CI still running at time of review
Note
Build/typecheck smoke tests could not be run — this is a macOS-only project and the review runner lacks Xcode. CI results should be checked before merge.
Reviewed with anthropic/claude-opus-4-6
2 participants
Summary
chatgpt_account_idwhile keeping existing account IDs for direct ChatGPT usageUpdate
Verification
xcodebuild build -project "CopilotMonitor/CopilotMonitor.xcodeproj" -scheme "CopilotMonitor" -configuration Debug -clonedSourcePackagesDirPath "/Users/rubenbeuker/Library/Developer/Xcode/DerivedData/CopilotMonitor-aggycqisuzewkzczboobrrtuajll/SourcePackages"xcodebuild test -project "CopilotMonitor/CopilotMonitor.xcodeproj" -scheme "CopilotMonitor" -destination 'platform=macOS' -clonedSourcePackagesDirPath "/Users/rubenbeuker/Library/Developer/Xcode/DerivedData/CopilotMonitor-aggycqisuzewkzczboobrrtuajll/SourcePackages"/usr/bin/log show --last 2m --predicate 'subsystem == "com.opencodeproviders"'