Repository navigation
Conversation
Read active integration credentials from the OpenCode SQLite database in read-only mode, including OAuth metadata and API keys. Respect XDG_DATA_HOME and OPENCODE_DB, and use legacy auth.json discovery only when the database is absent. Share credential loading with Gemini OAuth and show the actual credential source in provider menus and diagnostics.
Contributor
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Initial model: Model usage by lane
gantt
dateFormat x
axisFormat %H:%M
todayMarker off
section Main
claude-opus-5-5 : 1792478346537, 1792479153476
section DependencyContext
claude-sonnet-5-5 : 1792478436048, 1792478485231
section lane-a-slop
claude-sonnet-5-5 : 1792478436057, 1792478483720
section lane-b-dry
claude-sonnet-5-5 : 1792478436501, 1792478499082
section lane-c-reviewer
claude-opus-5-5 : 1792478435673, 1792478987079
section lane-d-ponytail
claude-sonnet-5-5 : 1792478436738, 1792478485854
section RepoContext
claude-sonnet-5-5 : 1792478435649, 1792478487130
section ResolvedIssues
claude-sonnet-5-5 : 1792478436516, 1792478465742
Input excludes cached tokens. Output includes reasoning tokens. Speed reflects the requested API tier; provider acceptance is not recorded. |
![opgg-ai-devops[bot]](https://avatars.githubusercontent.com/in/2680015?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Summary
This is a real fix with a real payoff: OpenCode 2.x users stop getting expired-token errors from a stale auth.json while OpenCode itself works fine. The core design is solid. Nothing blocking, so approving. There's one test you broke that you should fix before merging, plus some cleanup.
What's good
getOpenCodeAuthPaths()is now the single gate in front ofgetAuthFilePaths().readOpenCodeAuth,readGeminiOAuthAuthand both debug dumps go through it, and I couldn't find any Swift path that sneaks around it. So "an existing DB is authoritative, no silent fallback to stale JSON" actually holds.- The DB is opened
SQLITE_OPEN_READONLY,finalize/closeare both indefer, and you checkSQLITE_DONEafter the loop, so a partial read errors out instead of quietly returning half the credentials. That matches the repo's "usage collection only reads credentials, never refreshes them" rule. - Row selection is deterministic (
active DESC, time_updated DESC, first row wins per integration). A malformed active row doesn't bring a stale credential back, and the test covers that (anthropic→ nil). - Mapping rows back into the legacy shape so the existing lossy
OpenCodeAuthdecoder keeps working is the smallest way to support both formats. A second schema would have cost more. - The test runs against real SQLite instead of mocks and covers V1 fallback, V2 precedence, active vs newest, metadata, key→api, Gemini, empty table and dropped table. Solid.
What to fix
- major:
MiniMaxProviderTests.swift:123still asserts the old~/.local/share/opencode/auth.jsonliteral. It fails on any machine that has a MiniMax key. See inline. - minor:
accountId/enterpriseUrlget clobbered whenmetadatalacks the key. Three differentauthSourcefallback conventions now exist, and nil drops the "Token From" row. Diagnostics mark an empty DB asACTIVE. The DB-vs-JSON check is re-derived in three places, and the two debug search loops are copy-pasted. Data-dir resolution is duplicated. The README still has staleauth.json-only sections.
Out of diff (FYI, not blocking)
scripts/query-{codex,copilot,kimi,minimax,nano-gpt,claude}.shanddocs/AI_USAGE_API_REFERENCE.mdstill read or document only~/.local/share/opencode/auth.json. OpenCode 2.x users of those scripts will hit the same stale-token problem this PR fixes. Worth a follow-up.- Doc comments outside the hunks still say "OpenCode auth.json" for data that can now come from the DB:
GeminiAuthSource.opencodeAuth(TokenManager.swift:603), thecachedGeminiOAuthAuth/lastFoundGeminiOAuthPathcomments (:867,:871), and// 1. Check all possible auth.json pathsin both debug dumps. - The existing codex-lb reader (
queryCodexLBEncryptedAccounts) copies the DB plus-wal/-shmto a temp dir before a read-only open. This PR opens OpenCode's live DB in place. The repo doesn't tell us whether a read-only open of a WAL DB works when OpenCode isn't running and there's no-shmfile, and your runtime check was presumably with OpenCode running. Do a quick manual run with OpenCode quit before you ship. testGetOpenAIAccountsIncludesOpenCodeAPIKeyAccountdoesn't unsetOPENCODE_DB. If a runner's environment points that at a real DB, the DB takes over and the test gets flaky.
Validation signal
- Smoke tests: skipped (
fork-source-unavailable, recorded asOVERALL status=skipped), so this review has no independent build/test signal. - CI at review time:
Lintpassed.Test,CIandBuild and Releasewere still running, so their results are unknown. - No dependency or manifest changes. The new code uses the system
SQLite3module, which is already imported elsewhere. No known-critical advisory matched in existing manifests. - No env-surface concerns:
XDG_DATA_HOMEandOPENCODE_DBare OpenCode's own variables being honored, not new app config knobs.
Use a consistent source label across providers and recognize custom credential database paths. Carry the selected storage format into readers, share credential diagnostics, and distinguish empty stores from usable credentials. Preserve account metadata when optional database metadata is absent. Isolate credential fixtures from local accounts, cover WAL reads after the writer closes, and update troubleshooting guidance for OpenCode 2.x.
Author
|
Addressed the actionable review findings in 26309a8:
Validation: the full Debug suite completed with 314 tests, 15 skipped, and 0 failures. The MiniMax success case ran. WAL coverage exercises both a live writer and a closed/checkpointed writer with its shared-memory sidecar removed. This uses a disposable SQLite fixture rather than stopping the user's OpenCode service. The rebuilt menu app was relaunched and logged successful database loading; the rebuilt CLI fetched live ChatGPT usage. SwiftLint is still unavailable locally. I kept the existing V1 path discovery to preserve its macOS fallback behavior. OpenRouter currently uses |
Contributor
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Initial model: Model usage by lane
gantt
dateFormat x
axisFormat %H:%M
todayMarker off
section Main
claude-opus-5-5 : 1792479888424, 1792480842965
section ContextAudit
claude-sonnet-5-5 : 1792479972809, 1792480110724
section ContextAudit.lane-e-ponytail-audit
claude-sonnet-5-5 : 1792480040854, 1792480101059
section ContextAudit.ResolvedIssues
claude-sonnet-5-5 : 1792479981040, 1792480020294
section DependencyContext
claude-sonnet-5-5 : 1792479973530, 1792480021346
section lane-a-slop
claude-sonnet-5-5 : 1792479974251, 1792480043355
section lane-b-dry
claude-sonnet-5-5 : 1792479973115, 1792480038627
section lane-c-reviewer
claude-opus-5-5 : 1792479974229, 1792480487240
section lane-d-ponytail
claude-sonnet-5-5 : 1792479973542, 1792480024189
section RepoContext
claude-sonnet-5-5 : 1792479974026, 1792480077108
Input excludes cached tokens. Output includes reasoning tokens. Speed reflects the requested API tier; provider acceptance is not recorded. |
Contributor
There was a problem hiding this comment.
Summary
Round 2, and honestly this one cleaned up nicely. The fix commit (26309a8) resolves most of what I raised last time:
- Top-level
accountIdnow survives whenmetadatais empty, and a test covers it. - Every provider uses one
openCodeAuthSourceLabel, so the three competing authSource conventions are gone. OpenRouter really has no injectedtokenManager, soTokenManager.sharedthere is fine. - An absolute
OPENCODE_DBis now recognized as an OpenCode source in the menu. - Readers take the DB/JSON decision from
getOpenCodeAuthStores()instead of re-reading env in three places. - Diagnostics no longer mark an empty DB as
ACTIVE. Empty databases showNO USABLE CREDENTIALSand dropped tables showUNREADABLE, and tests assert both. MiniMaxProviderTestsis hermetic now and doesn't depend on whatever MiniMax key the developer has locally. Good.- README troubleshooting and "How It Works" now describe the DB-first rule.
The SQLite reader itself is solid. It opens with SQLITE_OPEN_READONLY, uses defer for close/finalize, and selects credentials deterministically with active DESC, time_updated DESC. That fits the repo rule that usage collection only reads existing credentials and never refreshes them. The WAL-without--shm test case is a nice touch tbh.
What's left is all minor, so I'm approving. The comments are cleanups worth doing before merge:
- A database file with no
credentialtable blocks theauth.jsonfallback. Probably cheap insurance to fix. - One dead clause in
parseSingleSourcemakes a main-threadqueue.synccall on every menu rebuild. - Diagnostics re-implement the readers' checks and can name the wrong file in the legacy multi-file case.
- The
enterpriseUrlmapping is dead. - Dropped rows are silent, and SQLite errors are logged as bare codes.
- Two README lines still say
auth.json.
Not inline (outside the diff or non-blocking)
scripts/query-*.shanddocs/AI_USAGE_API_REFERENCE.mdstill read~/.local/share/opencode/auth.jsondirectly. On 2.x they'll keep returning the stale tokens this PR fixes in the app. You called this out as follow-up, which is fair. Just don't let it rot, sinceAGENTS.mdpoints devs at those scripts for quota checks.testOpenCodeDatabasePrecedenceAndCredentialSelectionpacks about 10 scenarios into one function, and theXDG_DATA_HOME/OPENCODE_DBsave/restore fixture is copy-pasted acrossTokenManagerTestsandMiniMaxProviderTests. A smallwithOpenCodeDataHomehelper plus a few focused tests would make failures easier to read. Not blocking.- The
XDG_DATA_HOME SET / NOT SETblock is still duplicated across the two debug-report builders, and neither printsOPENCODE_DBor the resolved DB path. That path is what decides the store now, so it's worth logging. - Last round's data-dir duplication point: your reason (the DB has one default location, while legacy JSON keeps the macOS Application Support fallback) holds up, and the "absolute path shows as Unknown" symptom is fixed. Dropping it.
Validation signal
- Smoke tests: skipped. This is a fork PR and the fork source wasn't available to the isolated smoke job (
OVERALL status=skipped), so I have no independent build or test signal. - CI on
26309a8: Lint passed. Test, CI, and Build and Release were still running when the context was fetched. - The author reports 314 tests passing locally (15 skipped), plus a manual app and CLI run against a live
opencode.db. SwiftLint wasn't available locally, but the Lint workflow passed.
Additional findings
3 findings — expand
- delete: The OpenCode source check in the status bar's source parser has a redundant label-equality clause. Every value that clause can match already contains "opencode" (the database path, the auth.json path, and the fallback label), so the earlier substring test returns first. Only the database-path comparison for a custom absolute OPENCODE_DB adds anything. [CopilotMonitor/CopilotMonitor/App/StatusBarController.swift]
- yagni: getOpenCodeDatabasePath re-implements XDG_DATA_HOME and home-directory resolution that buildOpenCodeFilePaths already provides for auth.json. It is also widened to non-private only so the status bar can compare source strings. Two copies of the data-directory rule must now change together. [CopilotMonitor/CopilotMonitor/Services/TokenManager.swift, CopilotMonitor/CopilotMonitor/App/StatusBarController.swift]
- shrink: The two near-identical auth debug-dump routines in TokenManager still duplicate the XDG_DATA_HOME report block and the credential-store search section. The PR had to edit both copies identically, so one shared routine would be smaller and could not drift. [CopilotMonitor/CopilotMonitor/Services/TokenManager.swift]
Unresolved last round (not re-raised this round)
CopilotMonitor/CopilotMonitor/Services/TokenManager.swift— Metadata mapping clobbers top-level accountId: account ID can silently vanish (round 1)CopilotMonitor/CopilotMonitor/Providers/OpenRouterProvider.swift— Three authSource conventions now: Token From row can disappear (round 1)CopilotMonitor/CopilotMonitor/Services/TokenManager.swift— Data-dir resolution duplicated and hand-rolled: absolute OPENCODE_DB shows as Unknown (round 1)CopilotMonitor/CopilotMonitor/Providers/MiniMaxProvider.swift— MiniMax test still asserts old literal: fails on any machine with a MiniMax key (round 1)CopilotMonitor/CopilotMonitor/Services/TokenManager.swift— DB-vs-JSON decided by re-reading env in 3 places: reader can drift from path picker (round 1)CopilotMonitor/CopilotMonitor/Services/TokenManager.swift— Diagnostics mark an empty DB ACTIVE: troubleshooting output contradicts reality (round 1)
Probe for the credential table before choosing the SQLite auth store. Preserve legacy auth.json discovery for V1 databases that only contain session data while keeping database errors and invalid credential schemas authoritative. Report the sources selected by each auth reader, include actionable SQLite errors and rejected-row diagnostics, remove unused metadata mapping and redundant menu synchronization, and clarify V1/V2 discovery in the README.
Author
|
Addressed round 2 in 548e815. The V1 database compatibility concern is confirmed against OpenCode v1.2.0 source:
Store discovery now probes Other changes:
Validation: full Debug suite, 314 tests, 15 skipped, 0 failures. Relaunched the rebuilt menu app and confirmed database loading in its logs. The rebuilt CLI fetched live ChatGPT usage. SwiftLint remains unavailable locally. |
Contributor
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Initial model: Model usage by lane
gantt
dateFormat x
axisFormat %H:%M
todayMarker off
section Main
claude-opus-5-5 : 1792481394429, 1792481855649
section ContextAudit
claude-sonnet-5-5 : 1792481447764, 1792481597891
section ContextAudit.lane-e-ponytail-audit
claude-sonnet-5-5 : 1792481530198, 1792481588777
section ContextAudit.ResolvedIssues
claude-sonnet-5-5 : 1792481456100, 1792481515914
section DependencyContext
claude-sonnet-5-5 : 1792481448488, 1792481488748
section lane-a-slop
claude-sonnet-5-5 : 1792481448608, 1792481510134
section lane-b-dry
claude-sonnet-5-5 : 1792481447901, 1792481499340
section lane-c-reviewer
claude-opus-5-5 : 1792481448595, 1792481622670
section lane-d-ponytail
claude-sonnet-5-5 : 1792481448574, 1792481506069
section RepoContext
claude-sonnet-5-5 : 1792481448587, 1792481511413
Input excludes cached tokens. Output includes reasoning tokens. Speed reflects the requested API tier; provider acceptance is not recorded. |
Contributor
There was a problem hiding this comment.
Summary
Round 3, and honestly this one landed. Every open item from last round is fixed in 548e815:
- Store selection.
getOpenCodeAuthStores()now probessqlite_masterfor acredentialtable before treatingopencode.dbas authoritative. A V1 install that only keeps session data in the DB falls back toauth.jsonagain. The probe is read-only, and the test covers it. - Diagnostics.
[ACTIVE]now comes straight fromlastFoundAuthPath/lastFoundGeminiOAuthPath, the paths the readers actually picked. An empty DB shows up asREADABLEinstead ofACTIVE, and the two copy-pasted 60-lineauth.jsondump blocks are gone. Nice net deletion. - Debuggability. Skipped rows now log the
integration_idand the reason. SQLite errors carrysqlite3_errmsg, so "database is locked" and "no such table" no longer look identical. - Source labels. All nine providers plus the Claude/Copilot/OpenAI account builders read one
openCodeAuthSourceLabel. That replaces the hard-coded~/.local/share/opencode/auth.jsonliterals, so the "Token From" row can't disappear anymore. The redundant main-threadqueue.syncin the menu label parser is gone too. - Dead mapping and tests. The dead
enterpriseUrlmapping is removed. MiniMax's success test now uses a fixtureauth.jsoninstead of the dev's real key. - README. "How It Works" and troubleshooting describe the DB-first rule and the
OPENCODE_DB/XDG_DATA_HOMEoverrides. The two stale lines are fixed.
Functional benefit: OpenCode 2.x users get their live credentials from the credential table instead of expired leftovers in auth.json. That fixes the "expired token while OpenCode itself works" bug for every OpenCode-backed provider.
What's left is two minor simplifications in TokenManager.swift. Neither blocks:
readGeminiOAuthAuthandreadOpenCodeAuthstill run the same store loop. This PR added a second copy of the readability guard to keep them in sync.readOpenCodeAuthDatarepeats theNSErrorbuilder 3 times and advances the cursor in 2 places.
Both are inline.
Non-blocking notes (no action required)
openCodeAuthSourceLabelis read after each provider's network calls, not when the key was read. A concurrent cache refresh elsewhere could in theory change the label in between. It only affects the display, and both readers resolve the same store set, so I'm not pushing on it.- The new behavior is covered by one 120-line test,
testOpenCodeDatabasePrecedenceAndCredentialSelection. One failure there hides every later assertion. Splitting it with a shared env/temp-dir fixture would also let you drop the duplicate save/restore block inMiniMaxProviderTests. The 401 / sub-1% / row-selection MiniMax tests stillXCTSkipwithout a real key. - Two Gemini lines outside the diff (
TokenManager.swift:4612,:5084) still fall back to the literal"auth.json", whileopenCodeAuthSourceLabelfalls back to"OpenCode". Worth aligning someday.
Validation
- Smoke tests: skipped. The fork source wasn't available to the isolated smoke job (
fork-source-unavailable), so this review has no local test/typecheck signal. - CI at review time: Lint passed. CI, Test, and Build and Release were still running.
- Dependencies: none added or changed. The only new import is the system
SQLite3module in tests. I found no known-critical issue in the existing manifests. - Env vars:
OPENCODE_DBandXDG_DATA_HOMEare existing OpenCode/XDG conventions that take arbitrary paths, not new single-value knobs. No env-surface concern.
Verdict
APPROVE. No blocking issues remain, and the two inline items are cleanup you can do here or in a follow-up.
Additional findings
2 findings — expand
- shrink: The new SQLite credential path opens the database several times per lookup (a schema probe in store discovery, a full read, and another full read in the diagnostics listing). It also turns each row into legacy-shaped JSON, serializes it to Data, and decodes it again, so the legacy decoder stays the only model. Rows could map straight to the auth model in one open, with the probe and the read sharing one connection. [CopilotMonitor/CopilotMonitor/Services/TokenManager.swift]
- shrink: The database-path resolver re-implements the data-home and home-directory resolution that the shared path builder already does for auth.json, with a second non-empty-env check, so two places must change together when the OpenCode data directory rules change. [CopilotMonitor/CopilotMonitor/Services/TokenManager.swift, CopilotMonitor/CopilotMonitor/App/StatusBarController.swift]
Unresolved last round (not re-raised this round)
CopilotMonitor/CopilotMonitor/Services/TokenManager.swift— enterpriseUrl copied into payload nobody decodes: dead mapping (round 1)CopilotMonitor/CopilotMonitor/Services/TokenManager.swift— Diagnostics re-implement the readers' checks: ACTIVE can name a store no reader picked (round 1)CopilotMonitor/CopilotMonitor/App/StatusBarController.swift— Dead label comparison: extra main-thread queue.sync on every menu rebuild (round 1)CopilotMonitor/CopilotMonitor/Services/TokenManager.swift— Rows dropped silently and SQLite errors logged as bare codes: undebuggable in the field (round 1)README.md— Two README lines still say auth.json only: 2.x users are pointed at the wrong store (round 2)CopilotMonitor/CopilotMonitor/Services/TokenManager.swift— DB treated as authoritative by file existence alone: an opencode.db without a credential table disables auth.json (round 1)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OpenCode 2.x maintains credentials in the SQLite database after importing legacy
auth.json. Reading the old file can therefore produce expired-token errors even while OpenCode itself works.credentialtable withSQLITE_OPEN_READONLY, selecting the active credential per integration or the most recently updated entry.XDG_DATA_HOMEand absolute or relativeOPENCODE_DBpaths.Validation
TokenManagerTestsandOpenCodeAuthDecodingTests).opencode.db.git diff --checkpassed. SwiftLint was unavailable locally.The database is read-only; credential refresh remains owned by OpenCode.