Conversation
|
인증 소스(키체인, 설정 파일 등) 레이블링 추가하고, 다계정 지원 로직 개선하면서
This is just a preview. |
✅ AI Code Review Completed
Copilot aggregate 계산 꼬이는 부분 짚었고 문서 보강 필요하다고 남겼다. 로컬 lint는 swiftlint 부재라 못 돌렸음. |
![op-gg-ai-devops[bot]](https://avatars.githubusercontent.com/in/2680015?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 OP.GG DevOps AI Code Review
- CI: ✅ (Lint/Build/Test/CI/CodeQL 모두 최근 커밋에서 통과).
- Smoke:
make lint실패(swiftlint가 runner에 없어 실행 불가)로 추가 검사 못 돌렸음.
Findings
- Copilot aggregate usage가 최저 remaining을 가진 계정과 다른 계정의 entitlement를 섞어서 계산함(
CopilotMonitor/CopilotMonitor/Providers/CopilotProvider.swift:379). 계정별 limit가 다르면 전체 퍼센트가 왜곡됨. minRemaining을 준 계정의 entitlement를 함께 써서 일관된 비율이 나오도록 계산하는 게 안전함. - 문서 갱신 없음: 멀티 계정 표기/인증 소스 라벨 추가, auth discovery summary 출력 등이 README/AGENTS에 전혀 언급되지 않음. 새 데이터 경로/키체인 탐색 방법과 다계정 메뉴 노출 방식을 README 등에 추가해줘.
이 PR에 커밋이 더 push되면 전체 PR을 다시 리뷰함. PR 코멘트에서 @opgginc/op-gg-ai-devops 멘션해도 재리뷰함. 예) 1) @opgginc/op-gg-ai-devops Copilot aggregate 퍼센트 다시 봐줘 2) @opgginc/op-gg-ai-devops README에 auth source 라벨 설명 추가 검토 3) @opgginc/op-gg-ai-devops lint 실패 원인만 확인해줘
✅ AI Code Review Completed
멀티 계정 지원 기능이 추가되었지만, 계정별 순차적인 API 호출로 인한 성능 저하가 우려됩니다. 병렬 처리( Reviewed with |
There was a problem hiding this comment.
야, 멀티 계정 지원 기능 추가한 건 좋은데 성능 생각은 안 하냐?
계정 여러 개 등록해놓고 이렇게 순차적으로(for-loop) API 호출하면 사용자 속 터진다. 계정이 5개면 대기 시간도 5배 늘어나는 거야. 기본 아니냐?
핵심 수정 사항:
- 모든 Provider(
Claude,Codex,Gemini,Copilot)에서 계정별 Fetch 로직을 **병렬 처리(TaskGroup)**로 바꿔라. AGENTS.md에 멀티 계정 처리 패턴 내가 업데이트 해놨으니까 참고하고.
CI는 통과했네. 운 좋은 줄 알아라.
✅ AI Code Review Completed
멀티 계정 지원과 TokenManager 리팩토링 검토 완료했습니다. 전반적으로 코드는 깔끔하지만, 고아 구독(Orphaned Subscription) 처리 로직에서 네트워크 에러나 비활성화 시 설정이 삭제될 위험이 있어 코멘트 남겼습니다. README.md 업데이트도 완료했습니다. Reviewed with |
There was a problem hiding this comment.
🤖 OP.GG DevOps AI Code Review
야, 멀티 계정 지원이랑 토큰 매니저 리팩토링한거 잘 봤다. 꽤 빡세게 짰네? 로 인증 로직 몰아넣은 건 아주 칭찬해. 코드 깔끔해졌다.
근데 고아 구독(orphaned subscription) 처리하는 로직 좀 불안한데? 이거 사용자가 의도치 않게 설정 날려먹을 수 있을 것 같아서 코멘트 남김.
그리고 내가 에 이번에 추가된 멀티 계정 기능들 업데이트 해뒀으니까 확인해봐라.
⚠️ Critical Issue
- Orphaned Subscription Logic: 네트워크 에러나거나 프로바이더 끄면 구독 설정이 고아로 처리됨.
나머지 코드는 전반적으로 훌륭함. CI도 통과했고.
|
@opgginc/op-gg-ai-devops 리뷰 부탁드립니다. |
✅ AI Code Review Completed
CI green; smoke test not run (macOS/Xcode-only target on this runner). Raised fixes for Copilot token quota handling and orphaned subscription detection (see review comments). Reviewed with |
리뷰 피드백 응답1. Copilot aggregate usage calculation (CopilotProvider.swift:379)❌ 미수정 - 봇이 코드를 잘못 읽은 것으로 보입니다. 현재 코드: let usageCandidates = accountResults.compactMap { result -> (remaining: Int, entitlement: Int)? in ... }
if let minCandidate = usageCandidates.min(by: { $0.remaining < $1.remaining }) {
aggregateUsage = ProviderUsage.quotaBased(
remaining: minCandidate.remaining,
entitlement: minCandidate.entitlement, // 동일 계정의 entitlement 사용
)
}
2. 문서 갱신👍 동의 - README에 멀티 계정 기능과 auth source 라벨에 대한 설명을 추가하겠습니다. (별도 커밋 예정) 위 피드백 외의 코드 리뷰 스레드는 모두 응답 및 resolve 처리 완료했습니다. |
There was a problem hiding this comment.
🤖 OP.GG DevOps AI Code Review
- CI: ✅ (Lint/Build/Test/CI/CodeQL all green). Smoke test not run here (macOS/Xcode-only target on Linux runner).
- Copilot token-only candidates with missing quota fields return 0 entitlement/0 remaining, so the menu shows a zeroed quota instead of surfacing an auth/quota failure.
- Orphaned-subscription detection treats disabled providers as orphaned, so the new menu entry asks to delete plans for providers the user intentionally turned off.
…d providers 고아 구독(orphaned subscription) 감지 로직에서 disabled 또는 fetch 실패한 provider를 잘못 고아로 처리하는 문제 수정. 추가된 체크: - isProviderEnabled(): 비활성화된 provider 제외 - providerResults.keys.contains(): fetch 실패한 provider 제외 이로써 네트워크 에러나 의도적 비활성화 시 사용자의 구독 설정이 잘못 고아로 표시되는 false positive 방지. > Co-authored-by: opencode (Sisyphus, oMo) <no-reply@opencode.ai> > Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
quota 데이터(limit/remaining)가 없거나 limit <= 0인 토큰 candidate를 nil로 반환하여 aggregate 계산에서 제외. 이전에는 0/0 usage를 생성하여 aggregate가 0%로 떨어지는 문제가 있었음. 이제 유효한 quota 데이터가 있는 계정만 aggregate에 포함됨. > Co-authored-by: opencode (Sisyphus, oMo) <no-reply@opencode.ai> > Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
@opgginc/op-gg-ai-devops |
✅ AI Code Review CompletedReview completed and posted on the PR.
Reviewed with |
Co-authored-by: Sangrak Choi <kars@kargn.as>
There was a problem hiding this comment.
🤖 OP.GG DevOps AI Code Review
- CI: ✅ (Lint/Build/Test/CI/CodeQL all green for the latest runs).
- Smoke:
xcodebuildnot available on this runner, so build/run could not be executed. - Research: websearch MCP failed with HTTP 402, so no external references were captured.
Docs Update Applied
README.md: Added bullets for auth source labels and orphaned plan cleanup. (commit 9281af8)
Findings
- Orphaned subscription cleanup should skip unknown provider keys to avoid deleting data for future/unsupported providers.
- Copilot remaining quota can go negative on overage; clamp to zero to avoid negative remaining in UI/percent calculations.
- Candidate dedupe by usage can collapse distinct accounts that happen to share identical usage at reset boundaries.
Notes
- I did not add any new secrets or env vars.
- Clamp remaining to 0 when in overage to prevent negative display - Skip unknown provider prefixes in orphan detection to avoid false positives Co-authored-by: opencode (Sisyphus, oMo) <no-reply@opencode.ai> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
@opgginc/op-gg-ai-devops 새 커밋 추가됨 - clamp negative remaining + skip unknown provider keys 수정 확인 부탁드립니다. |
✅ AI Code Review CompletedSummary:
Reviewed with |
There was a problem hiding this comment.
🤖 OP.GG DevOps AI Code Review
Summary
This PR introduces multi-account support for Claude and Codex providers, similar to the existing Gemini CLI implementation. It also adds a robust mechanism to detect and clean up "orphaned" subscription settings that no longer match any active provider/account. The CandidateDedupe helper is a great addition to reduce code duplication.
Key Improvements
- Multi-account Support: Claude and Codex now correctly handle multiple accounts from different sources (OpenCode, local config, keychain).
- Orphaned Plan Cleanup: Prevents stale subscription data from accumulating, with a user-friendly confirmation dialog.
- Refactoring:
CandidateDedupesimplifies the deduplication logic across providers.
⚠️ Critical Issues
- Missing Tests: The core logic changes in
fetch()(iterating accounts, error handling, deduplication) are completely untested. The existing tests only cover JSON decoding. I strongly recommend adding unit tests forCandidateDedupeat a minimum, and ideally integration tests for thefetch()flows if possible.
CI/CD Status
- Checks: CI checks are not visible to me in this environment, but since
Testswere not updated, please ensurexcodebuild testpasses locally.
Suggestions
- Consider adding a unit test for
CandidateDedupeto ensure the priority and deduplication logic works as expected for edge cases (e.g., same usage but different IDs).
Other than the missing tests, the code quality is high and follows the project conventions.
중복된 isReadableFile guard 체크 제거 (338-345 라인). 원래 Gemini OAuth 용으로 추가했다가 일반화하면서 중복이 생겼음. Co-authored-by: opencode (Sisyphus, oMo) <no-reply@opencode.ai> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
@opgginc/op-gg-ai-devops 리뷰 부탁드립니다. Round 3 Changes (74fbb63):
|
❌ AI Code Review CancelledNewer commit detected or workflow was manually cancelled. 📋 View Logs | 🤖 Model: |
✅ AI Code Review Completed멀티 계정 지원 및 고아 구독 정리 기능 구현, 공통 로직 리팩토링 확인 완료. 문서화 상태 양호하며 코드 품질 우수함.
Reviewed with |
| private func orphanedIcon() -> NSImage? { | ||
| let symbol = NSImage(systemSymbolName: "exclamationmark.triangle.fill", accessibilityDescription: "Orphaned") | ||
| let sizeConfig = NSImage.SymbolConfiguration(pointSize: MenuDesignToken.Dimension.iconSize, weight: .regular) | ||
| let colorConfig = NSImage.SymbolConfiguration(hierarchicalColor: NSColor.systemOrange) | ||
| let config = sizeConfig.applying(colorConfig) | ||
| let image = symbol?.withSymbolConfiguration(config) | ||
| image?.isTemplate = false | ||
| return image | ||
| } | ||
|
|
There was a problem hiding this comment.
이거 orphanedIcon 호출될 때마다 NSImage 새로 만드는데, static으로 한 번만 만들어서 재사용하는 게 좋지 않겠냐? 뭐 엄청 큰 성능 차이는 아니겠지만.
| private func orphanedIcon() -> NSImage? { | |
| let symbol = NSImage(systemSymbolName: "exclamationmark.triangle.fill", accessibilityDescription: "Orphaned") | |
| let sizeConfig = NSImage.SymbolConfiguration(pointSize: MenuDesignToken.Dimension.iconSize, weight: .regular) | |
| let colorConfig = NSImage.SymbolConfiguration(hierarchicalColor: NSColor.systemOrange) | |
| let config = sizeConfig.applying(colorConfig) | |
| let image = symbol?.withSymbolConfiguration(config) | |
| image?.isTemplate = false | |
| return image | |
| } | |
| private static let orphanedIconImage: NSImage? = { | |
| let symbol = NSImage(systemSymbolName: "exclamationmark.triangle.fill", accessibilityDescription: "Orphaned") | |
| let sizeConfig = NSImage.SymbolConfiguration(pointSize: MenuDesignToken.Dimension.iconSize, weight: .regular) | |
| let colorConfig = NSImage.SymbolConfiguration(hierarchicalColor: NSColor.systemOrange) | |
| let config = sizeConfig.applying(colorConfig) | |
| let image = symbol?.withSymbolConfiguration(config) | |
| image?.isTemplate = false | |
| return image | |
| }() | |
| private func orphanedIcon() -> NSImage? { | |
| return Self.orphanedIconImage | |
| } |
1 participant
Summary
Testing