refactor: keep merged PRs above code health gate

This commit is contained in:
lucaronin
2026-05-26 10:26:29 +02:00
parent e3bfeba163
commit ae7ee4064f
6 changed files with 183 additions and 150 deletions

View File

@@ -4,6 +4,13 @@ use std::path::Path;
use super::conflict::get_conflict_files;
const GIT_CONFIG_SUBCOMMAND: &str = "config";
const GIT_CONFIG_GET_REGEXP: &str = "--get-regexp";
const REMOTE_URL_CONFIG_PATTERN: &str = r"^remote\..*\.url$";
const NO_REMOTE_STATUS: &str = "no_remote";
const NO_REMOTE_MESSAGE: &str = "No remote configured";
const GIT_INSPECT_REMOTES_ERROR: &str = "Failed to inspect git remotes";
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
pub struct GitPullResult {
pub status: String, // "up_to_date" | "updated" | "conflict" | "no_remote" | "error"
@@ -17,8 +24,16 @@ pub struct GitPullResult {
/// Check whether the vault repo has at least one remote configured.
pub fn has_remote(vault_path: &str) -> Result<bool, String> {
let vault = Path::new(vault_path);
remote_url_config_exists(vault)
}
fn remote_url_config_exists(vault: &Path) -> Result<bool, String> {
let output = git_command()
.args(["config", "--get-regexp", r"^remote\..*\.url$"])
.args([
GIT_CONFIG_SUBCOMMAND,
GIT_CONFIG_GET_REGEXP,
REMOTE_URL_CONFIG_PATTERN,
])
.current_dir(vault)
.output()
.map_err(|e| format!("Failed to inspect git remotes: {}", e))?;
@@ -29,7 +44,7 @@ pub fn has_remote(vault_path: &str) -> Result<bool, String> {
}
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
return Err(if stderr.is_empty() {
"Failed to inspect git remotes".to_string()
GIT_INSPECT_REMOTES_ERROR.to_string()
} else {
stderr
});
@@ -45,8 +60,8 @@ pub fn git_pull(vault_path: &str) -> Result<GitPullResult, String> {
if !has_remote(vault_path)? {
return Ok(GitPullResult {
status: "no_remote".to_string(),
message: "No remote configured".to_string(),
status: NO_REMOTE_STATUS.to_string(),
message: NO_REMOTE_MESSAGE.to_string(),
updated_files: vec![],
conflict_files: vec![],
});
@@ -313,8 +328,8 @@ pub fn git_push(vault_path: &str) -> Result<GitPushResult, String> {
if !has_remote(vault_path)? {
return Ok(GitPushResult {
status: "no_remote".to_string(),
message: "No remote configured".to_string(),
status: NO_REMOTE_STATUS.to_string(),
message: NO_REMOTE_MESSAGE.to_string(),
});
}
@@ -342,6 +357,11 @@ mod tests {
use crate::git::tests::{setup_git_repo, setup_remote_pair};
use std::fs;
fn commit_default_note(vault_path: &Path) {
fs::write(vault_path.join("note.md"), "# Note\n").unwrap();
git_commit(vault_path.to_str().unwrap(), "initial").unwrap();
}
#[test]
fn test_has_remote_returns_false_for_local_repo() {
let dir = setup_git_repo();
@@ -527,8 +547,7 @@ hint: have locally."#;
let (_bare, clone_a, _clone_b) = setup_remote_pair();
let vp_a = clone_a.path().to_str().unwrap();
fs::write(clone_a.path().join("note.md"), "# Note\n").unwrap();
git_commit(vp_a, "initial").unwrap();
commit_default_note(clone_a.path());
let result = git_push(vp_a).unwrap();
assert_eq!(result.status, "ok");
}
@@ -539,8 +558,7 @@ hint: have locally."#;
let vault = dir.path();
let vp = vault.to_str().unwrap();
fs::write(vault.join("note.md"), "# Note\n").unwrap();
git_commit(vp, "initial").unwrap();
commit_default_note(vault);
let result = git_push(vp).unwrap();
assert_eq!(result.status, "no_remote");

View File

@@ -1,4 +1,4 @@
import { useState } from 'react'
import { useState, type ComponentProps } from 'react'
import { act, fireEvent, render, screen, within } from '@testing-library/react'
import { describe, it, expect, vi } from 'vitest'
import { FolderTree } from './FolderTree'
@@ -22,6 +22,30 @@ const mockFolders: FolderNode[] = [
const defaultSelection: SidebarSelection = { kind: 'filter', filter: 'all' }
const vaultRootPath = '/Users/luca/Laputa'
function renderTree(props: Partial<ComponentProps<typeof FolderTree>> = {}) {
const onSelect = props.onSelect ?? vi.fn()
render(
<FolderTree
folders={mockFolders}
selection={defaultSelection}
onSelect={onSelect}
{...props}
/>,
)
return { onSelect }
}
function clickFolderRow(path: string) {
fireEvent.click(screen.getByTestId(`folder-row:${path}`))
}
async function submitNewFolder(name: string) {
fireEvent.click(screen.getByTestId('create-folder-btn'))
const input = screen.getByTestId('new-folder-input')
fireEvent.change(input, { target: { value: name } })
fireEvent.keyDown(input, { key: 'Enter' })
}
describe('FolderTree', () => {
it('renders nothing when folders is empty', () => {
const { container } = render(
@@ -153,32 +177,16 @@ describe('FolderTree', () => {
})
it('selects the vault root with the root path attached', () => {
const onSelect = vi.fn()
render(
<FolderTree
folders={mockFolders}
selection={defaultSelection}
onSelect={onSelect}
vaultRootPath={vaultRootPath}
/>,
)
const { onSelect } = renderTree({ vaultRootPath })
fireEvent.click(screen.getByTestId('folder-row:'))
clickFolderRow('')
expect(onSelect).toHaveBeenCalledWith({ kind: 'folder', path: '', rootPath: vaultRootPath })
})
it('selects child folders with the vault root path attached when the tree has a root', () => {
const onSelect = vi.fn()
render(
<FolderTree
folders={mockFolders}
selection={defaultSelection}
onSelect={onSelect}
vaultRootPath={vaultRootPath}
/>,
)
const { onSelect } = renderTree({ vaultRootPath })
fireEvent.click(screen.getByTestId('folder-row:areas'))
clickFolderRow('areas')
expect(onSelect).toHaveBeenCalledWith({ kind: 'folder', path: 'areas', rootPath: vaultRootPath })
})
@@ -237,20 +245,13 @@ describe('FolderTree', () => {
it('passes the selected folder as the parent when creating a new folder', async () => {
const onCreateFolder = vi.fn().mockResolvedValue(true)
render(
<FolderTree
folders={mockFolders}
selection={{ kind: 'folder', path: 'projects', rootPath: vaultRootPath }}
onSelect={vi.fn()}
onCreateFolder={onCreateFolder}
vaultRootPath={vaultRootPath}
/>,
)
renderTree({
selection: { kind: 'folder', path: 'projects', rootPath: vaultRootPath },
onCreateFolder,
vaultRootPath,
})
fireEvent.click(screen.getByTestId('create-folder-btn'))
const input = screen.getByTestId('new-folder-input')
fireEvent.change(input, { target: { value: 'laputa' } })
fireEvent.keyDown(input, { key: 'Enter' })
await submitNewFolder('laputa')
await vi.waitFor(() => {
expect(onCreateFolder).toHaveBeenCalledWith('laputa', {
@@ -278,20 +279,14 @@ describe('FolderTree', () => {
},
]
render(
<FolderTree
folders={folders}
selection={{ kind: 'folder', path: '', rootPath: otherVault }}
onSelect={vi.fn()}
onCreateFolder={onCreateFolder}
vaultRootPath={vaultRootPath}
/>,
)
renderTree({
folders,
selection: { kind: 'folder', path: '', rootPath: otherVault },
onCreateFolder,
vaultRootPath,
})
fireEvent.click(screen.getByTestId('create-folder-btn'))
const input = screen.getByTestId('new-folder-input')
fireEvent.change(input, { target: { value: 'inbox' } })
fireEvent.keyDown(input, { key: 'Enter' })
await submitNewFolder('inbox')
await vi.waitFor(() => {
expect(onCreateFolder).toHaveBeenCalledWith('inbox', {
@@ -303,20 +298,9 @@ describe('FolderTree', () => {
it('omits parent context when nothing folder-like is selected', async () => {
const onCreateFolder = vi.fn().mockResolvedValue(true)
render(
<FolderTree
folders={mockFolders}
selection={{ kind: 'filter', filter: 'all' }}
onSelect={vi.fn()}
onCreateFolder={onCreateFolder}
vaultRootPath={vaultRootPath}
/>,
)
renderTree({ onCreateFolder, vaultRootPath })
fireEvent.click(screen.getByTestId('create-folder-btn'))
const input = screen.getByTestId('new-folder-input')
fireEvent.change(input, { target: { value: 'inbox' } })
fireEvent.keyDown(input, { key: 'Enter' })
await submitNewFolder('inbox')
await vi.waitFor(() => {
expect(onCreateFolder).toHaveBeenCalledWith('inbox', undefined)

View File

@@ -94,6 +94,39 @@ function useDisplayedFolders(folders: FolderNode[], expanded: Record<string, boo
}, [expanded, folders, locale, vaultRootPath])
}
function creationParentForSelection(selection: SidebarSelection): FolderCreationParent | undefined {
if (selection.kind !== 'folder') return undefined
return { path: selection.path, rootPath: selection.rootPath }
}
function useCreateFolderSubmit({
closeCreateForm,
expandFolder,
onCreateFolder,
selection,
}: {
closeCreateForm: () => void
expandFolder: (key: string) => void
onCreateFolder?: (name: string, parent?: FolderCreationParent) => Promise<boolean> | boolean
selection: SidebarSelection
}) {
return useCallback(async (value: string) => {
const nextName = value.trim()
if (!nextName || !onCreateFolder) {
closeCreateForm()
return true
}
const parent = creationParentForSelection(selection)
const created = await onCreateFolder(nextName, parent)
if (!created) return created
closeCreateForm()
if (parent?.path) expandFolder(folderNodeKey(parent))
return created
}, [closeCreateForm, expandFolder, onCreateFolder, selection])
}
export const FolderTree = memo(function FolderTree({
folders,
selection,
@@ -140,29 +173,12 @@ export const FolderTree = memo(function FolderTree({
onStartRenameFolder,
})
const handleCreateFolderSubmit = useCallback(async (value: string) => {
const nextName = value.trim()
if (!nextName || !onCreateFolder) {
closeCreateForm()
return true
}
const parent: FolderCreationParent | undefined = selection.kind === 'folder'
? { path: selection.path, rootPath: selection.rootPath }
: undefined
const created = await onCreateFolder(nextName, parent)
if (created) {
closeCreateForm()
// Ensure the parent folder is open so the freshly created child is visible.
// The selection→ancestor-expansion machinery deliberately doesn't expand the
// selected node itself, so we expand it explicitly here. Vault-root keys are
// open by default and don't need this.
if (parent?.path) {
expandFolder(folderNodeKey(parent))
}
}
return created
}, [closeCreateForm, expandFolder, onCreateFolder, selection])
const handleCreateFolderSubmit = useCreateFolderSubmit({
closeCreateForm,
expandFolder,
onCreateFolder,
selection,
})
const handleCreateFolderClick = useCallback(() => {
closeContextMenu()

View File

@@ -30,53 +30,48 @@ function callApply(overrides: ChangeOverrides) {
}
describe('applyMountedChange', () => {
it('reroutes both the default workspace and the active vault when the unmounted vault is both', () => {
const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({
defaultPath: '/a',
vaultPath: '/a',
path: '/a',
})
it.each([
{
name: 'both default and active vault',
request: { defaultPath: '/a', vaultPath: '/a', path: '/a' },
expectedDefault: '/b',
expectedSwitch: '/b',
expectedIdentity: ['/a', { mounted: false }],
},
{
name: 'active vault only',
request: { defaultPath: '/b', vaultPath: '/a', path: '/a' },
expectedDefault: null,
expectedSwitch: '/b',
expectedIdentity: ['/a', { mounted: false }],
},
{
name: 'default workspace only',
request: { defaultPath: '/a', vaultPath: '/b', path: '/a' },
expectedDefault: '/b',
expectedSwitch: null,
expectedIdentity: ['/a', { mounted: false }],
},
{
name: 'neither default nor active vault',
request: {
defaultPath: '/a',
vaultPath: '/a',
path: '/b',
includedVaults: [vault('/a'), vault('/b'), vault('/c')],
},
expectedDefault: null,
expectedSwitch: null,
expectedIdentity: ['/b', { mounted: false }],
},
])('handles unmounting $name', ({ request, expectedDefault, expectedSwitch, expectedIdentity }) => {
const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply(request)
expect(onSetDefaultWorkspace).toHaveBeenCalledWith('/b')
expect(onSwitchVault).toHaveBeenCalledWith('/b')
expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/a', { mounted: false })
})
it('reroutes the active vault when only the active vault is unmounted (default workspace differs)', () => {
const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({
defaultPath: '/b',
vaultPath: '/a',
path: '/a',
})
expect(onSetDefaultWorkspace).not.toHaveBeenCalled()
expect(onSwitchVault).toHaveBeenCalledWith('/b')
expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/a', { mounted: false })
})
it('reroutes the default workspace when only the default is unmounted (active vault differs)', () => {
const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({
defaultPath: '/a',
vaultPath: '/b',
path: '/a',
})
expect(onSetDefaultWorkspace).toHaveBeenCalledWith('/b')
expect(onSwitchVault).not.toHaveBeenCalled()
expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/a', { mounted: false })
})
it('does not reroute when the unmounted vault is neither default nor active', () => {
const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({
defaultPath: '/a',
vaultPath: '/a',
path: '/b',
includedVaults: [vault('/a'), vault('/b'), vault('/c')],
})
expect(onSetDefaultWorkspace).not.toHaveBeenCalled()
expect(onSwitchVault).not.toHaveBeenCalled()
expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/b', { mounted: false })
if (expectedDefault) expect(onSetDefaultWorkspace).toHaveBeenCalledWith(expectedDefault)
else expect(onSetDefaultWorkspace).not.toHaveBeenCalled()
if (expectedSwitch) expect(onSwitchVault).toHaveBeenCalledWith(expectedSwitch)
else expect(onSwitchVault).not.toHaveBeenCalled()
expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith(...expectedIdentity)
})
it('bails out without changing anything when no alternative mounted vault exists', () => {

View File

@@ -19,6 +19,20 @@ function nextIncludedVaultPath(includedVaults: VaultOption[], currentPath: strin
return includedVaults.find((vault) => vault.path !== currentPath)?.path ?? null
}
function isCurrentPathUnmount(request: VaultMountChangeRequest): boolean {
if (request.mounted) return false
if (request.path === request.defaultPath) return true
return request.path === request.vaultPath
}
function rerouteUnmountedPath(request: VaultMountChangeRequest): boolean {
const nextPath = nextIncludedVaultPath(request.includedVaults, request.path)
if (!nextPath) return false
if (request.path === request.defaultPath) request.callbacks.onSetDefaultWorkspace?.(nextPath)
if (request.path === request.vaultPath) request.callbacks.onSwitchVault(nextPath)
return true
}
export function applyMountedChange({
defaultPath,
vaultPath,
@@ -27,11 +41,7 @@ export function applyMountedChange({
path,
callbacks,
}: VaultMountChangeRequest): void {
if (!mounted && (path === defaultPath || path === vaultPath)) {
const nextPath = nextIncludedVaultPath(includedVaults, path)
if (!nextPath) return
if (path === defaultPath) callbacks.onSetDefaultWorkspace?.(nextPath)
if (path === vaultPath) callbacks.onSwitchVault(nextPath)
}
const request = { defaultPath, vaultPath, includedVaults, mounted, path, callbacks }
if (isCurrentPathUnmount(request) && !rerouteUnmountedPath(request)) return
callbacks.onUpdateWorkspaceIdentity?.(path, { mounted })
}

View File

@@ -124,24 +124,34 @@ export function agentTargets(): AiTarget[] {
export function resolveAiTarget(settings: Settings): AiTarget {
const providers = normalizeAiModelProviders(settings.ai_model_providers)
const targets = [...agentTargets(), ...configuredModelTargets(providers)]
const agents = agentTargets()
const targets = [...agents, ...configuredModelTargets(providers)]
const storedLegacyAgent = normalizeStoredAiAgent(settings.default_ai_agent)
const legacyAgent = storedLegacyAgent ?? DEFAULT_AI_AGENT
const target = resolveStoredAiTarget(settings.default_ai_target, targets)
if (target) {
if (target.kind === 'agent' && storedLegacyAgent && target.agent === DEFAULT_AI_AGENT && target.agent !== storedLegacyAgent) {
return agentTargets().find((candidate) => candidate.kind === 'agent' && candidate.agent === legacyAgent) ?? target
}
if (shouldPreferLegacyAgent(target, storedLegacyAgent)) return agentTargetFor(agents, legacyAgent) ?? target
return target
}
return agentTargets().find((candidate) => candidate.kind === 'agent' && candidate.agent === legacyAgent) ?? agentTargets()[0]
return agentTargetFor(agents, legacyAgent) ?? agents[0]
}
export function targetAgent(target: AiTarget): AiAgentId {
return target.kind === 'agent' ? target.agent : DEFAULT_AI_AGENT
}
function shouldPreferLegacyAgent(target: AiTarget, storedLegacyAgent: AiAgentId | null): boolean {
if (target.kind !== 'agent') return false
if (!storedLegacyAgent) return false
if (target.agent !== DEFAULT_AI_AGENT) return false
return target.agent !== storedLegacyAgent
}
function agentTargetFor(agents: AiTarget[], agent: AiAgentId): AiTarget | undefined {
return agents.find((candidate) => candidate.kind === 'agent' && candidate.agent === agent)
}
function resolveStoredAiTarget(storedTarget: string | null | undefined, targets: AiTarget[]): AiTarget | null {
const target = storedTarget?.trim()
if (!target) return null