KM-13546 Set Small Packets default value to OFF for all protocols

[kp-diego-trevisan]

Summary

KM-13546 Set Small Packets default value to OFF for all protocols

This PR fixes/implements the following bugs/features

• Bug 1
• Bug 2
• Feature 1
• Feature 2
• Breaking changes

Test plan (required)

Code formatting

Closing issues

[claude]

Claude finished @kp-diego-trevisan's task —— View job

---

Code Review for KM-13546 Small Packets Default Value Change

Todo List:

• Read CLAUDE.md for repo-specific guidelines
• Analyze the changed file (AppPreferences.swift)
• Review code quality and best practices
• Check for potential bugs and issues
• Evaluate memory management concerns
• Assess threading considerations
• Review performance implications
• Examine security aspects
• Evaluate test coverage needs
• Provide final review summary

---

Summary

This PR changes the default values for small packets feature from true to false for WireGuard and IKEv2 protocols. The changes affect both the initial default registration and the reset functionality in AppPreferences.swift:629-631 and AppPreferences.swift:887-888.

Code Review Findings

✅ Code Quality & Best Practices

• Well-structured: The changes follow the existing code patterns and style
• Consistent: Both initial defaults and reset method are updated consistently
• Clear intent: The changes clearly implement the requirement to set small packets OFF by default

✅ No Critical Issues Found

• Memory management: No retain cycles or memory leaks introduced - these are simple boolean value assignments
• Threading: No threading concerns - UserDefaults operations are thread-safe and these are simple property assignments
• Performance: Negligible performance impact - only affects default value registration
• Security: No security implications for this configuration change

📋 Technical Analysis

Changes Made:

1. AppPreferences.swift:630-631: Changed WireGuard and IKEv2 default values from true to false
2. AppPreferences.swift:887-888: Updated reset behavior to set both protocols to false

Impact Assessment:

• Backward compatibility: ✅ Existing users with explicit settings won't be affected
• New installations: ✅ Will have small packets disabled by default for all protocols
• Settings consistency: ✅ OpenVPN was already false, now all protocols are consistent

🔍 Areas for Consideration

Minor Observations:

1. Code organization: Lines 886-888 could be better organized for readability:

// Current:
usesCustomDNS = false
useSmallPackets = false
ikeV2UseSmallPackets = false
wireGuardUseSmallPackets = false

// Suggestion: Group related settings
usesCustomDNS = false
useSmallPackets = false
ikeV2UseSmallPackets = false  
wireGuardUseSmallPackets = false

2. Documentation: Consider adding a comment explaining why small packets are disabled by default (performance/compatibility reasons)

🧪 Test Coverage Recommendations

While this change is low-risk, consider adding tests for:

1. Default value verification: Ensure new installations have correct defaults
2. Reset functionality: Verify reset sets all protocols to false
3. Migration testing: Confirm existing user settings aren't affected

Suggested test locations (per CLAUDE.md guidelines):

• Add unit tests to PIA VPNTests (preferred test target)
• Test both initial defaults and reset behavior

✅ Final Recommendation

APPROVE - This is a well-implemented, low-risk configuration change that:

• ✅ Correctly implements the requirement
• ✅ Maintains code consistency
• ✅ Follows existing patterns
• ✅ Has no security or performance concerns
• ✅ Provides good backward compatibility

The changes are straightforward, well-executed, and align with the repository's coding standards outlined in CLAUDE.md.

---

[kp-pietro-messineo]

LGTM