Add oc-chatgpt-multi-auth authentication support#120
Conversation
❌ AI Code Review CancelledNewer commit detected or workflow was manually cancelled. 📋 View Logs | 🤖 Model: |
4f428e1 to
77845ff
Compare
✅ AI Code Review Completedreview posting completed. 🤖 OP.GG AI DevOps 코드 리뷰 현황
Reviewed with |
There was a problem hiding this comment.
oc-chatgpt-multi-auth Support — Solid Addition
Clean integration of the ndycode/oc-chatgpt-multi-auth plugin ecosystem into the existing multi-source account discovery pipeline. The approach is well-aligned with the codebase's established patterns.
What's Good
- JWT claim canonicalization — Extracting stable
chatgpt_account_idfrom access token claims instead of relying on potentially org-scopedaccountIdis the right call. This matches the codebase's existingSubscription Key Stability (Email-First Rule)pattern nicely. resolvedOpenAIAuthMetadatacentralization — Instead of scattering JWT decode + field resolution logic, this helper struct consolidates it. All call sites (getOpenAIAccounts, getOpenAIAccountId, diagnostics, debug dump) go through the same path. Clean.- Caching follows existing patterns —
cachedOpenCodeMultiAuthAccounts/ timestamp pair mirrors the existing cache strategy. No surprises. - Test coverage — 3 new test cases covering endpoint config override, field decoding, and account ID canonicalization from JWT. The
makeTestJWThelper is reusable for future tests too. - Documentation is thorough — README, API reference, and auth file format docs all updated with the new paths and behavior. The explanation of why the localhost proxy is ignored for usage requests is particularly helpful.
Minor Observations
- The
readOpenAIMultiAuthFiles(at:)isfunc(internal) rather thanprivate func, which is intentional for test access — makes sense. openCodeMultiAuthPaths()usescontentsOfDirectorywhich returns both files and directories under~/.opencode/projects/, butreadJSONDictionarygracefully handles non-existent paths, so no issue.- The
default.profrawremoval is a nice cleanup.
All CI checks (Lint, Test, CI, Build and Release) passed. No blockers found.
Reviewed with openrouter/qwen/qwen3.6-plus:free
| } | ||
|
|
||
| private func extractOpenAIMultiAuthPayload(from dict: [String: Any]) -> OpenAIMultiAuthPayload? { | ||
| let accessKeys: Set<String> = ["accesstoken", "access", "oauthtoken", "token"] |
There was a problem hiding this comment.
Broad 'id' key match in recursive search: potential false positive
The accountKeys set includes "id" as a normalized match. Since findStringValue does recursive dictionary traversal, this could accidentally match an unrelated id field nested somewhere deeper in an unexpected JSON structure. Given that findDirectStringValue is checked first and the actual account file schema is fairly flat, this is unlikely to be a problem in practice — but worth noting as a potential false-positive source if the plugin's JSON format ever nests objects with their own id fields.
Consider removing "id" from the set or limiting the recursive search depth to 1 level for account key lookup.
| lines.append(" oc-chatgpt-multi-auth accounts (\(shortPath(defaultOpenCodeAccounts.path))): NOT FOUND") | ||
| } else { | ||
| for path in existingOpenCodeMultiAuthPaths { | ||
| let accountCount = readOpenAIMultiAuthFiles(at: [path]).count |
There was a problem hiding this comment.
Redundant file re-parse in diagnostics loop
This calls readOpenAIMultiAuthFiles(at: [path]) per path inside the diagnostics loop, re-parsing each file individually. The cached result from readOpenAIMultiAuthFiles() (no-args, line ~4670) was already loaded above. You could filter the cached accounts by authSource to get per-path counts instead of re-reading and re-parsing each file:
let accountCount = openCodeMultiAuthAccounts.filter { $0.authSource == path.path }.countNot a correctness issue, but avoids redundant file I/O in the diagnostics path.
Summary
Testing