feat(auth): add explicit opencode import adapter#82
feat(auth): add explicit opencode import adapter#82ndycode wants to merge 5 commits intogit-plan/17-health-summary-dashboardfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const candidate of candidates) { | ||
| if (existsSync(candidate)) { | ||
| return candidate; | ||
| } | ||
| } |
There was a problem hiding this comment.
unhandled existsSync throw in candidate loop
existsSync can throw on windows when antivirus holds an exclusive lock on the file, or when the path is a stale unc/network mount. here the call is bare — a single locked candidate crashes the entire detection and bubbles an unhandled error up through assessOpencodeAccountPool into the login loop. wrap in a try/catch and treat any exception as "not found" so the loop continues to the next candidate.
| for (const candidate of candidates) { | |
| if (existsSync(candidate)) { | |
| return candidate; | |
| } | |
| } | |
| for (const candidate of candidates) { | |
| try { | |
| if (existsSync(candidate)) { | |
| return candidate; | |
| } | |
| } catch { | |
| // file locked or inaccessible (e.g. AV hold on Windows) — skip | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/storage.ts
Line: 1537-1541
Comment:
**unhandled `existsSync` throw in candidate loop**
`existsSync` can throw on windows when antivirus holds an exclusive lock on the file, or when the path is a stale unc/network mount. here the call is bare — a single locked candidate crashes the entire detection and bubbles an unhandled error up through `assessOpencodeAccountPool` into the login loop. wrap in a try/catch and treat any exception as "not found" so the loop continues to the next candidate.
```suggestion
for (const candidate of candidates) {
try {
if (existsSync(candidate)) {
return candidate;
}
} catch {
// file locked or inaccessible (e.g. AV hold on Windows) — skip
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
| if (menuResult.mode === "import-opencode") { | ||
| const assessment = await assessOpencodeAccountPool({ | ||
| currentStorage, | ||
| }); | ||
| if (!assessment) { | ||
| console.log("No OpenCode account pool was detected."); | ||
| continue; | ||
| } | ||
| if (!assessment.valid || assessment.wouldExceedLimit) { | ||
| console.log( | ||
| assessment.error ?? "OpenCode account pool is not importable.", | ||
| ); | ||
| continue; | ||
| } | ||
| const confirmed = await confirm( | ||
| `Import OpenCode accounts from ${assessment.backup.path}?`, | ||
| ); | ||
| if (!confirmed) { | ||
| continue; | ||
| } | ||
| await runActionPanel( | ||
| "Import OpenCode Accounts", | ||
| `Importing from ${assessment.backup.path}`, | ||
| async () => { | ||
| const imported = await importAccounts(assessment.backup.path); | ||
| console.log( | ||
| `Imported ${imported.imported} account${imported.imported === 1 ? "" : "s"}. Skipped ${imported.skipped}. Total accounts: ${imported.total}.`, | ||
| ); | ||
| }, | ||
| displaySettings, | ||
| ); | ||
| continue; |
There was a problem hiding this comment.
TOCTOU race between assessment preview and actual import
assessOpencodeAccountPool reads the opencode pool file at time T₁ to build the preview shown to the user. the user can take seconds (or minutes) to confirm. importAccounts then re-reads the same file at time T₂. on a windows desktop, opencode may be running concurrently and can rotate/replace openai-codex-accounts.json between T₁ and T₂ — the user sees "import 2 accounts" but the actual mutation brings in a different set.
the fix is to snapshot the source before showing the confirmation, then pass the snapshotted path (or the already-parsed candidate.normalized data) directly to the write step, rather than re-resolving from disk. at minimum, re-assess after confirmation and bail if the account count has shifted. no regression test currently covers a mid-confirm file mutation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/codex-manager.ts
Line: 4973-5004
Comment:
**TOCTOU race between assessment preview and actual import**
`assessOpencodeAccountPool` reads the opencode pool file at time T₁ to build the preview shown to the user. the user can take seconds (or minutes) to confirm. `importAccounts` then re-reads the same file at time T₂. on a windows desktop, opencode may be running concurrently and can rotate/replace `openai-codex-accounts.json` between T₁ and T₂ — the user sees "import 2 accounts" but the actual mutation brings in a different set.
the fix is to snapshot the source before showing the confirmation, then pass the snapshotted path (or the already-parsed `candidate.normalized` data) directly to the write step, rather than re-resolving from disk. at minimum, re-assess after confirmation and bail if the account count has shifted. no regression test currently covers a mid-confirm file mutation.
How can I resolve this? If you propose a fix, please make it concise.| const assessment = await assessOpencodeAccountPool({ | ||
| currentStorage, | ||
| }); |
There was a problem hiding this comment.
stale currentStorage makes preview misleading
currentStorage is loaded once at the top of the runAuthLogin loop iteration and reused across multiple menu actions. by the time import-opencode is reached the on-disk state may differ (another process added/removed accounts). the assessment preview ("would add N accounts") is shown based on stale data, while importAccounts internally calls withAccountStorageTransaction which re-reads the live storage. pass a freshly loaded storage to assessOpencodeAccountPool here so the preview matches what the transaction will actually do:
| const assessment = await assessOpencodeAccountPool({ | |
| currentStorage, | |
| }); | |
| const freshStorage = await loadAccounts(); | |
| const assessment = await assessOpencodeAccountPool({ | |
| currentStorage: freshStorage, | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/codex-manager.ts
Line: 4974-4976
Comment:
**stale `currentStorage` makes preview misleading**
`currentStorage` is loaded once at the top of the `runAuthLogin` loop iteration and reused across multiple menu actions. by the time `import-opencode` is reached the on-disk state may differ (another process added/removed accounts). the assessment preview ("would add N accounts") is shown based on stale data, while `importAccounts` internally calls `withAccountStorageTransaction` which re-reads the live storage. pass a freshly loaded storage to `assessOpencodeAccountPool` here so the preview matches what the transaction will actually do:
```suggestion
const freshStorage = await loadAccounts();
const assessment = await assessOpencodeAccountPool({
currentStorage: freshStorage,
});
```
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/getting-started.md">
<violation number="1" location="docs/getting-started.md:85">
P2: This new doc line introduces "OpenCode"/"opencode" wording in a user doc, but documentation.test.ts explicitly forbids any "opencode" references in user docs, so CI will fail. Either rephrase to avoid the opencode string or update the test allowlist/policy to permit this reference.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Summary
Summary by cubic
Adds an explicit OpenCode import adapter so users can preview and import an existing OpenCode account pool into the auth manager from the login dashboard. Integrates a new “Import OpenCode Accounts” flow with validation and confirmation before import.
import-opencode) in the Recovery section.CODEX_OPENCODE_POOL_PATH,%LOCALAPPDATA%/OpenCodeoropencode,%APPDATA%/OpenCodeoropencode, or~/.opencode/openai-codex-accounts.json.assessOpencodeAccountPoolwith clear errors and limit checks; no changes on invalid sources.importAccountsand prints a summary (imported, skipped, total).Written for commit 9e04080. Summary will update on new commits.
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
this pr adds an explicit opencode account-pool import adapter: a new
import-opencodelogin mode, auto-detection viadetectOpencodeAccountPoolPath, assessment viaassessOpencodeAccountPool, and a menu action that previews and imports the pool into the auth manager. the storage refactor (assessBackupRestoreFromPath) is clean and the new exported types (NamedBackupMetadata,RotatingBackupMetadata,BackupRestoreAssessment) tighten the contract.key gaps to address before merging:
assessment.importedis available but not shown; users can't make an informed decision, and a zero-import case (all duplicates) still prompts and confuses.null,!valid, andwouldExceedLimitpaths in theimport-opencodehandler are untested; these are the paths most likely to fire on windows due to antivirus locks and stale paths.detectOpencodeAccountPoolPath()is synchronous and could gate visibility cheaply.existsSync, stalecurrentStorage, TOCTOU between assessment and import) remain unresolved and are not addressed in this iteration.Confidence Score: 3/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant U as User participant CM as codex-manager participant S as storage participant FS as Filesystem U->>CM: select import-opencode CM->>S: assessOpencodeAccountPool(currentStorage) S->>FS: existsSync candidates at T1 FS-->>S: path found S->>FS: readFile pool JSON S-->>CM: BackupRestoreAssessment CM->>U: confirm prompt (no count shown) U-->>CM: confirmed CM->>S: importAccounts(backup.path) S->>FS: re-read at T2 S->>S: withAccountStorageTransaction S-->>CM: imported skipped total CM->>U: summary messagePrompt To Fix All With AI
Last reviewed commit: 9e04080
Context used: