mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-18 13:23:33 +08:00
Stabilize portable mode PR before merge
The contributor PR adds useful Windows terminal and portable mode support, but it also reintroduced an older General settings zoom block and left the new native settings paths without enough regression coverage. This commit keeps the feature direction intact while removing the duplicate UI, making invalid bash paths fail at save time, and covering the portable cache and app-mode paths with focused tests. Constraint: This commit lands directly on the contributor PR branch to avoid a long review-comment loop. Rejected: Ask the contributor to rework the PR from scratch | the remaining issues are narrow and maintainable by us. Confidence: high Scope-risk: moderate Directive: Keep future portable-mode changes covered at the native boundary and the desktop store boundary. Tested: cd desktop && bun run test src/pages/TerminalSettings.test.tsx src/__tests__/generalSettings.test.tsx src/stores/settingsStore.test.ts Tested: bun test src/utils/__tests__/cachePaths.test.ts Tested: cd desktop/src-tauri && cargo test Tested: cd desktop && bun run lint Tested: cd desktop/src-tauri && cargo check Tested: bun run check:server Tested: bun run check:desktop Not-tested: Manual Windows packaged-app portable-mode smoke; to be covered before a future release.
This commit is contained in:
parent
700832c5b6
commit
e8c045876e
@ -258,17 +258,6 @@ impl Default for AppModeConfig {
|
||||
}
|
||||
}
|
||||
|
||||
/// Read the persisted app-mode.json from the given config directory.
|
||||
fn read_app_mode_config(config_dir: &Path) -> Option<AppModeConfig> {
|
||||
let path = config_dir.join(APP_MODE_FILE);
|
||||
fs::read_to_string(&path).ok().and_then(|data| {
|
||||
serde_json::from_str(&data).ok().or_else(|| {
|
||||
eprintln!("[desktop] failed to parse app-mode.json: {}", data);
|
||||
None
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
/// Write the persisted app-mode.json to the given config directory.
|
||||
fn write_app_mode_config(config_dir: &Path, config: &AppModeConfig) {
|
||||
let path = config_dir.join(APP_MODE_FILE);
|
||||
@ -329,24 +318,25 @@ impl TerminalConfig {
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn save(&self, app: &AppHandle) {
|
||||
let Some(path) = terminal_config_path(app) else { return };
|
||||
fn save(&self, app: &AppHandle) -> Result<(), String> {
|
||||
let Some(path) = terminal_config_path(app) else {
|
||||
return Err("terminal config path is unavailable".to_string());
|
||||
};
|
||||
if let Some(parent) = path.parent() {
|
||||
if let Err(err) = fs::create_dir_all(parent) {
|
||||
eprintln!("[desktop] failed to create terminal config directory: {err}");
|
||||
return;
|
||||
return Err(format!("create terminal config directory: {err}"));
|
||||
}
|
||||
}
|
||||
let data = match serde_json::to_string_pretty(self) {
|
||||
Ok(data) => data,
|
||||
Err(err) => {
|
||||
eprintln!("[desktop] failed to serialize terminal config: {err}");
|
||||
return;
|
||||
return Err(format!("serialize terminal config: {err}"));
|
||||
}
|
||||
};
|
||||
if let Err(err) = fs::write(&path, data) {
|
||||
eprintln!("[desktop] failed to write terminal config: {err}");
|
||||
return Err(format!("write terminal config: {err}"));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@ -1043,10 +1033,10 @@ fn get_terminal_bash_path(app: AppHandle) -> Option<String> {
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
fn set_terminal_bash_path(app: AppHandle, path: Option<String>) {
|
||||
fn set_terminal_bash_path(app: AppHandle, path: Option<String>) -> Result<(), String> {
|
||||
let mut config = TerminalConfig::load(&app);
|
||||
config.bash_path = path;
|
||||
config.save(&app);
|
||||
config.bash_path = normalize_terminal_bash_path(path)?;
|
||||
config.save(&app)
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
@ -1270,12 +1260,27 @@ fn home_dir() -> Option<PathBuf> {
|
||||
.map(PathBuf::from)
|
||||
}
|
||||
|
||||
fn default_shell(custom_bash: Option<&str>) -> String {
|
||||
fn normalize_terminal_bash_path(path: Option<String>) -> Result<Option<String>, String> {
|
||||
let Some(path) = path else {
|
||||
return Ok(None);
|
||||
};
|
||||
let trimmed = path.trim();
|
||||
if trimmed.is_empty() {
|
||||
return Ok(None);
|
||||
}
|
||||
let bash_path = PathBuf::from(trimmed);
|
||||
if !bash_path.is_file() {
|
||||
return Err(format!("terminal bash path does not exist: {trimmed}"));
|
||||
}
|
||||
Ok(Some(trimmed.to_string()))
|
||||
}
|
||||
|
||||
fn default_shell(_custom_bash: Option<&str>) -> String {
|
||||
// On Windows, use configured bash path if set and valid
|
||||
#[cfg(target_os = "windows")]
|
||||
if let Some(bash_path) = custom_bash {
|
||||
if let Some(bash_path) = _custom_bash {
|
||||
let trimmed = bash_path.trim();
|
||||
if !trimmed.is_empty() && PathBuf::from(trimmed).exists() {
|
||||
if !trimmed.is_empty() && PathBuf::from(trimmed).is_file() {
|
||||
return trimmed.to_string();
|
||||
}
|
||||
}
|
||||
@ -1686,9 +1691,9 @@ fn kill_windows_sidecars() {
|
||||
mod tests {
|
||||
use super::{
|
||||
decode_terminal_output, default_utf8_locale, ensure_utf8_locale,
|
||||
has_meaningful_intersection, is_persistable_window_state, parse_env_block,
|
||||
run_notification_bridge, select_h5_dist_dir, StoredWindowState, SERVER_BIND_HOST,
|
||||
SERVER_CONTROL_HOST,
|
||||
has_meaningful_intersection, is_persistable_window_state, normalize_terminal_bash_path,
|
||||
parse_env_block, run_notification_bridge, select_h5_dist_dir, StoredWindowState,
|
||||
SERVER_BIND_HOST, SERVER_CONTROL_HOST,
|
||||
};
|
||||
use std::{collections::HashMap, fs};
|
||||
|
||||
@ -1788,6 +1793,48 @@ mod tests {
|
||||
assert_eq!(env.get("EMPTY").map(String::as_str), Some(""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_bash_path_normalizer_clears_blank_values() {
|
||||
assert_eq!(
|
||||
normalize_terminal_bash_path(Some(" ".to_string())).expect("blank path clears"),
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_terminal_bash_path(None).expect("missing path clears"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_bash_path_normalizer_rejects_missing_files() {
|
||||
let missing = std::env::temp_dir().join(format!(
|
||||
"cchh-missing-bash-{}",
|
||||
std::process::id()
|
||||
));
|
||||
|
||||
let error = normalize_terminal_bash_path(Some(missing.to_string_lossy().to_string()))
|
||||
.expect_err("missing path should be rejected");
|
||||
|
||||
assert!(error.contains("terminal bash path does not exist"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_bash_path_normalizer_accepts_existing_files() {
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"cchh-bash-path-test-{}",
|
||||
std::process::id()
|
||||
));
|
||||
fs::write(&path, "").expect("write bash path fixture");
|
||||
|
||||
assert_eq!(
|
||||
normalize_terminal_bash_path(Some(format!(" {} ", path.display())))
|
||||
.expect("existing file is accepted"),
|
||||
Some(path.to_string_lossy().to_string())
|
||||
);
|
||||
|
||||
fs::remove_file(path).expect("remove bash path fixture");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_environment_forces_utf8_locale_when_shell_uses_c_locale() {
|
||||
let mut env = HashMap::from([
|
||||
|
||||
@ -1990,33 +1990,6 @@ function GeneralSettings() {
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
{/* UI Zoom */}
|
||||
<div className="mt-8">
|
||||
<h2 className="text-base font-semibold text-[var(--color-text-primary)] mb-1">{t('settings.general.uiZoom')}</h2>
|
||||
<p className="text-sm text-[var(--color-text-tertiary)] mb-3">{t('settings.general.uiZoomDescription')}</p>
|
||||
<div className="flex flex-col gap-2">
|
||||
<div className="flex items-center justify-end">
|
||||
<span className="text-sm text-[var(--color-text-secondary)]">
|
||||
{Math.round(uiZoom * 100)}%
|
||||
</span>
|
||||
</div>
|
||||
<div className="flex items-center gap-3">
|
||||
<span className="text-xs text-[var(--color-text-tertiary)]">{Math.round(UI_ZOOM_MIN * 100)}%</span>
|
||||
<input
|
||||
type="range"
|
||||
min={UI_ZOOM_MIN}
|
||||
max={UI_ZOOM_MAX}
|
||||
step={UI_ZOOM_STEP}
|
||||
value={uiZoom}
|
||||
onChange={(e) => setUiZoom(parseFloat(e.target.value))}
|
||||
onMouseUp={(e) => e.currentTarget.blur()}
|
||||
className="flex-1"
|
||||
/>
|
||||
<span className="text-xs text-[var(--color-text-tertiary)]">{Math.round(UI_ZOOM_MAX * 100)}%</span>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Confirm dialog for mode switch */}
|
||||
<ConfirmDialog
|
||||
open={modeSwitchConfirmOpen}
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
import { act, render, screen, waitFor } from '@testing-library/react'
|
||||
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import '@testing-library/jest-dom'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { useSettingsStore } from '../stores/settingsStore'
|
||||
@ -28,6 +28,8 @@ const terminalMocks = vi.hoisted(() => {
|
||||
kill: vi.fn(),
|
||||
onOutput: vi.fn(),
|
||||
onExit: vi.fn(),
|
||||
getBashPath: vi.fn(),
|
||||
setBashPath: vi.fn(),
|
||||
}
|
||||
})
|
||||
|
||||
@ -48,6 +50,8 @@ vi.mock('../api/terminal', () => ({
|
||||
kill: terminalMocks.kill,
|
||||
onOutput: terminalMocks.onOutput,
|
||||
onExit: terminalMocks.onExit,
|
||||
getBashPath: terminalMocks.getBashPath,
|
||||
setBashPath: terminalMocks.setBashPath,
|
||||
},
|
||||
}))
|
||||
|
||||
@ -55,6 +59,7 @@ import { TerminalSettings } from './TerminalSettings'
|
||||
|
||||
describe('TerminalSettings', () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
useSettingsStore.setState({ locale: 'en' })
|
||||
terminalMocks.available = false
|
||||
terminalMocks.spawn.mockReset()
|
||||
@ -63,6 +68,8 @@ describe('TerminalSettings', () => {
|
||||
terminalMocks.kill.mockReset()
|
||||
terminalMocks.onOutput.mockReset()
|
||||
terminalMocks.onExit.mockReset()
|
||||
terminalMocks.getBashPath.mockReset()
|
||||
terminalMocks.setBashPath.mockReset()
|
||||
terminalMocks.terminalInstance.loadAddon.mockClear()
|
||||
terminalMocks.terminalInstance.open.mockClear()
|
||||
terminalMocks.terminalInstance.dispose.mockClear()
|
||||
@ -73,6 +80,8 @@ describe('TerminalSettings', () => {
|
||||
terminalMocks.fitInstance.fit.mockClear()
|
||||
terminalMocks.onOutput.mockResolvedValue(vi.fn())
|
||||
terminalMocks.onExit.mockResolvedValue(vi.fn())
|
||||
terminalMocks.getBashPath.mockResolvedValue(null)
|
||||
terminalMocks.setBashPath.mockResolvedValue(undefined)
|
||||
terminalMocks.write.mockResolvedValue(undefined)
|
||||
terminalMocks.resize.mockResolvedValue(undefined)
|
||||
terminalMocks.kill.mockResolvedValue(undefined)
|
||||
@ -85,6 +94,7 @@ describe('TerminalSettings', () => {
|
||||
observe = vi.fn()
|
||||
disconnect = vi.fn()
|
||||
})
|
||||
vi.spyOn(navigator, 'platform', 'get').mockReturnValue('MacIntel')
|
||||
})
|
||||
|
||||
it('shows a desktop-runtime empty state outside Tauri', () => {
|
||||
@ -142,4 +152,35 @@ describe('TerminalSettings', () => {
|
||||
expect(terminalMocks.terminalInstance.write).toHaveBeenCalledWith('hello\r\n')
|
||||
expect(terminalMocks.terminalInstance.write).not.toHaveBeenCalledWith('ignored\r\n')
|
||||
})
|
||||
|
||||
it('saves a custom Windows bash path from the terminal settings panel', async () => {
|
||||
vi.spyOn(navigator, 'platform', 'get').mockReturnValue('Win32')
|
||||
terminalMocks.available = true
|
||||
terminalMocks.getBashPath.mockResolvedValue('C:\\Program Files\\Git\\bin\\bash.exe')
|
||||
|
||||
render(<TerminalSettings />)
|
||||
|
||||
const input = await screen.findByDisplayValue('C:\\Program Files\\Git\\bin\\bash.exe')
|
||||
fireEvent.change(input, { target: { value: ' C:\\Tools\\Git\\bin\\bash.exe ' } })
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Save' }))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(terminalMocks.setBashPath).toHaveBeenCalledWith('C:\\Tools\\Git\\bin\\bash.exe')
|
||||
})
|
||||
expect(await screen.findByRole('button', { name: 'Saved' })).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('shows an invalid path message when native bash path validation fails', async () => {
|
||||
vi.spyOn(navigator, 'platform', 'get').mockReturnValue('Win32')
|
||||
terminalMocks.available = true
|
||||
terminalMocks.setBashPath.mockRejectedValue(new Error('terminal bash path does not exist'))
|
||||
|
||||
render(<TerminalSettings />)
|
||||
|
||||
const input = await screen.findByPlaceholderText('Bash Path')
|
||||
fireEvent.change(input, { target: { value: 'C:\\missing\\bash.exe' } })
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Save' }))
|
||||
|
||||
expect(await screen.findByText('Path does not exist. Select a valid Bash executable.')).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
@ -63,6 +63,66 @@ describe('settingsStore UI zoom', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('settingsStore app mode', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules()
|
||||
vi.clearAllMocks()
|
||||
delete (window as unknown as { __TAURI_INTERNALS__?: object }).__TAURI_INTERNALS__
|
||||
})
|
||||
|
||||
it('hydrates app mode from the native desktop command', async () => {
|
||||
const invoke = vi.fn().mockResolvedValue({
|
||||
mode: 'portable',
|
||||
portableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR',
|
||||
defaultPortableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR',
|
||||
})
|
||||
vi.doMock('@tauri-apps/api/core', () => ({ invoke }))
|
||||
const tauriWindow = window as unknown as { __TAURI_INTERNALS__?: object }
|
||||
tauriWindow.__TAURI_INTERNALS__ = {}
|
||||
|
||||
const { useSettingsStore } = await import('./settingsStore')
|
||||
|
||||
await useSettingsStore.getState().fetchAppMode()
|
||||
|
||||
expect(invoke).toHaveBeenCalledWith('get_app_mode')
|
||||
expect(useSettingsStore.getState().appMode).toEqual({
|
||||
mode: 'portable',
|
||||
portableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR',
|
||||
defaultPortableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR',
|
||||
})
|
||||
})
|
||||
|
||||
it('persists app mode through the native desktop command and marks restart required', async () => {
|
||||
const invoke = vi.fn().mockResolvedValue(undefined)
|
||||
vi.doMock('@tauri-apps/api/core', () => ({ invoke }))
|
||||
const tauriWindow = window as unknown as { __TAURI_INTERNALS__?: object }
|
||||
tauriWindow.__TAURI_INTERNALS__ = {}
|
||||
|
||||
const { useSettingsStore } = await import('./settingsStore')
|
||||
useSettingsStore.setState({
|
||||
appMode: {
|
||||
mode: 'default',
|
||||
portableDir: null,
|
||||
defaultPortableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR',
|
||||
},
|
||||
appModeRequiresRestart: false,
|
||||
})
|
||||
|
||||
await useSettingsStore.getState().setAppMode('portable')
|
||||
|
||||
expect(invoke).toHaveBeenCalledWith('set_app_mode', {
|
||||
mode: 'portable',
|
||||
portableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR',
|
||||
})
|
||||
expect(useSettingsStore.getState().appMode).toEqual({
|
||||
mode: 'portable',
|
||||
portableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR',
|
||||
defaultPortableDir: 'C:\\cc-haha\\CLAUDE_CONFIG_DIR',
|
||||
})
|
||||
expect(useSettingsStore.getState().appModeRequiresRestart).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('settingsStore desktop notification persistence', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules()
|
||||
|
||||
26
src/utils/__tests__/cachePaths.test.ts
Normal file
26
src/utils/__tests__/cachePaths.test.ts
Normal file
@ -0,0 +1,26 @@
|
||||
import { afterEach, describe, expect, test } from 'bun:test'
|
||||
import { join } from 'path'
|
||||
import { tmpdir } from 'os'
|
||||
import { CACHE_PATHS } from '../cachePaths.js'
|
||||
|
||||
const originalClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR
|
||||
|
||||
afterEach(() => {
|
||||
if (originalClaudeConfigDir === undefined) {
|
||||
delete process.env.CLAUDE_CONFIG_DIR
|
||||
} else {
|
||||
process.env.CLAUDE_CONFIG_DIR = originalClaudeConfigDir
|
||||
}
|
||||
})
|
||||
|
||||
describe('CACHE_PATHS portable mode', () => {
|
||||
test('places logs under CLAUDE_CONFIG_DIR when portable mode is active', () => {
|
||||
const configDir = join(tmpdir(), 'cc-haha-portable-cache')
|
||||
process.env.CLAUDE_CONFIG_DIR = configDir
|
||||
|
||||
expect(CACHE_PATHS.baseLogs().startsWith(join(configDir, 'Cache'))).toBe(true)
|
||||
expect(CACHE_PATHS.errors().startsWith(join(configDir, 'Cache'))).toBe(true)
|
||||
expect(CACHE_PATHS.messages().startsWith(join(configDir, 'Cache'))).toBe(true)
|
||||
expect(CACHE_PATHS.mcpLogs('test:server').startsWith(join(configDir, 'Cache'))).toBe(true)
|
||||
})
|
||||
})
|
||||
@ -6,10 +6,10 @@ import { djb2Hash } from './hash.js'
|
||||
// When CLAUDE_CONFIG_DIR is set (portable mode), place cache under it
|
||||
// so the install is fully self-contained. Otherwise fall back to the
|
||||
// system default (%LOCALAPPDATA%\claude-cli-nodejs\Cache on Windows).
|
||||
const claudConfigDir = (process.env as Record<string, string | undefined>).CLAUDE_CONFIG_DIR
|
||||
const paths: { cache: string } = claudConfigDir
|
||||
? { cache: join(claudConfigDir, 'Cache') }
|
||||
: envPaths('claude-cli')
|
||||
function getCacheRoot() {
|
||||
const claudeConfigDir = (process.env as Record<string, string | undefined>).CLAUDE_CONFIG_DIR
|
||||
return claudeConfigDir ? join(claudeConfigDir, 'Cache') : envPaths('claude-cli').cache
|
||||
}
|
||||
|
||||
// Local sanitizePath using djb2Hash — NOT the shared version from
|
||||
// sessionStoragePortable.ts which uses Bun.hash (wyhash) when available.
|
||||
@ -29,14 +29,14 @@ function getProjectDir(cwd: string): string {
|
||||
}
|
||||
|
||||
export const CACHE_PATHS = {
|
||||
baseLogs: () => join(paths.cache, getProjectDir(getFsImplementation().cwd())),
|
||||
baseLogs: () => join(getCacheRoot(), getProjectDir(getFsImplementation().cwd())),
|
||||
errors: () =>
|
||||
join(paths.cache, getProjectDir(getFsImplementation().cwd()), 'errors'),
|
||||
join(getCacheRoot(), getProjectDir(getFsImplementation().cwd()), 'errors'),
|
||||
messages: () =>
|
||||
join(paths.cache, getProjectDir(getFsImplementation().cwd()), 'messages'),
|
||||
join(getCacheRoot(), getProjectDir(getFsImplementation().cwd()), 'messages'),
|
||||
mcpLogs: (serverName: string) =>
|
||||
join(
|
||||
paths.cache,
|
||||
getCacheRoot(),
|
||||
getProjectDir(getFsImplementation().cwd()),
|
||||
// Sanitize server name for Windows compatibility (colons are reserved for drive letters)
|
||||
`mcp-logs-${sanitizePath(serverName)}`,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user