fix: critical permission modal and config issues #127

Merged
naomi merged 19 commits from feat/many into main 2026-02-07 01:55:50 -08:00
2 changed files with 329 additions and 3 deletions
Showing only changes of commit f6906640e4 - Show all commits
+3 -3
View File
@@ -36,11 +36,11 @@ echo -e "${YELLOW}🔍 Running all checks for Hikari Desktop...${NC}"
run_check "Frontend lint" "pnpm lint" || failed=1
run_check "Frontend format check" "pnpm format:check" || failed=1
run_check "Frontend type check" "pnpm check" || failed=1
run_check "Frontend tests" "pnpm test" || failed=1
run_check "Frontend tests with coverage" "pnpm test:coverage" || failed=1
# Backend checks
run_check "Backend clippy (strict)" "cd src-tauri && cargo clippy --all-targets --all-features -- -D warnings" || failed=1
run_check "Backend tests" "cargo test" || failed=1
run_check "Backend clippy (strict)" "(cd src-tauri && cargo clippy --all-targets --all-features -- -D warnings)" || failed=1
run_check "Backend tests with coverage" "(cd src-tauri && cargo llvm-cov --fail-under-lines 50)" || failed=1
# Summary
echo -e "\n${YELLOW}========================================${NC}"
+326
View File
@@ -14,6 +14,7 @@ import {
type Theme,
type CustomThemeColors,
} from "./config";
import { invoke } from "@tauri-apps/api/core";
// Mock Tauri APIs
vi.mock("@tauri-apps/api/core", () => ({
@@ -487,4 +488,329 @@ describe("config store", () => {
expect(typeof configStore.saveError.subscribe).toBe("function");
});
});
describe("Race Condition Tests", () => {
beforeEach(async () => {
// Setup mock to return a default config for load_config
const mockInvokeImpl = vi.mocked(invoke);
mockInvokeImpl.mockResolvedValue({
model: null,
api_key: null,
custom_instructions: null,
mcp_servers_json: null,
auto_granted_tools: [],
theme: "dark",
greeting_enabled: false,
greeting_custom_prompt: null,
notifications_enabled: false,
notification_volume: 0.7,
always_on_top: false,
update_checks_enabled: false,
character_panel_width: null,
font_size: 14,
streamer_mode: false,
streamer_hide_paths: false,
compact_mode: false,
profile_name: null,
profile_avatar_path: null,
profile_bio: null,
custom_theme_colors: {
bg_primary: null,
bg_secondary: null,
bg_terminal: null,
accent_primary: null,
accent_secondary: null,
text_primary: null,
text_secondary: null,
border_color: null,
},
budget_enabled: false,
session_token_budget: null,
session_cost_budget: null,
budget_action: "warn",
budget_warning_threshold: 0.8,
discord_rpc_enabled: false,
});
// Load initial config
await configStore.loadConfig();
vi.clearAllMocks();
});
it("handles rapid sequential config updates correctly", async () => {
// This test validates the fix for the config race condition that caused data loss
const mockInvokeImpl = vi.mocked(invoke);
const invokeCalls: Array<{ command: string; config: HikariConfig }> = [];
mockInvokeImpl.mockImplementation(async (command: string, args?: unknown) => {
if (command === "save_config" && args && typeof args === "object" && "config" in args) {
invokeCalls.push({ command, config: args.config as HikariConfig });
// Simulate small delay in saving
await new Promise((resolve) => setTimeout(resolve, 10));
}
return null;
});
// Perform rapid updates
await Promise.all([
configStore.updateConfig({ font_size: 16 }),
configStore.updateConfig({ theme: "light" }),
configStore.updateConfig({ compact_mode: true }),
]);
// All three updates should have been saved
expect(invokeCalls.length).toBe(3);
// Get final config
const finalConfig = configStore.getConfig();
// Final config should have all updates
// Note: The last update wins for each field, but all fields should be preserved
expect(finalConfig.compact_mode).toBe(true);
});
it("preserves previous field values during concurrent updates", async () => {
// Set initial values
await configStore.updateConfig({
font_size: 16,
theme: "dark",
compact_mode: false,
streamer_mode: false,
});
vi.clearAllMocks();
const mockInvokeImpl = vi.mocked(invoke);
const invokeCalls: Array<{ command: string; config: HikariConfig }> = [];
mockInvokeImpl.mockImplementation(async (command: string, args?: unknown) => {
if (command === "save_config" && args && typeof args === "object" && "config" in args) {
invokeCalls.push({ command, config: args.config as HikariConfig });
await new Promise((resolve) => setTimeout(resolve, 5));
}
return null;
});
// Update different fields concurrently
await Promise.all([
configStore.updateConfig({ font_size: 18 }),
configStore.updateConfig({ theme: "light" }),
configStore.updateConfig({ compact_mode: true }),
]);
// Check that each save included all previous config values
invokeCalls.forEach((call) => {
// Each save should have a complete config, not just the updated field
expect(call.config).toHaveProperty("font_size");
expect(call.config).toHaveProperty("theme");
expect(call.config).toHaveProperty("compact_mode");
expect(call.config).toHaveProperty("streamer_mode");
expect(call.config).toHaveProperty("model");
expect(call.config).toHaveProperty("api_key");
});
});
it("handles update during save operation", async () => {
const mockInvokeImpl = vi.mocked(invoke);
let firstSaveStarted = false;
let firstSaveCompleted = false;
mockInvokeImpl.mockImplementation(async (command: string) => {
if (command === "save_config") {
if (!firstSaveStarted) {
firstSaveStarted = true;
// Simulate slow save
await new Promise((resolve) => setTimeout(resolve, 50));
firstSaveCompleted = true;
} else {
// Second save starts while first is in progress
expect(firstSaveStarted).toBe(true);
// First save might not be complete yet (race condition scenario)
}
}
return null;
});
// Start first update
const firstUpdate = configStore.updateConfig({ font_size: 16 });
// Wait a bit then start second update whilst first is still saving
await new Promise((resolve) => setTimeout(resolve, 10));
const secondUpdate = configStore.updateConfig({ theme: "light" });
// Wait for both to complete
await Promise.all([firstUpdate, secondUpdate]);
// Both should complete successfully without errors
expect(firstSaveCompleted).toBe(true);
});
it("getConfig returns most recently set configuration", async () => {
await configStore.updateConfig({ font_size: 14 });
expect(configStore.getConfig().font_size).toBe(14);
await configStore.updateConfig({ font_size: 16 });
expect(configStore.getConfig().font_size).toBe(16);
await configStore.updateConfig({ font_size: 18 });
expect(configStore.getConfig().font_size).toBe(18);
});
it("updates do not lose data from previous operations", async () => {
// Set multiple fields
await configStore.updateConfig({
font_size: 16,
theme: "dark",
compact_mode: true,
streamer_mode: true,
model: "claude-sonnet-4",
});
// Update just one field
await configStore.updateConfig({ theme: "light" });
// Other fields should be preserved
const config = configStore.getConfig();
expect(config.theme).toBe("light");
expect(config.font_size).toBe(16);
expect(config.compact_mode).toBe(true);
expect(config.streamer_mode).toBe(true);
expect(config.model).toBe("claude-sonnet-4");
});
it("auto granted tools are not lost during other updates", async () => {
// Add some tools
await configStore.addAutoGrantedTool("Read");
await configStore.addAutoGrantedTool("Write");
expect(configStore.getConfig().auto_granted_tools).toContain("Read");
expect(configStore.getConfig().auto_granted_tools).toContain("Write");
// Update another field
await configStore.updateConfig({ theme: "light" });
// Tools should still be there
expect(configStore.getConfig().auto_granted_tools).toContain("Read");
expect(configStore.getConfig().auto_granted_tools).toContain("Write");
});
it("custom theme colors persist across other config updates", async () => {
const customColors: CustomThemeColors = {
bg_primary: "#1a1a2e",
bg_secondary: "#16213e",
bg_terminal: "#0f0f23",
accent_primary: "#e94560",
accent_secondary: "#533483",
text_primary: "#eaeaea",
text_secondary: "#a0a0a0",
border_color: "#333355",
};
await configStore.setCustomThemeColors(customColors);
// Update another field
await configStore.updateConfig({ font_size: 18 });
// Colors should still be there
const config = configStore.getConfig();
expect(config.custom_theme_colors.bg_primary).toBe("#1a1a2e");
expect(config.custom_theme_colors.accent_primary).toBe("#e94560");
});
it("handles save errors gracefully without losing data", async () => {
const mockInvokeImpl = vi.mocked(invoke);
// Set initial config
await configStore.updateConfig({ font_size: 14 });
// Make next save fail
mockInvokeImpl.mockRejectedValueOnce(new Error("Save failed"));
// Try to update - should throw
await expect(configStore.updateConfig({ theme: "light" })).rejects.toThrow();
// Original config should still be accessible
expect(configStore.getConfig().font_size).toBe(14);
});
});
describe("Config Persistence Tests", () => {
it("loadConfig retrieves saved configuration", async () => {
const mockConfig: HikariConfig = {
model: "claude-sonnet-4",
api_key: "test-key",
custom_instructions: "Be helpful",
mcp_servers_json: "{}",
auto_granted_tools: ["Read", "Write"],
theme: "light",
greeting_enabled: false,
greeting_custom_prompt: null,
notifications_enabled: false,
notification_volume: 0.5,
always_on_top: true,
update_checks_enabled: false,
character_panel_width: 400,
font_size: 18,
streamer_mode: true,
streamer_hide_paths: true,
compact_mode: true,
profile_name: "Test User",
profile_avatar_path: "/test/avatar.png",
profile_bio: "Test bio",
custom_theme_colors: {
bg_primary: null,
bg_secondary: null,
bg_terminal: null,
accent_primary: null,
accent_secondary: null,
text_primary: null,
text_secondary: null,
border_color: null,
},
budget_enabled: true,
session_token_budget: 100000,
session_cost_budget: 1.5,
budget_action: "block",
budget_warning_threshold: 0.9,
discord_rpc_enabled: false,
};
const mockInvokeImpl = vi.mocked(invoke);
mockInvokeImpl.mockResolvedValueOnce(mockConfig);
await configStore.loadConfig();
const loadedConfig = configStore.getConfig();
expect(loadedConfig.model).toBe("claude-sonnet-4");
expect(loadedConfig.theme).toBe("light");
expect(loadedConfig.font_size).toBe(18);
expect(loadedConfig.auto_granted_tools).toEqual(["Read", "Write"]);
});
it("saveConfig persists configuration to backend", async () => {
const mockInvokeImpl = vi.mocked(invoke);
const savedConfigs: HikariConfig[] = [];
mockInvokeImpl.mockImplementation(async (command: string, args?: unknown) => {
if (command === "save_config" && args && typeof args === "object" && "config" in args) {
savedConfigs.push(args.config as HikariConfig);
}
return null;
});
const configToSave: Partial<HikariConfig> = {
model: "claude-sonnet-4",
theme: "dark",
font_size: 16,
};
await configStore.updateConfig(configToSave);
expect(savedConfigs.length).toBeGreaterThan(0);
const lastSaved = savedConfigs[savedConfigs.length - 1];
expect(lastSaved.model).toBe("claude-sonnet-4");
expect(lastSaved.theme).toBe("dark");
expect(lastSaved.font_size).toBe(16);
});
});
});