Repository navigation
Conversation
📝 WalkthroughWalkthroughSnapshot handling now distinguishes empty Claude OAuth credentials from captured and unknown Keychain values. Capture records empty scoped and legacy credentials, and restore paths write null for empty values. A new test covers selection after a partial legacy Keychain write failure, retry, and deselection. Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to If a Keychain write partially fails when the original entries are absent, rollback can remove managed credentials while the managed account remains selected. Fix that path before merging. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
8d0c72c2-ced6-429c-ab4c-2d67f6b68621
📒 Files selected for processing (5)
src/main/claude-accounts/runtime-auth-service-keychain-snapshots.test.tssrc/main/claude-accounts/runtime-auth/runtime-auth-keychain-snapshots.tssrc/main/claude-accounts/runtime-auth/runtime-auth-snapshot-capture.tssrc/main/claude-accounts/runtime-auth/runtime-auth-snapshot-restore.tssrc/main/claude-accounts/runtime-auth/runtime-auth-types.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| const scopedKeychainSnapshot = this.snapshotKeychainCredentials( | ||
| scopedKeychainCredentials.credentialsJson, | ||
| options.previousSnapshot, | ||
| 'scoped', | ||
| options.managedCredentialsJson | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
set -eu
base=2dc98c93de675947958960061a335bcfbfb3f838
head=5cfcc85508ae37f9ee09a813a4219b42f53d9263
file=src/main/claude-accounts/runtime-auth/runtime-auth-snapshot-capture.ts
printf '%s\n' '--- diff ---'
git diff --no-ext-diff --unified=50 "$base" "$head" -- "$file"
printf '%s\n' '--- current file ---'
nl -ba "$file"
printf '%s\n' '--- relevant symbols and test references ---'
rg -n -F -e 'hasEmptyClaudeKeychainSnapshot' -e 'snapshotKeychainCredentials' -e 'partial' srcRepository: stablyai/orca
Length of output: 45130
🏁 Script executed:
set -eu
base=2dc98c93de675947958960061a335bcfbfb3f838
head=5cfcc85508ae37f9ee09a813a4219b42f53d9263
file=src/main/claude-accounts/runtime-auth/runtime-auth-snapshot-capture.ts
printf '%s\n' '--- diff ---'
git diff --no-ext-diff --unified=50 "$base" "$head" -- "$file"
printf '%s\n' '--- current file ---'
nl -ba "$file"
printf '%s\n' '--- relevant symbols and test references ---'
rg -n -F -e 'hasEmptyClaudeKeychainSnapshot' -e 'snapshotKeychainCredentials' -e 'partial' src || test "$?" -eq 1Repository: stablyai/orca
Length of output: 45278
🏁 Script executed:
set -eu
dir=src/main/claude-accounts/runtime-auth
printf '%s\n' '--- runtime-auth files ---'
rg --files "$dir"
printf '%s\n' '--- exact symbol references ---'
rg -n -C 24 -F -e 'snapshotKeychainCredentials' -e 'hasEmptyClaudeKeychainSnapshot' -e 'scopedKeychainCredentialsEmpty' -e 'legacyKeychainCredentialsEmpty' "$dir" || test "$?" -eq 1
printf '%s\n' '--- related test declarations ---'
rg -n -C 8 'partial|rollback|managed.*selected|Keychain.*null|initial.*Keychain' "$dir" -g '*test*' || test "$?" -eq 1Repository: stablyai/orca
Length of output: 36529
🏁 Script executed:
set -eu
dir=src/main/claude-accounts
printf '%s\n' '--- claude-account test files ---'
rg --files "$dir" | rg '(^|/)[^/]*(test|spec)\.'
printf '%s\n' '--- restore callers and rollback references ---'
rg -n -C 10 -F -e 'restoreSystemDefaultSnapshot' -e 'hasUnchangedActiveClaudeKeychainCredentials' -e 'writeActiveClaudeKeychainCredentials' -e 'removeActiveClaudeKeychainCredentials' "$dir" || test "$?" -eq 1
printf '%s\n' '--- complete restore implementation ---'
nl -ba "$dir/runtime-auth/runtime-auth-snapshot-restore.ts"
printf '%s\n' '--- keychain snapshot operations ---'
nl -ba "$dir/runtime-auth/runtime-auth-keychain-snapshots.ts" | sed -n '35,180p'Repository: stablyai/orca
Length of output: 42616
🏁 Script executed:
set -eu
dir=src/main/claude-accounts
printf '%s\n' '--- sync write and rollback blocks ---'
rg -n -C 28 'writeActiveClaudeKeychainCredentials|restoreSystemDefaultSnapshot|catch \(error\)' "$dir/runtime-auth/runtime-auth-sync.ts" || test "$?" -eq 1
printf '%s\n' '--- ownership and file restoration helpers ---'
rg -n -C 20 'hasUnchangedRuntimeCredentials|hasUnchangedActiveClaudeKeychainCredentials|restoreRuntimeCredentials' "$dir/runtime-auth"
printf '%s\n' '--- keychain snapshot test cases ---'
rg -n -C 24 'partial|legacy|rollback|empty token|empty OAuth|Keychain' "$dir/runtime-auth-service-keychain-snapshots.test.ts" || test "$?" -eq 1Repository: stablyai/orca
Length of output: 42647
🏁 Script executed:
set -eu
dir=src/main/claude-accounts
printf '%s\n' '--- Keychain read/write implementations ---'
rg -n -C 16 -F -e 'readActiveClaudeKeychainCredentialsStrict' -e 'writeActiveClaudeKeychainCredentialsForRuntime' "$dir/keychain.ts"
printf '%s\n' '--- ownership helpers ---'
rg -n -C 18 -F -e 'hasUnchangedRuntimeCredentials' -e 'hasUnchangedActiveClaudeKeychainCredentials' "$dir/runtime-auth"
printf '%s\n' '--- empty predicate call sites ---'
rg -n -F 'isEmptyClaudeOAuthCredentials(' "$dir"
printf '%s\n' '--- focused test setup and relevant test ---'
sed -n '1,115p' "$dir/runtime-auth-service-keychain-snapshots.test.ts"
sed -n '542,635p' "$dir/runtime-auth-service-keychain-snapshots.test.ts"Repository: stablyai/orca
Length of output: 26799
🏁 Script executed:
set -eu
dir=src/main/claude-accounts
printf '%s\n' '--- shared credential merge definition ---'
rg -n -C 18 -F 'mergeSharedClaudeCredentialFields' "$dir"
printf '%s\n' '--- runtime credential restore definition ---'
rg -n -C 18 -F 'restoreRuntimeCredentials(' "$dir/runtime-auth"Repository: stablyai/orca
Length of output: 41590
Treat null Keychain captures as empty rollback targets.
A successful Keychain read with no entry is recorded as captured with null, so the selected-account rollback guard is skipped. If the scoped write succeeds and the legacy write fails, rollback can restore the system credentials file and clear the managed OAuth credentials from the scoped Keychain while the account remains selected.
Classify null as empty. Also keep the pending rollback eligible for restoration if the account is deselected before a retry. Add a partial-write test with both initial Keychain entries set to null; check that managed auth survives the failure and that deselection restores the system file and clears the entries.
Suggested fix
diff --git a/src/main/claude-accounts/runtime-auth/runtime-auth-keychain-snapshots.ts b/src/main/claude-accounts/runtime-auth/runtime-auth-keychain-snapshots.ts
@@
- if (this.isEmptyClaudeOAuthCredentials(credentialsJson)) {
+ if (credentialsJson === null || this.isEmptyClaudeOAuthCredentials(credentialsJson)) {
return { status: 'empty' }
}
diff --git a/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts b/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts
@@
- if (this.lastSyncedAccountId !== null) {
+ if (this.lastSyncedAccountId !== null || this.lastWrittenCredentialsJson !== null) {
await (previousAccount
? this.restoreSystemDefaultSnapshot(
|
Closing as obsolete. Current
The CodeRabbit note about treating a null Keychain capture as an empty rollback target applied to that removed restore path. I am not porting it onto the new account-folder model in this PR. |
1 participant
Summary
Do not restore an empty Claude OAuth Keychain snapshot while a managed Claude account remains selected. Empty scoped and legacy values are recorded as metadata-only markers, so rollback keeps the current managed runtime auth and the next sync can retry. Deselecting managed auth still clears empty Keychain entries.
Screenshots
No visual change.
Testing
pnpm lintpnpm typecheckpnpm testpnpm buildAI Review Report
Reviewed snapshot capture, compatibility with older persisted snapshots, and both restoration paths. The regression test proves the rollback guard prevents an empty snapshot from replacing the selected managed auth. The change adds no platform-specific paths, shell commands, shortcuts, or Electron behavior; Keychain access remains in the existing macOS-only path, and Linux and Windows behavior is unchanged.
Security Audit
Empty OAuth Keychain JSON is omitted from the snapshot; only empty-state flags are persisted. Non-empty snapshots retain their existing handling. No commands, paths, dependencies, or IPC were added. The Keychain write failure cause remains unknown and is not attributed to this change.
Notes
CI and maintainer review have not been run.