From bcb9759a917c2679f1eadecc09aab0d70f036b33 Mon Sep 17 00:00:00 2001 From: murashit Date: Sat, 11 Jul 2026 17:39:09 +0900 Subject: [PATCH] Resolve audited lifecycle, protocol, Vault, and async-boundary debt --- src/app-server/connection/client-access.ts | 2 +- src/app-server/connection/json-rpc-client.ts | 36 +++++++ src/domain/vault/paths.ts | 20 +++- src/domain/vault/write-paths.ts | 21 +++++ .../application/composer/slash-commands.ts | 4 + .../chat/application/composer/suggestions.ts | 11 +-- .../chat/application/web-clipping/web-clip.ts | 12 ++- .../obsidian/composer-attachments.obsidian.ts | 34 +++---- .../vault-note-candidate-provider.obsidian.ts | 2 + .../markdown-renderer.obsidian.ts | 21 +++-- .../threads/workflows/archive-export.ts | 12 ++- src/main.ts | 16 ++-- src/settings/tab.obsidian.tsx | 50 ++++++---- src/shared/obsidian/components.obsidian.tsx | 94 +++++++++++++------ .../vault-write-destination.obsidian.ts | 1 + tests/app-server/app-server-client.test.ts | 23 +++++ .../vault-note-candidate-provider.test.ts | 4 +- .../chat/host/web-clipper.integration.test.ts | 27 ++++++ 18 files changed, 290 insertions(+), 100 deletions(-) diff --git a/src/app-server/connection/client-access.ts b/src/app-server/connection/client-access.ts index 88144c55..dca18fdd 100644 --- a/src/app-server/connection/client-access.ts +++ b/src/app-server/connection/client-access.ts @@ -1,6 +1,6 @@ import type { AppServerClient } from "./client"; -export type AppServerClientRequestPolicy = { kind: "interactive" } | { kind: "reject"; message: string }; +export type AppServerClientRequestPolicy = { kind: "reject"; message: string }; export interface AppServerClientAccessOptions { serverRequests?: AppServerClientRequestPolicy; diff --git a/src/app-server/connection/json-rpc-client.ts b/src/app-server/connection/json-rpc-client.ts index d5e1fc14..418e44dd 100644 --- a/src/app-server/connection/json-rpc-client.ts +++ b/src/app-server/connection/json-rpc-client.ts @@ -107,6 +107,13 @@ export class JsonRpcClient { return; } + const invalidParamsRequest = invalidKnownParamsRequest(parsed); + if (invalidParamsRequest) { + this.options.onLog(`Invalid app-server params: ${invalidParamsRequest.method}`); + this.reject(invalidParamsRequest.id, -32602, "Invalid params."); + return; + } + const message = rpcInboundMessage(parsed); if (!message) { this.options.onLog(`Invalid app-server JSON-RPC message: ${line}`); @@ -200,6 +207,35 @@ function isRequestId(value: unknown): value is RequestId { return typeof value === "string" || (typeof value === "number" && Number.isFinite(value)); } +function invalidKnownParamsRequest(value: unknown): { id: RequestId; method: string } | null { + if (!isRecord(value) || !Object.hasOwn(value, "id") || typeof value["method"] !== "string") return null; + const method = value["method"]; + const id = value["id"]; + const params = value["params"]; + const required = requiredStringFields(method); + if (!required) return null; + if (!isRecord(params)) return isRequestId(id) ? { id, method } : null; + return required.every((field) => typeof params[field] === "string") ? null : isRequestId(id) ? { id, method } : null; +} + +function requiredStringFields(method: string): readonly string[] | null { + switch (method) { + case "item/commandExecution/requestApproval": + case "item/fileChange/requestApproval": + return ["threadId", "turnId", "itemId"]; + case "item/tool/requestUserInput": + case "mcpServer/elicitation/request": + return ["threadId", "turnId", "itemId"]; + case "currentTime/read": + return ["threadId"]; + case "applyPatchApproval": + case "execCommandApproval": + return ["conversationId", "callId"]; + default: + return null; + } +} + function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } diff --git a/src/domain/vault/paths.ts b/src/domain/vault/paths.ts index f3c213ab..a07538dc 100644 --- a/src/domain/vault/paths.ts +++ b/src/domain/vault/paths.ts @@ -8,18 +8,18 @@ export function vaultRelativePath(vaultPath: string, path: string, options: Vaul if (!normalizedPath || !normalizedVaultPath) return null; if (!isFilesystemAbsolutePath(normalizedPath)) return options.allowRelative === true ? normalizedPath : null; - if (normalizedPath === normalizedVaultPath) return null; + if (pathsEqual(normalizedPath, normalizedVaultPath)) return null; const vaultPrefix = normalizedVaultPath.endsWith("/") ? normalizedVaultPath : `${normalizedVaultPath}/`; - return normalizedPath.startsWith(vaultPrefix) ? normalizedPath.slice(vaultPrefix.length) : null; + return pathStartsWith(normalizedPath, vaultPrefix) ? normalizedPath.slice(vaultPrefix.length) : null; } export function pathRelativeToRoot(path: string, root?: string | null): string { const normalizedPath = normalizeFilePath(path); const normalizedRoot = normalizeFilePath(root ?? ""); if (!normalizedRoot) return normalizedPath; - if (normalizedPath === normalizedRoot) return "."; - return normalizedPath.startsWith(`${normalizedRoot}/`) ? normalizedPath.slice(normalizedRoot.length + 1) : normalizedPath; + if (pathsEqual(normalizedPath, normalizedRoot)) return "."; + return pathStartsWith(normalizedPath, `${normalizedRoot}/`) ? normalizedPath.slice(normalizedRoot.length + 1) : normalizedPath; } export function isFilesystemAbsolutePath(path: string): boolean { @@ -29,7 +29,7 @@ export function isFilesystemAbsolutePath(path: string): boolean { export function isVaultConfigPath(path: string, configDir: string): boolean { const normalizedPath = normalizeFilePath(path); const normalizedConfigDir = normalizeFilePath(configDir); - return normalizedPath === normalizedConfigDir || normalizedPath.startsWith(`${normalizedConfigDir}/`); + return pathsEqual(normalizedPath, normalizedConfigDir) || pathStartsWith(normalizedPath, `${normalizedConfigDir}/`); } export function normalizeFilePath(path: string): string { @@ -40,3 +40,13 @@ export function normalizeFilePath(path: string): string { function isWindowsAbsolutePath(path: string): boolean { return /^[a-z]:[\\/]/i.test(path); } + +function pathsEqual(left: string, right: string): boolean { + return isWindowsAbsolutePath(left) || isWindowsAbsolutePath(right) ? left.toLowerCase() === right.toLowerCase() : left === right; +} + +function pathStartsWith(path: string, prefix: string): boolean { + return isWindowsAbsolutePath(path) || isWindowsAbsolutePath(prefix) + ? path.toLowerCase().startsWith(prefix.toLowerCase()) + : path.startsWith(prefix); +} diff --git a/src/domain/vault/write-paths.ts b/src/domain/vault/write-paths.ts index 10e82dcb..c5d02e49 100644 --- a/src/domain/vault/write-paths.ts +++ b/src/domain/vault/write-paths.ts @@ -1,4 +1,5 @@ export interface VaultPathDestination { + readonly writeLockKey?: object; normalizePath(path: string): string; exists(path: string): Promise; createFolder(path: string): Promise; @@ -8,6 +9,26 @@ export interface VaultMarkdownDestination extends VaultPathDestination { createMarkdownFile(path: string, content: string): Promise; } +const writeTails = new WeakMap>(); + +export async function withVaultWriteLock(destination: VaultPathDestination, operation: () => Promise): Promise { + const key = destination.writeLockKey ?? destination; + const previous = writeTails.get(key) ?? Promise.resolve(); + let release!: () => void; + const current = new Promise((resolve) => { + release = resolve; + }); + const tail = previous.then(() => current); + writeTails.set(key, tail); + await previous; + try { + return await operation(); + } finally { + release(); + if (writeTails.get(key) === tail) writeTails.delete(key); + } +} + export interface VaultRelativeFolderPathOptions { normalizePath(path: string): string; emptyPathMessage: string; diff --git a/src/features/chat/application/composer/slash-commands.ts b/src/features/chat/application/composer/slash-commands.ts index ef9818ed..7010a569 100644 --- a/src/features/chat/application/composer/slash-commands.ts +++ b/src/features/chat/application/composer/slash-commands.ts @@ -185,6 +185,10 @@ type SlashCommand = (typeof SLASH_COMMANDS)[number]["command"]; export type SlashCommandName = SlashCommand extends `/${infer Name}` ? Name : never; +export function isSlashCommandName(value: string): value is SlashCommandName { + return SLASH_COMMANDS.some((item) => item.command === `/${value}`); +} + export type SlashCommandDefinition = (typeof SLASH_COMMANDS)[number]; const CONNECTION_INDEPENDENT_SLASH_COMMANDS = new Set(["compact", "reconnect"]); diff --git a/src/features/chat/application/composer/suggestions.ts b/src/features/chat/application/composer/suggestions.ts index a8bcb0cd..fc2244d7 100644 --- a/src/features/chat/application/composer/suggestions.ts +++ b/src/features/chat/application/composer/suggestions.ts @@ -19,7 +19,7 @@ import { selectionContextReferenceMarker, } from "./context-references"; import type { DailyNoteReferenceCandidate } from "./daily-note-references"; -import { SLASH_COMMANDS, type SlashCommandName, slashCommandSubcommands } from "./slash-commands"; +import { isSlashCommandName, SLASH_COMMANDS, type SlashCommandName, slashCommandSubcommands } from "./slash-commands"; export interface ComposerSuggestion { display: string; @@ -99,8 +99,8 @@ const SELECTION_SUGGESTION_PREVIEW_LIMIT = 500; export function parseSlashCommand(text: string): { command: SlashCommandName; args: string } | null { const match = /^\/([A-Za-z-]+)(?:\s+([\s\S]*))?$/.exec(text); if (!match) return null; - const command = match[1] as SlashCommandName; - if (!SLASH_COMMANDS.some((item) => item.command === `/${command}`)) return null; + const command = match[1]; + if (!command || !isSlashCommandName(command)) return null; return { command, args: match.at(2)?.trim() ?? "" }; } @@ -416,10 +416,9 @@ function activeSlashSubcommandSuggestions(beforeCursor: string): ComposerSuggest const match = /^\/([A-Za-z-]+)\s+([A-Za-z-]{0,120})$/.exec(beforeCursor); if (!match) return null; - const command = match[1] as SlashCommandName | undefined; + const command = match[1]; const rawQuery = match[2]; - if (!command || rawQuery === undefined) return null; - if (!SLASH_COMMANDS.some((item) => item.command === `/${command}`)) return null; + if (!command || !isSlashCommandName(command) || rawQuery === undefined) return null; const query = rawQuery.toLowerCase(); const subcommands = slashCommandSubcommands(command); diff --git a/src/features/chat/application/web-clipping/web-clip.ts b/src/features/chat/application/web-clipping/web-clip.ts index 0a650608..71b0ccf3 100644 --- a/src/features/chat/application/web-clipping/web-clip.ts +++ b/src/features/chat/application/web-clipping/web-clip.ts @@ -10,6 +10,7 @@ import { sanitizeVaultPathSegment, uniqueVaultPath, type VaultMarkdownDestination, + withVaultWriteLock, } from "../../../../domain/vault/write-paths"; export interface WebClipSettings { @@ -53,11 +54,12 @@ export async function saveWebClipMarkdown( const normalizePath = (path: string): string => destination.normalizePath(path); const folder = folderPath(settings.clipFolder, normalizePath); const filename = filenameFromTemplate(settings.clipFilenameTemplate, context, normalizePath); - await ensureVaultFolder(destination, folder); - - const path = await uniqueVaultPath(destination, folder, filename); - await destination.createMarkdownFile(path, webClipMarkdown(page, settings, context.title, now)); - return { path, wikilink: `[[${path}]]` }; + return withVaultWriteLock(destination, async () => { + await ensureVaultFolder(destination, folder); + const path = await uniqueVaultPath(destination, folder, filename); + await destination.createMarkdownFile(path, webClipMarkdown(page, settings, context.title, now)); + return { path, wikilink: `[[${path}]]` }; + }); } export function webClipMarkdown(page: WebClipPage, settings: Pick, title?: string, now = new Date()): string { diff --git a/src/features/chat/host/obsidian/composer-attachments.obsidian.ts b/src/features/chat/host/obsidian/composer-attachments.obsidian.ts index c81eb859..a095c865 100644 --- a/src/features/chat/host/obsidian/composer-attachments.obsidian.ts +++ b/src/features/chat/host/obsidian/composer-attachments.obsidian.ts @@ -5,6 +5,7 @@ import { sanitizeVaultPathSegment, uniqueVaultPath, vaultRelativeFolderPath, + withVaultWriteLock, } from "../../../../domain/vault/write-paths"; import { DEFAULT_ATTACHMENT_FOLDER } from "../../../../settings/model"; import { createObsidianVaultPathDestination } from "../../../../shared/obsidian/vault-write-destination.obsidian"; @@ -50,22 +51,23 @@ async function saveComposerAttachmentFiles( const vault = options.app.vault; const destination = createObsidianVaultPathDestination(vault); const folder = attachmentFolderPath(options.attachmentFolder(), (path) => destination.normalizePath(path)); - await ensureVaultFolder(destination, folder); - - const attachments: ComposerAttachment[] = []; - for (const file of files) { - const filename = attachmentFilename(file, options.now?.() ?? new Date()); - const path = await uniqueVaultPath(destination, folder, filename); - await vault.createBinary(path, await file.arrayBuffer()); - const kind = isImageFile(file, path) ? "image" : "file"; - attachments.push({ - kind, - name: attachmentDisplayName(path), - path, - marker: kind === "image" ? `![[${path}]]` : `[[${path}]]`, - }); - } - return attachments; + return withVaultWriteLock(destination, async () => { + await ensureVaultFolder(destination, folder); + const attachments: ComposerAttachment[] = []; + for (const file of files) { + const filename = attachmentFilename(file, options.now?.() ?? new Date()); + const path = await uniqueVaultPath(destination, folder, filename); + await vault.createBinary(path, await file.arrayBuffer()); + const kind = isImageFile(file, path) ? "image" : "file"; + attachments.push({ + kind, + name: attachmentDisplayName(path), + path, + marker: kind === "image" ? `![[${path}]]` : `[[${path}]]`, + }); + } + return attachments; + }); } function attachmentFolderPath(value: string, normalizePath: (path: string) => string): string { diff --git a/src/features/chat/host/obsidian/vault-note-candidate-provider.obsidian.ts b/src/features/chat/host/obsidian/vault-note-candidate-provider.obsidian.ts index 990dc1a6..78f18487 100644 --- a/src/features/chat/host/obsidian/vault-note-candidate-provider.obsidian.ts +++ b/src/features/chat/host/obsidian/vault-note-candidate-provider.obsidian.ts @@ -87,6 +87,8 @@ class VaultNoteCandidateCatalog { this.registerEvent(app.vault, app.vault.on("modify", invalidate)); this.registerEvent(app.metadataCache, app.metadataCache.on("changed", invalidate)); this.registerEvent(app.metadataCache, app.metadataCache.on("deleted", invalidate)); + this.registerEvent(app.workspace, app.workspace.on("file-open", invalidate)); + this.registerEvent(app.workspace, app.workspace.on("active-leaf-change", invalidate)); } candidates(sourcePath: string): readonly NoteCandidate[] { diff --git a/src/features/chat/ui/thread-stream/markdown-renderer.obsidian.ts b/src/features/chat/ui/thread-stream/markdown-renderer.obsidian.ts index 103a1306..31de362e 100644 --- a/src/features/chat/ui/thread-stream/markdown-renderer.obsidian.ts +++ b/src/features/chat/ui/thread-stream/markdown-renderer.obsidian.ts @@ -41,14 +41,19 @@ export class ThreadStreamMarkdownRenderer { this.renderGenerations.set(parent, generation); const staging = parent.createDiv(); staging.remove(); - void MarkdownRenderer.render(this.options.app, text, staging, sourcePath, this.options.owner).then(() => { - if (!parent.isConnected || this.renderGenerations.get(parent) !== generation) return; - parent.replaceChildren(...Array.from(staging.childNodes)); - bindRenderedWikiLinks(parent, sourcePath, this.options); - bindRenderedMarkdownFileLinks(parent, sourcePath, this.options); - bindRenderedTags(parent, this.options); - notifyThreadStreamContentRendered(parent); - }); + void MarkdownRenderer.render(this.options.app, text, staging, sourcePath, this.options.owner) + .then(() => { + if (!parent.isConnected || this.renderGenerations.get(parent) !== generation) return; + parent.replaceChildren(...Array.from(staging.childNodes)); + bindRenderedWikiLinks(parent, sourcePath, this.options); + bindRenderedMarkdownFileLinks(parent, sourcePath, this.options); + bindRenderedTags(parent, this.options); + notifyThreadStreamContentRendered(parent); + }) + .catch((error: unknown) => { + if (!parent.isConnected || this.renderGenerations.get(parent) !== generation) return; + new Notice(`Failed to render Codex message: ${error instanceof Error ? error.message : String(error)}`); + }); } } diff --git a/src/features/threads/workflows/archive-export.ts b/src/features/threads/workflows/archive-export.ts index e2317623..3dc48be6 100644 --- a/src/features/threads/workflows/archive-export.ts +++ b/src/features/threads/workflows/archive-export.ts @@ -12,6 +12,7 @@ import { sanitizeVaultPathSegment, uniqueVaultPath, type VaultMarkdownDestination, + withVaultWriteLock, } from "../../../domain/vault/write-paths"; export interface ArchiveExportResult { @@ -38,11 +39,12 @@ export async function exportArchivedThreadMarkdown( const normalizePath = (path: string): string => destination.normalizePath(path); const folder = folderPath(settings.archiveExportFolderTemplate, normalizePath); const filename = filenameFromTemplate(settings.archiveExportFilenameTemplate, context, normalizePath); - await ensureVaultFolder(destination, folder); - - const path = await uniqueVaultPath(destination, folder, filename); - await destination.createMarkdownFile(path, archivedThreadMarkdown(thread, now, settings)); - return { path }; + return withVaultWriteLock(destination, async () => { + await ensureVaultFolder(destination, folder); + const path = await uniqueVaultPath(destination, folder, filename); + await destination.createMarkdownFile(path, archivedThreadMarkdown(thread, now, settings)); + return { path }; + }); } function templateContext(thread: ArchiveThreadInput, now: Date): TemplateContext { diff --git a/src/main.ts b/src/main.ts index 37bd4b84..00d150f2 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,4 +1,4 @@ -import { Plugin } from "obsidian"; +import { Notice, Plugin } from "obsidian"; import { VIEW_TYPE_CODEX_PANEL, VIEW_TYPE_CODEX_THREADS, VIEW_TYPE_CODEX_TURN_DIFF } from "./constants"; import { CodexChatView } from "./features/chat/host/view.obsidian"; @@ -35,25 +35,25 @@ export default class CodexPanelPlugin extends Plugin { ); this.addRibbonIcon("bot-message-square", "Open panel", () => { - void this.runtime.activatePanel(); + void this.runtime.activatePanel().catch(reportCommandError); }); this.addCommand({ id: "open-panel", name: "Open panel", - callback: () => void this.runtime.activatePanel(), + callback: () => void this.runtime.activatePanel().catch(reportCommandError), }); this.addCommand({ id: "open-new-panel", name: "Open new panel", - callback: () => void this.runtime.activateNewPanel(), + callback: () => void this.runtime.activateNewPanel().catch(reportCommandError), }); this.addCommand({ id: "open-threads-view", name: "Open threads view", - callback: () => void this.runtime.activateThreadsView(), + callback: () => void this.runtime.activateThreadsView().catch(reportCommandError), }); this.addCommand({ @@ -67,7 +67,7 @@ export default class CodexPanelPlugin extends Plugin { this.addCommand({ id: "new-chat", name: "Start new chat", - callback: () => void this.runtime.startNewChat(), + callback: () => void this.runtime.startNewChat().catch(reportCommandError), }); registerSelectionRewriteCommand(this); @@ -94,3 +94,7 @@ export default class CodexPanelPlugin extends Plugin { await this.saveData(this.settings); } } + +function reportCommandError(error: unknown): void { + new Notice(`Codex Panel command failed: ${error instanceof Error ? error.message : String(error)}`); +} diff --git a/src/settings/tab.obsidian.tsx b/src/settings/tab.obsidian.tsx index 804f0bae..0f458351 100644 --- a/src/settings/tab.obsidian.tsx +++ b/src/settings/tab.obsidian.tsx @@ -6,6 +6,7 @@ import { listenDomEvent } from "../shared/dom/events.dom"; import { renderUiRoot, unmountUiRoot } from "../shared/dom/preact-root.dom"; import { SettingsDynamicSectionsController } from "./dynamic-sections-controller"; import type { CodexPanelSettingTabHost } from "./host"; +import type { CodexPanelSettings } from "./model"; import { DEFAULT_ARCHIVE_EXPORT_FILENAME_TEMPLATE, DEFAULT_ARCHIVE_EXPORT_FOLDER_TEMPLATE, @@ -20,6 +21,7 @@ const SETTINGS_INTRO_TEXT = "Codex Panel stores panel preferences only. Runtime export class CodexPanelSettingTab extends PluginSettingTab { private readonly dynamicSections: SettingsDynamicSectionsController; + private lastSavedSettings: CodexPanelSettings; private archivedDeleteConfirmThreadId: string | null = null; private disposeOutsidePointer: (() => void) | null = null; private readonly cancelArchivedDeleteConfirmOnOutsidePointer = (event: PointerEvent): void => { @@ -40,6 +42,7 @@ export class CodexPanelSettingTab extends PluginSettingTab { private readonly plugin: CodexPanelSettingTabHost, ) { super(app, owner); + this.lastSavedSettings = { ...plugin.settings }; this.dynamicSections = new SettingsDynamicSectionsController(plugin, { display: () => { this.renderSettingsShell(); @@ -201,7 +204,7 @@ export class CodexPanelSettingTab extends PluginSettingTab { const codexPath = value.trim() || DEFAULT_CODEX_PATH; if (codexPath === this.plugin.settings.codexPath) return; this.plugin.settings.codexPath = codexPath; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.dynamicSections.resetDynamicSectionContext(); this.plugin.dynamicData.notifyContextChanged(); this.plugin.refreshOpenViews(); @@ -210,74 +213,74 @@ export class CodexPanelSettingTab extends PluginSettingTab { private async setShowToolbar(value: boolean): Promise { this.plugin.settings.showToolbar = value; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.plugin.refreshOpenViews(); this.renderSettingsShell(); } private async setSendShortcut(value: "enter" | "mod-enter"): Promise { this.plugin.settings.sendShortcut = value; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setScrollThreadFromComposerEdges(value: boolean): Promise { this.plugin.settings.scrollThreadFromComposerEdges = value; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setReferenceActiveNoteOnSend(value: boolean): Promise { this.plugin.settings.referenceActiveNoteOnSend = value; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setAttachmentFolder(value: string): Promise { this.plugin.settings.attachmentFolder = value.trim() || DEFAULT_ATTACHMENT_FOLDER; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setClipFolder(value: string): Promise { this.plugin.settings.clipFolder = value.trim() || DEFAULT_CLIP_FOLDER; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setClipFilenameTemplate(value: string): Promise { this.plugin.settings.clipFilenameTemplate = value.trim() || DEFAULT_CLIP_FILENAME_TEMPLATE; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setClipTags(value: string): Promise { this.plugin.settings.clipTags = value.trim(); - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setArchiveExportEnabled(enabled: boolean): Promise { this.plugin.settings.archiveExportEnabled = enabled; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setArchiveExportFolderTemplate(value: string): Promise { this.plugin.settings.archiveExportFolderTemplate = value.trim() || DEFAULT_ARCHIVE_EXPORT_FOLDER_TEMPLATE; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setArchiveExportFilenameTemplate(value: string): Promise { this.plugin.settings.archiveExportFilenameTemplate = value.trim() || DEFAULT_ARCHIVE_EXPORT_FILENAME_TEMPLATE; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setArchiveExportTags(value: string): Promise { this.plugin.settings.archiveExportTags = value.trim(); - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } @@ -286,13 +289,13 @@ export class CodexPanelSettingTab extends PluginSettingTab { if (!this.dynamicSections.namingEffortSupported(this.plugin.settings.threadNamingEffort)) { this.plugin.settings.threadNamingEffort = null; } - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setThreadNamingEffort(value: ReasoningEffort | null): Promise { this.plugin.settings.threadNamingEffort = value; - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; } private async setRewriteSelectionModel(value: string | null): Promise { @@ -300,12 +303,25 @@ export class CodexPanelSettingTab extends PluginSettingTab { if (!this.dynamicSections.rewriteSelectionEffortSupported(this.plugin.settings.rewriteSelectionEffort)) { this.plugin.settings.rewriteSelectionEffort = null; } - await this.plugin.saveSettings(); + if (!(await this.persistSettings())) return; this.renderSettingsShell(); } private async setRewriteSelectionEffort(value: ReasoningEffort | null): Promise { this.plugin.settings.rewriteSelectionEffort = value; - await this.plugin.saveSettings(); + await this.persistSettings(); + } + + private async persistSettings(): Promise { + try { + await this.plugin.saveSettings(); + this.lastSavedSettings = { ...this.plugin.settings }; + return true; + } catch (error) { + Object.assign(this.plugin.settings, this.lastSavedSettings); + new Notice(`Failed to save Codex Panel settings: ${error instanceof Error ? error.message : String(error)}`); + this.renderSettingsShell(); + return false; + } } } diff --git a/src/shared/obsidian/components.obsidian.tsx b/src/shared/obsidian/components.obsidian.tsx index ec7e547c..6f9a1d67 100644 --- a/src/shared/obsidian/components.obsidian.tsx +++ b/src/shared/obsidian/components.obsidian.tsx @@ -122,21 +122,32 @@ export function ObsidianDropdown({ }): UiNode { const ref = useRef(null); const onChangeRef = useLatestRef(onChange); + const optionsRef = useLatestRef(options); + const dropdownRef = useRef(null); + const optionsKey = options.map((option) => `${option.value}\u0000${option.label}`).join("\u0001"); useLayoutEffect(() => { const container = ref.current; if (!container) return; - container.empty(); const dropdown = new DropdownComponent(container); - for (const option of options) { - dropdown.addOption(option.value, option.label); - } - dropdown.setValue(value).onChange((selected) => { + dropdownRef.current = dropdown; + dropdown.onChange((selected) => { onChangeRef.current(selected); }); return () => { + dropdownRef.current = null; container.empty(); }; - }, [onChangeRef, options, value]); + }, [onChangeRef]); + // biome-ignore lint/correctness/useExhaustiveDependencies: the latest ref lets option identity stay stable while the semantic key controls reconstruction. + useLayoutEffect(() => { + const dropdown = dropdownRef.current; + if (!dropdown) return; + dropdown.selectEl.replaceChildren(); + for (const option of optionsRef.current) dropdown.addOption(option.value, option.label); + }, [optionsKey]); + useLayoutEffect(() => { + dropdownRef.current?.setValue(value); + }, [value]); return ; } @@ -155,12 +166,12 @@ export function ObsidianCommitTextInput({ const ref = useRef(null); const normalizeValueRef = useLatestRef(normalizeValue); const onCommitRef = useLatestRef(onCommit); + const textRef = useRef(null); useLayoutEffect(() => { const container = ref.current; if (!container) return; - container.empty(); const text = new TextComponent(container); - text.setPlaceholder(placeholder).setValue(value); + textRef.current = text; const commit = () => { const committedValue = normalizeValueRef.current?.(text.inputEl.value) ?? text.inputEl.value; text.inputEl.value = committedValue; @@ -174,10 +185,17 @@ export function ObsidianCommitTextInput({ commit(); }), () => { + textRef.current = null; container.empty(); }, ); - }, [normalizeValueRef, onCommitRef, placeholder, value]); + }, [normalizeValueRef, onCommitRef]); + useLayoutEffect(() => { + const text = textRef.current; + if (!text) return; + text.setPlaceholder(placeholder); + if (text.inputEl !== text.inputEl.ownerDocument.activeElement) text.setValue(value); + }, [placeholder, value]); return ; } @@ -185,18 +203,23 @@ export function ObsidianCommitTextInput({ export function ObsidianToggle({ checked, onChange }: { checked: boolean; onChange: (checked: boolean) => void }): UiNode { const ref = useRef(null); const onChangeRef = useLatestRef(onChange); + const toggleRef = useRef(null); useLayoutEffect(() => { const container = ref.current; if (!container) return; - container.empty(); const toggle = new ToggleComponent(container); - toggle.setValue(checked).onChange((nextValue) => { + toggleRef.current = toggle; + toggle.onChange((nextValue) => { onChangeRef.current(nextValue); }); return () => { + toggleRef.current = null; container.empty(); }; - }, [checked, onChangeRef]); + }, [onChangeRef]); + useLayoutEffect(() => { + toggleRef.current?.setValue(checked); + }, [checked]); return ; } @@ -213,46 +236,59 @@ export function ObsidianExtraButton({ onClick: () => void; }): UiNode { const ref = useRef(null); + const onClickRef = useLatestRef(onClick); + const buttonRef = useRef(null); + const classPartsRef = useRef([]); useLayoutEffect(() => { const container = ref.current; if (!container) return; - container.empty(); - const button = new ExtraButtonComponent(container).setIcon(icon).setTooltip(label).onClick(onClick); - button.extraSettingsEl.ariaLabel = label; + const button = new ExtraButtonComponent(container).onClick(() => { + onClickRef.current(); + }); + buttonRef.current = button; const stopPointerDown = (event: PointerEvent): void => { event.stopPropagation(); }; const disposePointerDown = listenDomEvent(button.extraSettingsEl, "pointerdown", stopPointerDown); - if (className) { - for (const classPart of className.split(" ").filter(Boolean)) { - button.extraSettingsEl.addClass(classPart); - } - } return disposeDomListeners(disposePointerDown, () => { + buttonRef.current = null; container.empty(); }); - }, [className, icon, label, onClick]); + }, [onClickRef]); + useLayoutEffect(() => { + const button = buttonRef.current; + if (!button) return; + button.setIcon(icon).setTooltip(label); + button.extraSettingsEl.ariaLabel = label; + for (const classPart of classPartsRef.current) button.extraSettingsEl.classList.remove(classPart); + const classParts = className?.split(" ").filter(Boolean) ?? []; + for (const classPart of classParts) button.extraSettingsEl.classList.add(classPart); + classPartsRef.current = classParts; + }, [className, icon, label]); return ; } export function ObsidianButton({ text, disabled, onClick }: { text: string; disabled?: boolean; onClick: () => void }): UiNode { const ref = useRef(null); + const onClickRef = useLatestRef(onClick); + const buttonRef = useRef(null); useLayoutEffect(() => { const container = ref.current; if (!container) return; - container.empty(); - const button = new ButtonComponent(container) - .setButtonText(text) - .setDisabled(disabled ?? false) - .onClick(() => { - onClick(); - }); + const button = new ButtonComponent(container).onClick(() => { + onClickRef.current(); + }); + buttonRef.current = button; button.buttonEl.type = "button"; return () => { + buttonRef.current = null; container.empty(); }; - }, [disabled, onClick, text]); + }, [onClickRef]); + useLayoutEffect(() => { + buttonRef.current?.setButtonText(text).setDisabled(disabled ?? false); + }, [disabled, text]); return ; } diff --git a/src/shared/obsidian/vault-write-destination.obsidian.ts b/src/shared/obsidian/vault-write-destination.obsidian.ts index 5e555cf5..5a6b3124 100644 --- a/src/shared/obsidian/vault-write-destination.obsidian.ts +++ b/src/shared/obsidian/vault-write-destination.obsidian.ts @@ -4,6 +4,7 @@ import type { VaultMarkdownDestination, VaultPathDestination } from "../../domai export function createObsidianVaultPathDestination(vault: Vault): VaultPathDestination { return { + writeLockKey: vault, normalizePath, exists: async (path: string): Promise => vault.getAbstractFileByPath(normalizePath(path)) !== null, createFolder: async (path: string): Promise => { diff --git a/tests/app-server/app-server-client.test.ts b/tests/app-server/app-server-client.test.ts index f5ac357d..f296f926 100644 --- a/tests/app-server/app-server-client.test.ts +++ b/tests/app-server/app-server-client.test.ts @@ -269,6 +269,29 @@ describe("AppServerClient", () => { expect(logs).toEqual(["App-server notification handler failed: notification failed"]); }); + it("rejects known server requests with malformed required params", async () => { + let transport!: FakeTransport; + const client = createTestClient({ + handlers: { + onNotification: () => undefined, + onServerRequest: () => undefined, + onLog: () => undefined, + onExit: () => undefined, + }, + transportFactory: (handlers) => { + transport = new FakeTransport(handlers); + return transport; + }, + }); + const connecting = client.connect(); + transport.emitLine({ id: 1, result: { codexHome: "/tmp/codex" } satisfies Partial }); + await connecting; + + transport.emitLine({ id: 12, method: "currentTime/read", params: { threadId: 42 } }); + + expect(latestSent(transport)).toEqual({ id: 12, error: { code: -32602, message: "Invalid params." } }); + }); + it("sends typed client requests", async () => { const { client, transport } = await connectedClient(); diff --git a/tests/features/chat/host/vault-note-candidate-provider.test.ts b/tests/features/chat/host/vault-note-candidate-provider.test.ts index c829ab03..867890d5 100644 --- a/tests/features/chat/host/vault-note-candidate-provider.test.ts +++ b/tests/features/chat/host/vault-note-candidate-provider.test.ts @@ -209,7 +209,7 @@ describe("VaultNoteCandidateProvider", () => { provider.dispose(); - expect(app.offref).toHaveBeenCalledTimes(6); + expect(app.offref).toHaveBeenCalledTimes(8); }); it("shares candidate caches and event subscriptions across panel providers", () => { @@ -226,7 +226,7 @@ describe("VaultNoteCandidateProvider", () => { expect(app.offref).not.toHaveBeenCalled(); second.dispose(); - expect(app.offref).toHaveBeenCalledTimes(6); + expect(app.offref).toHaveBeenCalledTimes(8); }); it("resolves wikilinks through metadata cache before direct path fallback", () => { diff --git a/tests/features/chat/host/web-clipper.integration.test.ts b/tests/features/chat/host/web-clipper.integration.test.ts index 04b8c37c..e11226e2 100644 --- a/tests/features/chat/host/web-clipper.integration.test.ts +++ b/tests/features/chat/host/web-clipper.integration.test.ts @@ -55,6 +55,33 @@ describe("vault web clipper parser integration", () => { expect(markdown).not.toContain("Navigation that should not be clipped"); expect(result?.text).toBe("[[Clips/Integration Article.md]]"); }); + + it("preserves full-parser Markdown features required by the clip contract", async () => { + mocks.requestUrl.mockResolvedValue({ + text: `Rich Article
+

Rich heading

Formula x

+

See the note1.

+

Obsidian callout

+ 
const value = 1;
+
  1. Footnote detail
+
`, + }); + const { vault, createdContent } = memoryVault(); + + await createVaultWebClipper({ + vault, + settings: () => ({ clipFolder: "Clips", clipFilenameTemplate: "{{title}}.md", clipTags: "" }), + prepareInput: () => ({ text: "", input: [{ type: "text", text: "" }] }), + viewWindow: () => window, + now: () => new Date("2026-07-10T00:00:00.000Z"), + }).clipUrl("https://example.com/rich", "", {} as ComposerInputSnapshot); + + const markdown = createdContent.get("Clips/Rich Article.md") ?? ""; + expect(markdown).toContain("Rich heading"); + expect(markdown).toMatch(/\$|\\frac|math/); + expect(markdown).toContain("Footnote detail"); + expect(markdown).toContain("```\nconst value = 1;\n```"); + }); }); function memoryVault(): { vault: Vault; createdContent: Map } {