Refactor AI Providers

[modelorona]

goal is to make it easier to extend providers, add settings for models via the UI and env variables, and prevent pre-defined providers from being deleted

[jazzberry-ai[bot]]

An error occured.

_{This error may be due to rate limits. If this error persists, please email us.}

[github-actions[bot]]

Hey, leave a @claude-review comment below and I'll do a code review!

[modelorona]

@claude-review

[claude[bot]]

PR Review: Refactor AI Providers

This PR successfully refactors the AI provider system. Overall excellent work with clean architecture.

---

Critical Issues

1. API Key Exposure Risk (HIGH PRIORITY)

• Location: core/graph/schema.resolvers.go:165-173, 221-229, 848-856
• Issue: Settings are marshaled and returned without filtering sensitive fields
• Risk: If API keys are stored in settings map, they would be exposed in GraphQL responses
• Fix: Add sanitization to filter apiKey/api_key fields before marshaling settings

2. Ignored JSON Marshal Errors (MEDIUM)

• Location: core/graph/schema.resolvers.go:156, 212, 839
• Issue: Errors from json.Marshal are silently ignored
• Fix: Handle errors with proper logging

3. Missing License Header (MEDIUM)

• Location: core/graph/generated.go:1
• Issue: Copyright header was removed from generated file
• Check if code generator config needs adjustment

---

Medium Issues

4. Provider Persistence Missing

• Location: core/src/llm/provider_registry.go:121, 152, 174
• User-created providers lost on restart
• Create follow-up issue for implementation

5. Potential Race Condition

• Location: core/src/llm/llm_instance.go:28-65
• Mutex unlocked before reading from provider registry
• Consider extended lock scope or document safety

6. HTTP Resolver Changes

• Location: core/graph/http.resolvers.go:152-197
• Changes appear justified but against stated guidelines
• Worth noting for future reviews

---

Strengths

• Clean ProviderRegistry singleton pattern
• Proper GraphQL-first approach
• No SQL injection vulnerabilities
• Plugin architecture respected
• CE/EE separation maintained
• Modern Go syntax throughout
• Secure API key handling in memory

---

Recommendation

Approve with Changes - Solid refactor but address API key exposure before merge.

Priority:

1. Add settings sanitization
2. Handle marshal errors
3. Track persistence as follow-up

Excellent architecture improvements overall!

[modelorona]

closing as too many changes since then, need to do it again and update