fix: enable dynamic permission mode switching without restart

[YishenTu]

Summary

• Enable bidirectional permission mode switching (safe↔yolo) without process restart
• Always set allowDangerouslySkipPermissions: true at init to enable dynamic switching via setPermissionMode()
• Remove unused allowDangerouslySkip field from PersistentQueryConfig

Test plan

• Typecheck passes
• Lint passes
• Unit tests pass (1999 tests)
• Integration tests pass (253 tests)
• Added new integration test for normal→YOLO transition
• Manual test: toggle permission mode in sidebar while streaming

[claude]

PR Review: Enable Dynamic Permission Mode Switching

Summary

This PR successfully enables bidirectional permission mode switching (safe ↔ YOLO) without requiring process restart by always setting allowDangerouslySkipPermissions: true at initialization.

---

✅ Strengths

1. Clean Architecture

• The solution is elegant and minimal, removing unnecessary conditional logic
• Properly centralizes permission mode handling in applyPermissionMode()
• Good separation of concerns between QueryOptionsBuilder (init) and ClaudianService (dynamic updates)

2. Excellent Documentation

• Clear inline comments explaining the rationale (lines 950-951 in ClaudianService.ts, lines 356-358 in QueryOptionsBuilder.ts)
• Commit message thoroughly explains the "why" behind the change
• Comment in needsRestart() correctly notes permission mode is handled dynamically

3. Robust Testing

• Added integration test for normal→YOLO transition (the previously missing case)
• Test correctly validates setPermissionMode('bypassPermissions') is called
• Unit tests properly updated to remove allowDangerouslySkip field

4. Code Simplification

• Before: 20+ lines of conditional logic with restart handling
• After: 5 lines of clean, symmetric logic
• Removed unused allowDangerouslySkip field from PersistentQueryConfig (good cleanup)

---

🔍 Observations & Considerations

1. Security Model Validation ⚠️

Finding: Always setting allowDangerouslySkipPermissions: true means the SDK process always has the capability to bypass permissions, even when in "safe" mode.

Analysis:

• The actual enforcement is controlled by permissionMode ('default' vs 'bypassPermissions')
• This matches the Claude Code CLI behavior (as noted in comments)
• However, it's worth validating that permissionMode: 'default' properly enforces permissions even when allowDangerouslySkipPermissions: true

Recommendation: Consider adding a comment in the code or PR description explicitly confirming this security model is validated against the Claude Agent SDK behavior.

2. Restart Logic Simplification

The needsRestart() method in QueryOptionsBuilder.ts has this comment (lines 124-125):

// Note: Permission mode is handled dynamically via setPermissionMode() in ClaudianService.
// Since allowDangerouslySkipPermissions is always true, both directions work without restart.

Observation: This is correct, but the old logic that checked allowDangerouslySkip for restart decisions is now completely removed from needsRestart(). This is good, but ensure no other code paths were relying on that field.

Verification: I don't see any other references, which is good. ✅

3. Test Coverage Gap (Minor)

Current coverage:

• ✅ YOLO→normal transition (existing test)
• ✅ normal→YOLO transition (new test)

Missing edge case:

• Multiple rapid toggles between modes (YOLO→normal→YOLO→normal)
• Error handling if setPermissionMode() fails

Impact: Low priority, but could catch SDK-level issues.

---

🐛 Potential Issues

None Found

I did not identify any bugs, race conditions, or breaking changes. The implementation is solid.

---

🎯 Performance Considerations

Positive Impact

• Eliminates unnecessary restarts: Previously, normal→YOLO required full process restart, which is expensive
• Faster mode switching: Now both directions use lightweight setPermissionMode() call
• Better UX: User can toggle permission mode during streaming without interruption

---

🔒 Security Assessment

Risk Level: Low ✅

What changed:

• SDK process now always has bypass capability enabled at init
• Actual permission enforcement depends on runtime permissionMode setting

Why it's safe:

1. Permission mode is still enforced by the SDK based on permissionMode parameter
2. Matches Claude Code CLI security model (as documented)
3. User's permission mode setting still controls behavior
4. canUseTool callback is still invoked in "safe" mode (line 373 in QueryOptionsBuilder.ts)

Recommendation: ✅ Approved from security perspective, assuming the Claude Agent SDK properly respects permissionMode even when allowDangerouslySkipPermissions is true.

---

📊 Code Quality

Metrics

• Lines changed: -28 lines (net reduction is excellent)
• Complexity: Significantly reduced (removed nested conditionals)
• Maintainability: Improved (clearer intent, less branching)
• Type safety: Maintained (removed unused field from interface)

Style Adherence

✅ Follows CLAUDE.md conventions:

• Proper module separation (core/agent)
• Good documentation
• Test coverage for changes

---

🧪 Test Coverage Assessment

What's Tested ✅

1. Integration test for normal→YOLO transition (new)
2. Integration test for YOLO→normal transition (existing)
3. Unit tests updated to remove allowDangerouslySkip field
4. All 1999 unit tests + 253 integration tests pass (per PR description)

Suggestions for Additional Tests (Optional)

1. Error handling: What happens if setPermissionMode() throws?
2. Race conditions: Multiple rapid permission mode changes
3. State consistency: Verify this.currentConfig.permissionMode stays in sync

Priority: Low (current coverage is adequate)

---

📝 Documentation

Inline Comments ✅

• Clear explanation of why allowDangerouslySkipPermissions is always true
• Reference to Claude Code CLI behavior

Commit Message ✅

• Excellent description of problem and solution
• Clear "before/after" behavior

---

✅ Final Recommendation

APPROVE with minor suggestions

Required Actions (None)

All critical checks pass.

Optional Improvements

1. Documentation: Add a brief comment in applyPermissionMode() confirming that permission enforcement still works when allowDangerouslySkipPermissions: true + permissionMode: 'default' (just for clarity)
2. Test coverage: Consider adding error handling test for setPermissionMode() failure (low priority)

Checklist

• ✅ Code quality: Excellent (net -28 lines, reduced complexity)
• ✅ Best practices: Follows CLAUDE.md conventions
• ✅ Bugs: None identified
• ✅ Performance: Improved (eliminates restart)
• ✅ Security: Low risk, proper enforcement model
• ✅ Tests: Good coverage, new integration test added
• ⚠️ Manual testing: Author noted manual test is incomplete (toggle during streaming)

---

🎉 Summary

This is a high-quality PR that meaningfully improves the user experience by eliminating an unnecessary restart requirement. The implementation is clean, well-documented, and properly tested. The security model change is low-risk and aligns with Claude Code CLI behavior.

Great work! 🚀