agoraio/review-case

Review Case Skill — macOS

When to Use

Use this skill when you need to:

• Review a new or modified example for correctness
• Ensure the example follows project conventions
• Verify lifecycle management and thread safety
• Check API usage and error handling

Review Dimensions (Priority Order)

1. Engine Lifecycle (CRITICAL)

Check:

• [ ] Engine is created in initializeAgoraEngine() or similar
• [ ] Engine is initialized with AgoraRtcEngineConfig
• [ ] leaveChannel() is called before destroy()
• [ ] destroy() is called in viewWillClose() or cleanup method
• [ ] No engine leaks (engine not recreated on every join)

Correct Pattern:

override func viewDidLoad() {
    super.viewDidLoad()
    initializeAgoraEngine()  // Create once
}

override func viewWillClose() {
    leaveChannel()
    agoraKit.destroy()
    super.viewWillClose()
}

func joinChannel() {
    agoraKit.joinChannel(byToken: token, channelName: channel, info: nil, uid: 0)
}

func leaveChannel() {
    agoraKit.leaveChannel(nil)
}

Incorrect Pattern: See references/incorrect-lifecycle.swift for common mistakes.

---

2. Thread Safety (CRITICAL)

Check:

• [ ] All UI updates in delegate callbacks use DispatchQueue.main.async
• [ ] No direct UI updates from background threads
• [ ] Video/audio frame callbacks dispatch to main thread before updating UI

Correct Pattern:

func rtcEngine(_ engine: AgoraRtcEngineKit, didJoinChannel channel: String, withUid uid: UInt, elapsed: Int) {
    // Callback may arrive on background thread
    DispatchQueue.main.async {
        self.statusLabel.stringValue = "Joined channel"
    }
}

Incorrect Pattern: See references/incorrect-thread-safety.swift for common mistakes.

---

3. Permission Handling (HIGH)

Check:

• [ ] Microphone permission requested before enableAudio()
• [ ] Camera permission requested before enableVideo()
• [ ] Permissions checked before accessing devices

Correct Pattern:

func initializeAgoraEngine() {
    // Request permissions first
    AVCaptureDevice.requestAccess(for: .video) { granted in
        if granted {
            self.agoraKit.enableVideo()
        }
    }
    
    AVAudioSession.sharedInstance().requestRecordPermission { granted in
        if granted {
            self.agoraKit.enableAudio()
        }
    }
}

---

4. Error Handling (HIGH)

Check:

• [ ] joinChannel() failures are handled
• [ ] Token expiration is handled
• [ ] Network errors are logged or displayed
• [ ] Invalid parameters are validated

Correct Pattern:

func rtcEngine(_ engine: AgoraRtcEngineKit, didOccurError errorCode: AgoraErrorCode) {
    DispatchQueue.main.async {
        self.showError("Error: \(errorCode.rawValue)")
    }
}

func rtcEngine(_ engine: AgoraRtcEngineKit, tokenPrivilegeWillExpire token: String) {
    // Refresh token before expiration
    let newToken = KeyCenter.Token(channelName: self.channelName)
    self.agoraKit.renewToken(newToken)
}

---

5. Code Convention (MEDIUM)

Check:

• [ ] Class name follows pattern: <ExampleName>Main
• [ ] Extends BaseViewController
• [ ] File name matches class name (PascalCase)
• [ ] Properties are properly declared with @IBOutlet or var
• [ ] Methods are organized with // MARK: sections
• [ ] Comments explain non-obvious logic

Correct Pattern:

class ScreenShareMain: BaseViewController {
    
    var agoraKit: AgoraRtcEngineKit!
    var remoteUid: UInt = 0
    
    @IBOutlet weak var Container: AGEVideoContainer!
    
    // MARK: - Lifecycle
    override func viewDidLoad() { ... }
    
    // MARK: - Agora Engine Setup
    func initializeAgoraEngine() { ... }
    
    // MARK: - Actions
    @IBAction func joinButtonTapped(_ sender: Any) { ... }
}

---

6. API Usage Correctness (MEDIUM)

Check:

• [ ] SDK methods called in correct order
• [ ] Required parameters are provided
• [ ] Optional parameters are used correctly
• [ ] Return values are checked where necessary
• [ ] Deprecated APIs are not used

Correct Pattern:

// Correct order: enable -> setup -> join
agoraKit.enableVideo()
agoraKit.setupLocalVideo(AgoraRtcVideoCanvas(uid: 0))
agoraKit.joinChannel(byToken: token, channelName: channel, info: nil, uid: 0)

Incorrect Pattern:

// ❌ Wrong order
agoraKit.joinChannel(...)  // Join first
agoraKit.enableVideo()     // Enable after join (too late)

---

7. Resource Cleanup (MEDIUM)

Check:

• [ ] Audio files are stopped and released
• [ ] Video captures are stopped
• [ ] Custom audio/video sources are cleaned up
• [ ] Observers are unregistered
• [ ] Timers are invalidated

Correct Pattern:

func leaveChannel() {
    agoraKit.stopAudioMixing()  // Stop audio
    agoraKit.stopScreenCapture()  // Stop screen share
    agoraKit.leaveChannel(nil)
}

override func viewWillClose() {
    leaveChannel()
    agoraKit.destroy()
    super.viewWillClose()
}

---

Review Output Format

When reviewing, provide feedback in this format:

## Review Results

### ✅ Passed
- Engine lifecycle correctly managed
- Thread safety ensured with DispatchQueue.main.async
- Permissions requested before device access

### ⚠️ Issues Found

**[HIGH] Thread Safety Issue**
- File: `ScreenShare.swift`
- Line: 45
- Issue: UI update in delegate callback without DispatchQueue.main.async
- Suggestion: Wrap UI update with `DispatchQueue.main.async { ... }`

**[MEDIUM] Missing Error Handling**
- File: `ScreenShare.swift`
- Line: 78
- Issue: joinChannel() result not checked
- Suggestion: Implement `rtcEngine(_:didOccurError:)` delegate method

### 🔧 Recommendations
- Add logging for debugging
- Consider adding retry logic for network failures

---

Platform-Specific Checks

macOS-Specific

Check:

• [ ] Using Cocoa (AppKit) — not UIKit or SwiftUI
• [ ] Window/view lifecycle properly handled
• [ ] No Combine or async/await unless already in codebase
• [ ] Storyboard/XIB files properly configured if used

Correct Pattern:

// macOS: Use Cocoa
import Cocoa
import AgoraRtcKit

class ExampleMain: BaseViewController {
    @IBOutlet weak var Container: AGEVideoContainer!
    // Cocoa-based UI
}

Incorrect Pattern:

// ❌ iOS patterns in macOS
import UIKit  // Wrong framework
class ExampleMain: UIViewController { }  // Wrong base class

---

NEVER List

Do NOT accept:

• Engine not destroyed (memory leak)
• UI updates from background threads without DispatchQueue.main.async
• Multiple engine instances in one example
• Hardcoded App ID or token (must use KeyCenter)
• Missing leaveChannel() before destroy()
• Objective-C files (Swift only)
• UIKit or SwiftUI (Cocoa only)
• Examples outside APIExample/Examples/[Basic|Advanced]/ structure
• Missing delegate implementation for event handling
• No error handling for joinChannel failures

---

Review Checklist

Use this checklist when reviewing an example:

Lifecycle:

• [ ] Engine created once in initialization
• [ ] leaveChannel() called before destroy()
• [ ] destroy() called in cleanup
• [ ] No engine leaks

Thread Safety:

• [ ] All UI updates use DispatchQueue.main.async
• [ ] No direct UI updates from callbacks
• [ ] Frame callbacks dispatch to main thread

Permissions:

• [ ] Microphone permission requested
• [ ] Camera permission requested
• [ ] Permissions checked before use

Error Handling:

• [ ] joinChannel failures handled
• [ ] Token expiration handled
• [ ] Network errors logged

Code Quality:

• [ ] Follows naming conventions
• [ ] Properly organized with MARK sections
• [ ] Comments explain non-obvious logic
• [ ] No hardcoded credentials

API Usage:

• [ ] Methods called in correct order
• [ ] Required parameters provided
• [ ] Return values checked
• [ ] No deprecated APIs

Resources:

• [ ] Audio/video properly stopped
• [ ] Observers unregistered
• [ ] Timers invalidated
• [ ] No resource leaks

Platform:

• [ ] Using Cocoa (AppKit)
• [ ] No UIKit or SwiftUI
• [ ] Window lifecycle handled
• [ ] No modern C++ patterns unless existing

---

Common Issues and Fixes

Issue: "Engine not initialized"

Cause: destroy() called without leaveChannel() first Fix: Always call leaveChannel() before destroy()

Issue: "UI updates crash the app"

Cause: Direct UI update from background thread Fix: Wrap with DispatchQueue.main.async { ... }

Issue: "Memory leak detected"

Cause: destroy() not called or engine recreated Fix: Ensure destroy() in viewWillClose() and create engine once

Issue: "Token expired error"

Cause: No token refresh handling Fix: Implement tokenPrivilegeWillExpire() delegate method

Issue: "No audio/video"

Cause: Permissions not requested Fix: Request permissions before enableAudio() / enableVideo()

---

References

• Agora RTC SDK for macOS: Documentation
• Existing examples: Review APIExample/Examples/Basic/JoinChannelVideo/ for reference
• BaseViewController: Check APIExample/Common/ for base class implementation