fix: guard inspector property mutations

This commit is contained in:
lucaronin
2026-05-08 17:37:04 +02:00
parent 852c153d24
commit eb731bbaad
9 changed files with 172 additions and 52 deletions

View File

@@ -11,6 +11,7 @@ import { RUNTIME_STYLE_NONCE } from '../lib/runtimeStyleNonce'
import type { VaultEntry, GitCommit, NoteWidthMode, NoteStatus } from '../types'
import type { NoteListItem } from '../utils/ai-context'
import type { FrontmatterValue } from './Inspector'
import type { FrontmatterOpOptions } from '../hooks/frontmatterOps'
import { ResizeHandle } from './ResizeHandle'
import { useDiffMode, type CommitDiffRequest } from '../hooks/useDiffMode'
import { useEditorFocus } from '../hooks/useEditorFocus'
@@ -66,9 +67,9 @@ interface EditorProps {
inspectorEntry: VaultEntry | null
inspectorContent: string | null
gitHistory: GitCommit[]
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onDeleteProperty?: (path: string, key: string) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onDeleteProperty?: (path: string, key: string, options?: FrontmatterOpOptions) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onCreateMissingType?: (path: string, missingType: string, nextTypeName: string) => Promise<boolean | void>
onCreateAndOpenNote?: (title: string) => Promise<boolean>
onInitializeProperties?: (path: string) => void
@@ -437,9 +438,9 @@ function EditorLayout({
noteList?: NoteListItem[]
noteListFilter?: { type: string | null; query: string }
handleViewCommitDiff: (commitHash: string) => Promise<void>
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onDeleteProperty?: (path: string, key: string) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onDeleteProperty?: (path: string, key: string, options?: FrontmatterOpOptions) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onCreateMissingType?: (path: string, missingType: string, nextTypeName: string) => Promise<boolean | void>
onCreateAndOpenNote?: (title: string) => Promise<boolean>
onInitializeProperties?: (path: string) => void

View File

@@ -6,6 +6,7 @@ import type { AppLocale } from '../lib/i18n'
import type { VaultEntry, GitCommit } from '../types'
import type { NoteListItem } from '../utils/ai-context'
import { Inspector, type FrontmatterValue } from './Inspector'
import type { FrontmatterOpOptions } from '../hooks/frontmatterOps'
import { AiPanelView } from './AiPanel'
import { useAiPanelController } from './useAiPanelController'
import { NEW_AI_CHAT_EVENT } from '../utils/aiPromptBridge'
@@ -34,9 +35,9 @@ interface EditorRightPanelProps {
onToggleTableOfContents?: () => void
onNavigateWikilink: (target: string) => void
onViewCommitDiff: (commitHash: string) => Promise<void>
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onDeleteProperty?: (path: string, key: string) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onDeleteProperty?: (path: string, key: string, options?: FrontmatterOpOptions) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onCreateMissingType?: (path: string, missingType: string, nextTypeName: string) => Promise<boolean | void>
onCreateAndOpenNote?: (title: string) => Promise<boolean>
onInitializeProperties?: (path: string) => void

View File

@@ -187,6 +187,19 @@ Status: Evergreened
expect(btn).toBeDisabled()
})
it('guards delete property callbacks against stale active notes', () => {
const onDeleteProperty = vi.fn().mockResolvedValue(undefined)
renderSelectedInspector({ onDeleteProperty })
fireEvent.click(screen.getAllByTitle('Delete property')[0])
expect(onDeleteProperty).toHaveBeenCalledWith(
mockEntry.path,
'Status',
{ requireActivePath: mockEntry.path },
)
})
it('shows cadence when present', () => {
// Cadence is now read from frontmatter in content (already in mockContent)
render(<Inspector {...defaultProps} entry={mockEntry} content={mockContent} />)

View File

@@ -4,6 +4,7 @@ import { cn } from '@/lib/utils'
import { Separator } from './ui/separator'
import { parseFrontmatter, detectFrontmatterState, detectFrontmatterWarnings } from '../utils/frontmatter'
import { DynamicPropertiesPanel } from './DynamicPropertiesPanel'
import type { FrontmatterOpOptions } from '../hooks/frontmatterOps'
import {
DynamicRelationshipsPanel,
BacklinksPanel,
@@ -30,9 +31,9 @@ interface InspectorProps {
vaultPath?: string
onNavigate: (target: string) => void
onViewCommitDiff?: (commitHash: string) => void
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onDeleteProperty?: (path: string, key: string) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onDeleteProperty?: (path: string, key: string, options?: FrontmatterOpOptions) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onCreateMissingType?: (path: string, missingType: string, nextTypeName: string) => Promise<boolean | void>
onCreateAndOpenNote?: (title: string) => Promise<boolean>
onInitializeProperties?: (path: string) => void

View File

@@ -1,22 +1,43 @@
import { useMemo } from 'react'
import type { VaultEntry } from '../../types'
import type { FrontmatterOpOptions } from '../../hooks/frontmatterOps'
type FrontmatterValue = string | number | boolean | string[] | null
interface InspectorPropertyActionsConfig {
entry: VaultEntry | null
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onDeleteProperty?: (path: string, key: string) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue) => Promise<void>
onUpdateFrontmatter?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onDeleteProperty?: (path: string, key: string, options?: FrontmatterOpOptions) => Promise<void>
onAddProperty?: (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => Promise<void>
onCreateMissingType?: (path: string, missingType: string, nextTypeName: string) => Promise<boolean | void>
}
function bindEntryAction<TArgs extends unknown[], TResult>(
function activeEntryOptions(entry: VaultEntry): FrontmatterOpOptions {
return { requireActivePath: entry.path }
}
function bindUpdateAction(
entry: VaultEntry | null,
action: ((path: string, ...args: TArgs) => TResult) | undefined,
action: InspectorPropertyActionsConfig['onUpdateFrontmatter'],
) {
if (!entry || !action) return undefined
return (...args: TArgs) => action(entry.path, ...args)
return (key: string, value: FrontmatterValue) => action(entry.path, key, value, activeEntryOptions(entry))
}
function bindDeleteAction(
entry: VaultEntry | null,
action: InspectorPropertyActionsConfig['onDeleteProperty'],
) {
if (!entry || !action) return undefined
return (key: string) => action(entry.path, key, activeEntryOptions(entry))
}
function bindAddAction(
entry: VaultEntry | null,
action: InspectorPropertyActionsConfig['onAddProperty'],
) {
if (!entry || !action) return undefined
return (key: string, value: FrontmatterValue) => action(entry.path, key, value, activeEntryOptions(entry))
}
function bindMissingTypeAction(
@@ -36,15 +57,15 @@ export function useInspectorPropertyActions({
onCreateMissingType,
}: InspectorPropertyActionsConfig) {
const handleUpdateProperty = useMemo(
() => bindEntryAction<[string, FrontmatterValue], Promise<void>>(entry, onUpdateFrontmatter),
() => bindUpdateAction(entry, onUpdateFrontmatter),
[entry, onUpdateFrontmatter],
)
const handleDeleteProperty = useMemo(
() => bindEntryAction<[string], Promise<void>>(entry, onDeleteProperty),
() => bindDeleteAction(entry, onDeleteProperty),
[entry, onDeleteProperty],
)
const handleAddProperty = useMemo(
() => bindEntryAction<[string, FrontmatterValue], Promise<void>>(entry, onAddProperty),
() => bindAddAction(entry, onAddProperty),
[entry, onAddProperty],
)
const handleCreateMissingType = useMemo(

View File

@@ -259,6 +259,8 @@ async function executeFrontmatterOp(
export interface FrontmatterOpOptions {
/** Suppress toast feedback (caller manages its own toast). */
silent?: boolean
/** Require this note to still be active before applying UI-side mutation state. */
requireActivePath?: VaultPath
}
export interface FrontmatterApplyCallbacks {
@@ -266,6 +268,7 @@ export interface FrontmatterApplyCallbacks {
updateEntry: (path: VaultPath, patch: Partial<VaultEntry>) => void
toast: (message: ToastMessage) => void
getEntry?: (path: VaultPath) => VaultEntry | undefined
shouldApply?: (path: VaultPath) => boolean
}
export interface FrontmatterRunRequest {
@@ -350,6 +353,7 @@ export async function runFrontmatterAndApply(request: FrontmatterRunRequest): Pr
const { op, path, key, value, callbacks, options } = request
try {
const newContent = await executeFrontmatterOp(op, path, key, value)
if (callbacks.shouldApply && !callbacks.shouldApply(path)) return undefined
callbacks.updateTab(path, newContent)
applyEntryPatch(path, callbacks, frontmatterToEntryPatch(op, key, value))
notifyFrontmatterSuccess(op, callbacks, options)

View File

@@ -226,6 +226,22 @@ describe('useNoteActions hook', () => {
expect(setToastMessage).toHaveBeenCalledWith('Property deleted')
})
it('ignores guarded inspector property deletes after the active note changes', async () => {
const noteA = makeEntry({ path: '/vault/note-a.md', filename: 'note-a.md', title: 'Note A' })
const noteB = makeEntry({ path: '/vault/note-b.md', filename: 'note-b.md', title: 'Note B' })
const { result } = renderHook(() => useNoteActions(makeConfig([noteA, noteB])))
act(() => {
result.current.handleSwitchTab(noteB.path)
})
await act(async () => {
await result.current.handleDeleteProperty(noteA.path, 'status', { requireActivePath: noteA.path })
})
expect(updateEntry).not.toHaveBeenCalled()
expect(setToastMessage).not.toHaveBeenCalled()
})
it('handleCreateNoteImmediate creates note with timestamp-based title', async () => {
const createdEntry = await createImmediateEntry()
expect(createdEntry.title).toBe('Untitled Note 1700000000')

View File

@@ -1,4 +1,4 @@
import { useCallback, useEffect } from 'react'
import { useCallback, useEffect, type MutableRefObject } from 'react'
import type { VaultEntry } from '../types'
import type { FrontmatterValue } from '../components/Inspector'
import { useTabManagement } from './useTabManagement'
@@ -163,6 +163,16 @@ async function flushBeforeNoteMutation(
}
}
function activePathGuardAllowsMutation(
path: string,
activeTabPathRef: MutableRefObject<string | null>,
options?: FrontmatterOpOptions,
): boolean {
const requiredPath = options?.requireActivePath
if (!requiredPath) return true
return notePathsMatch(path, requiredPath) && notePathsMatch(activeTabPathRef.current, requiredPath)
}
async function maybeRenameAfterFrontmatterUpdate({
path,
key,
@@ -183,16 +193,18 @@ interface UpdateFrontmatterAndMaybeRenameParams {
key: string
options?: FrontmatterOpOptions
path: string
runFrontmatterOp: (
op: 'update' | 'delete',
path: string,
key: string,
value?: FrontmatterValue,
options?: FrontmatterOpOptions,
) => Promise<string | undefined>
runFrontmatterOp: RunFrontmatterOp
value: FrontmatterValue
}
type RunFrontmatterOp = (
op: 'update' | 'delete',
path: string,
key: string,
value?: FrontmatterValue,
options?: FrontmatterOpOptions,
) => Promise<string | undefined>
async function updateFrontmatterAndMaybeRename({
config,
deps,
@@ -202,8 +214,10 @@ async function updateFrontmatterAndMaybeRename({
runFrontmatterOp,
value,
}: UpdateFrontmatterAndMaybeRenameParams): Promise<void> {
if (!activePathGuardAllowsMutation(path, deps.activeTabPathRef, options)) return
const canFlush = await flushBeforeNoteMutation(path, config.flushBeforeNoteMutation)
if (!canFlush) return
if (!activePathGuardAllowsMutation(path, deps.activeTabPathRef, options)) return
const newContent = await runFrontmatterOp('update', path, key, value, options)
if (!applyFrontmatterCallbacks({ config, path, newContent })) return
@@ -288,13 +302,7 @@ function useFrontmatterActionHandlers({
handleSwitchTab: (path: string) => void
setToastMessage: (msg: string | null) => void
updateTabContent: (path: string, newContent: string) => void
runFrontmatterOp: (
op: 'update' | 'delete',
path: string,
key: string,
value?: FrontmatterValue,
options?: FrontmatterOpOptions,
) => Promise<string | undefined>
runFrontmatterOp: RunFrontmatterOp
}) {
const handleUpdateFrontmatter = useCallback(async (
path: string,
@@ -325,22 +333,26 @@ function useFrontmatterActionHandlers({
}, [activeTabPathRef, config, handleSwitchTab, renameTabsRef, runFrontmatterOp, setTabs, setToastMessage, updateTabContent])
const handleDeleteProperty = useCallback(async (path: string, key: string, options?: FrontmatterOpOptions) => {
if (!activePathGuardAllowsMutation(path, activeTabPathRef, options)) return
const canFlush = await flushBeforeNoteMutation(path, config.flushBeforeNoteMutation)
if (!canFlush) return
if (!activePathGuardAllowsMutation(path, activeTabPathRef, options)) return
const newContent = await runFrontmatterOp('delete', path, key, undefined, options)
if (!applyFrontmatterCallbacks({ config, path, newContent })) return
await notifyFrontmatterPersisted(config, key)
}, [config, runFrontmatterOp])
}, [activeTabPathRef, config, runFrontmatterOp])
const handleAddProperty = useCallback(async (path: string, key: string, value: FrontmatterValue) => {
const handleAddProperty = useCallback(async (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => {
if (!activePathGuardAllowsMutation(path, activeTabPathRef, options)) return
const canFlush = await flushBeforeNoteMutation(path, config.flushBeforeNoteMutation)
if (!canFlush) return
if (!activePathGuardAllowsMutation(path, activeTabPathRef, options)) return
const newContent = await runFrontmatterOp('update', path, key, value)
const newContent = await runFrontmatterOp('update', path, key, value, options)
if (!applyFrontmatterCallbacks({ config, path, newContent })) return
await notifyFrontmatterPersisted(config, key)
}, [config, runFrontmatterOp])
}, [activeTabPathRef, config, runFrontmatterOp])
return {
handleUpdateFrontmatter,
@@ -349,6 +361,38 @@ function useFrontmatterActionHandlers({
}
}
function useFrontmatterRunner({
activeTabPathRef,
entries,
setToastMessage,
updateEntry,
updateTabContent,
}: {
activeTabPathRef: MutableRefObject<string | null>
entries: VaultEntry[]
setToastMessage: NoteActionsConfig['setToastMessage']
updateEntry: NoteActionsConfig['updateEntry']
updateTabContent: (path: string, newContent: string) => void
}): RunFrontmatterOp {
return useCallback(
(op, path, key, value, options) => runFrontmatterAndApply({
op,
path,
key,
value,
callbacks: {
updateTab: updateTabContent,
updateEntry,
toast: setToastMessage,
getEntry: (p) => findByNotePath(entries, p),
shouldApply: (p) => activePathGuardAllowsMutation(p, activeTabPathRef, options),
},
options,
}),
[activeTabPathRef, entries, setToastMessage, updateEntry, updateTabContent],
)
}
export function useNoteActions(config: NoteActionsConfig) {
const { entries, setToastMessage, updateEntry } = config
const tabMgmt = useTabManagement(buildTabManagementOptions(config))
@@ -379,18 +423,7 @@ export function useNoteActions(config: NoteActionsConfig) {
[entries, handleSelectNote, tabMgmt.activeTabPath, tabMgmt.tabs],
)
const runFrontmatterOp = useCallback(
(op: 'update' | 'delete', path: string, key: string, value?: FrontmatterValue, options?: FrontmatterOpOptions) =>
runFrontmatterAndApply({
op,
path,
key,
value,
callbacks: { updateTab: updateTabContent, updateEntry, toast: setToastMessage, getEntry: (p) => findByNotePath(entries, p) },
options,
}),
[updateTabContent, updateEntry, setToastMessage, entries],
)
const runFrontmatterOp = useFrontmatterRunner({ activeTabPathRef, entries, setToastMessage, updateEntry, updateTabContent })
const frontmatterActions = useFrontmatterActionHandlers({
config,
renameTabsRef: rename.tabsRef,

View File

@@ -21,6 +21,13 @@ async function openRawMode(page: Page) {
await expect(page.locator('.cm-content')).toBeVisible({ timeout: 5_000 })
}
async function openPropertiesPanel(page: Page) {
const openPanelButton = page.getByRole('button', { name: 'Open the properties panel' })
if (await openPanelButton.count()) {
await openPanelButton.click()
}
}
async function getRawEditorContent(page: Page): Promise<string> {
return page.evaluate(() => {
const el = document.querySelector('.cm-content')
@@ -81,3 +88,26 @@ test('@smoke switching notes persists unsaved raw edits without waiting for the
{ timeout: 450, intervals: [50, 100, 100, 100, 100] },
).toContain(appendedText)
})
test('@smoke deleting a property after switching notes keeps the current note editable', async ({ page }) => {
const alphaPath = path.join(tempVaultDir, 'project', 'alpha-project.md')
await openNote(page, 'Note B')
await openPropertiesPanel(page)
await expect(page.getByTestId('editable-property').filter({ hasText: 'Status' })).toBeVisible()
await openNote(page, 'Alpha Project')
await expect(page.getByTestId('breadcrumb-filename-trigger')).toContainText('alpha-project', { timeout: 5_000 })
const ownerRow = page.getByTestId('editable-property').filter({ hasText: 'Owner' })
await expect(ownerRow).toBeVisible()
await ownerRow.hover()
await ownerRow.getByTitle('Delete property').click({ force: true })
await expect(ownerRow).toHaveCount(0)
await expect(page.getByTestId('breadcrumb-filename-trigger')).toContainText('alpha-project')
await expect.poll(
() => fs.readFileSync(alphaPath, 'utf8'),
{ timeout: 1_000, intervals: [50, 100, 200, 300, 350] },
).not.toContain('Owner:')
})