From 2de0dc83998e07b84934620587cb0745238f6b98 Mon Sep 17 00:00:00 2001 From: murashit Date: Sun, 21 Jun 2026 19:23:06 +0900 Subject: [PATCH] Move single-use helpers to owning boundaries --- eslint.config.mjs | 10 +----- scripts/lint/eslint-plugin-codex-panel.mjs | 24 +++----------- .../services/abortable-operation.ts} | 6 ++-- .../services/ephemeral-structured-turn.ts | 6 ++-- src/app-server/services/runtime-overrides.ts | 32 ++++++++++++++++-- src/domain/runtime/overrides.ts | 33 ------------------- .../application/composer/wikilink-context.ts | 18 ++++++++-- src/shared/obsidian/wikilinks.ts | 23 ------------- tests/scripts/eslint-config.test.ts | 23 ++++++++++--- 9 files changed, 75 insertions(+), 100 deletions(-) rename src/{shared/lifecycle/abortable.ts => app-server/services/abortable-operation.ts} (56%) delete mode 100644 src/domain/runtime/overrides.ts delete mode 100644 src/shared/obsidian/wikilinks.ts diff --git a/eslint.config.mjs b/eslint.config.mjs index 0b03769f..02c188b9 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -190,7 +190,6 @@ const nonChatImperativeDomBridgeFiles = [ "src/shared/ui/textarea-caret.ts", "src/shared/ui/ui-root.tsx", ]; -const nonUiEventListenerFiles = ["src/shared/lifecycle/abortable.ts", "src/shared/ui/dom-events.ts"]; const appServerProjectionRpcMethodPattern = "^(resumeThread|threadTurnsList|forkThread|rollbackThread)$"; const appServerProjectionRpcRestrictions = [ { @@ -315,7 +314,7 @@ export default defineConfig([ }, { files: ["src/**/*.{ts,tsx}"], - ignores: ["src/features/chat/**/*.{ts,tsx}", ...nonChatImperativeDomBridgeFiles, ...nonUiEventListenerFiles], + ignores: ["src/features/chat/**/*.{ts,tsx}", ...nonChatImperativeDomBridgeFiles], rules: { ...restrictedSyntaxRule(sourceSyntaxRestrictions), "codex-panel/no-imperative-dom": "error", @@ -357,13 +356,6 @@ export default defineConfig([ files: nonChatImperativeDomBridgeFiles, rules: restrictedSyntaxRule(nonChatDomBridgeSyntaxRestrictions), }, - { - files: nonUiEventListenerFiles, - rules: { - ...restrictedSyntaxRule(sourceSyntaxRestrictions), - "codex-panel/no-imperative-dom": ["error", { allowEvents: true }], - }, - }, { files: uiRootBridgeFiles, rules: restrictedSyntaxRule(sourceSyntaxRestrictionsWithoutUiRoot), diff --git a/scripts/lint/eslint-plugin-codex-panel.mjs b/scripts/lint/eslint-plugin-codex-panel.mjs index e7250d26..919a54c5 100644 --- a/scripts/lint/eslint-plugin-codex-panel.mjs +++ b/scripts/lint/eslint-plugin-codex-panel.mjs @@ -168,21 +168,9 @@ const codexPanelEslintPlugin = { event: "Keep imperative DOM event wiring in an explicit bridge module or Obsidian-owned UI boundary.", write: "Keep imperative DOM writes in an explicit bridge module or Obsidian-owned UI boundary.", }, - schema: [ - { - type: "object", - additionalProperties: false, - properties: { - allowEvents: { type: "boolean" }, - allowWrites: { type: "boolean" }, - }, - }, - ], + schema: [], }, create(context) { - const options = context.options[0] ?? {}; - const allowEvents = options.allowEvents === true; - const allowWrites = options.allowWrites === true; let parserServices = null; let checker = null; @@ -206,7 +194,7 @@ const codexPanelEslintPlugin = { return { AssignmentExpression(node) { - if (allowWrites || !isMemberExpression(node.left)) return; + if (!isMemberExpression(node.left)) return; const property = staticPropertyName(node.left.property); if (!property || !imperativeDomAssignmentProperties.has(property)) return; if (isDomTarget(node.left.object)) context.report({ node: node.left, messageId: "write" }); @@ -215,11 +203,11 @@ const codexPanelEslintPlugin = { if (!isMemberExpression(node.callee)) return; const method = staticPropertyName(node.callee.property); if (!method) return; - if (!allowWrites && imperativeDomWriteMethods.has(method) && isDomTarget(node.callee.object)) { + if (imperativeDomWriteMethods.has(method) && isDomTarget(node.callee.object)) { context.report({ node: node.callee, messageId: "write" }); return; } - if (!allowEvents && imperativeDomEventMethods.has(method) && isDomTarget(node.callee.object)) { + if (imperativeDomEventMethods.has(method) && isDomTarget(node.callee.object)) { context.report({ node: node.callee, messageId: "event" }); } }, @@ -332,9 +320,7 @@ function typeCanCarryChatStateMutation(type, seen = new Set()) { } function domTypeName(name) { - return /\b(?:AbortSignal|Document|Element|EventTarget|HTML[A-Za-z]*Element|HTMLElement|Node|SVG[A-Za-z]*Element|SVGElement|Window)\b/.test( - name, - ); + return /\b(?:Document|Element|HTML[A-Za-z]*Element|HTMLElement|Node|SVG[A-Za-z]*Element|SVGElement|Window)\b/.test(name); } function chatStateTypeName(name) { diff --git a/src/shared/lifecycle/abortable.ts b/src/app-server/services/abortable-operation.ts similarity index 56% rename from src/shared/lifecycle/abortable.ts rename to src/app-server/services/abortable-operation.ts index beefd089..00a29d7a 100644 --- a/src/shared/lifecycle/abortable.ts +++ b/src/app-server/services/abortable-operation.ts @@ -1,10 +1,10 @@ -export function throwIfAbortSignalAborted(signal: AbortSignal | undefined, abortError: () => Error): void { +export function throwIfSignalAborted(signal: AbortSignal | undefined, abortError: () => Error): void { if (signal?.aborted) throw abortError(); } -export function abortablePromise(promise: Promise, signal: AbortSignal | undefined, abortError: () => Error): Promise { +export function abortableOperation(promise: Promise, signal: AbortSignal | undefined, abortError: () => Error): Promise { if (!signal) return promise; - throwIfAbortSignalAborted(signal, abortError); + throwIfSignalAborted(signal, abortError); return new Promise((resolve, reject) => { const onAbort = (): void => { reject(abortError()); diff --git a/src/app-server/services/ephemeral-structured-turn.ts b/src/app-server/services/ephemeral-structured-turn.ts index c73a2581..e8cedf87 100644 --- a/src/app-server/services/ephemeral-structured-turn.ts +++ b/src/app-server/services/ephemeral-structured-turn.ts @@ -4,10 +4,10 @@ import { type AppServerStartEphemeralThreadOptions, type AppServerStartStructuredTurnOptions, } from "../connection/client"; -import { abortablePromise, throwIfAbortSignalAborted } from "../../shared/lifecycle/abortable"; import type { RequestId, ServerNotification } from "../connection/rpc-messages"; import type { ModelMetadataClient } from "../catalog"; import { lastAgentMessageTextFromTurnRecord, type TurnItem, type TurnRecord } from "../protocol/turn"; +import { abortableOperation, throwIfSignalAborted } from "./abortable-operation"; export type StructuredTurnOutputSchema = AppServerStartStructuredTurnOptions["outputSchema"]; @@ -258,11 +258,11 @@ function turnWithCollectedItems(turn: TurnRecord, completedItems: readonly TurnI } function throwIfAborted(signal: AbortSignal | undefined, message: string | undefined): void { - throwIfAbortSignalAborted(signal, () => ephemeralStructuredTurnAbortError(message)); + throwIfSignalAborted(signal, () => ephemeralStructuredTurnAbortError(message)); } function abortable(promise: Promise, signal: AbortSignal | undefined, abortError: () => Error): Promise { - return abortablePromise(promise, signal, abortError); + return abortableOperation(promise, signal, abortError); } function ephemeralStructuredTurnAbortError(message: string | undefined): Error { diff --git a/src/app-server/services/runtime-overrides.ts b/src/app-server/services/runtime-overrides.ts index 0fccc116..86bc09b6 100644 --- a/src/app-server/services/runtime-overrides.ts +++ b/src/app-server/services/runtime-overrides.ts @@ -1,7 +1,17 @@ -import type { RuntimeOverride, RuntimeOverrideSettings } from "../../domain/runtime/overrides"; -import { runtimeOverride, validatedRuntimeOverrideForModelMetadata } from "../../domain/runtime/overrides"; +import type { ModelMetadata, ReasoningEffort } from "../../domain/catalog/metadata"; +import { findModelMetadataByIdOrName, supportedEffortsForModelMetadata } from "../../domain/catalog/metadata"; import { listModelMetadata, type ModelMetadataClient } from "../catalog"; +export interface RuntimeOverrideSettings { + model: string | null; + effort: ReasoningEffort | null; +} + +export interface RuntimeOverride { + model?: string; + effort?: ReasoningEffort; +} + export async function resolvedRuntimeOverrideForClient( client: ModelMetadataClient, settings: RuntimeOverrideSettings, @@ -14,3 +24,21 @@ export async function resolvedRuntimeOverrideForClient( return runtime; } } + +function runtimeOverride(settings: RuntimeOverrideSettings): RuntimeOverride { + return { + ...(settings.model ? { model: settings.model } : {}), + ...(settings.effort ? { effort: settings.effort } : {}), + }; +} + +function validatedRuntimeOverrideForModelMetadata(settings: RuntimeOverrideSettings, models: readonly ModelMetadata[]): RuntimeOverride { + const runtime = runtimeOverride(settings); + if (!runtime.model || !runtime.effort) return runtime; + + const model = findModelMetadataByIdOrName(models, runtime.model); + if (!model) return runtime; + + const supportedEfforts = new Set(supportedEffortsForModelMetadata(model)); + return supportedEfforts.has(runtime.effort) ? runtime : { model: runtime.model }; +} diff --git a/src/domain/runtime/overrides.ts b/src/domain/runtime/overrides.ts deleted file mode 100644 index 91fe327a..00000000 --- a/src/domain/runtime/overrides.ts +++ /dev/null @@ -1,33 +0,0 @@ -import type { ModelMetadata, ReasoningEffort } from "../catalog/metadata"; -import { findModelMetadataByIdOrName, supportedEffortsForModelMetadata } from "../catalog/metadata"; - -export interface RuntimeOverrideSettings { - model: string | null; - effort: ReasoningEffort | null; -} - -export interface RuntimeOverride { - model?: string; - effort?: ReasoningEffort; -} - -export function runtimeOverride(settings: RuntimeOverrideSettings): RuntimeOverride { - return { - ...(settings.model ? { model: settings.model } : {}), - ...(settings.effort ? { effort: settings.effort } : {}), - }; -} - -export function validatedRuntimeOverrideForModelMetadata( - settings: RuntimeOverrideSettings, - models: readonly ModelMetadata[], -): RuntimeOverride { - const runtime = runtimeOverride(settings); - if (!runtime.model || !runtime.effort) return runtime; - - const model = findModelMetadataByIdOrName(models, runtime.model); - if (!model) return runtime; - - const supportedEfforts = new Set(supportedEffortsForModelMetadata(model)); - return supportedEfforts.has(runtime.effort) ? runtime : { model: runtime.model }; -} diff --git a/src/features/chat/application/composer/wikilink-context.ts b/src/features/chat/application/composer/wikilink-context.ts index 7941f0a0..81de22c6 100644 --- a/src/features/chat/application/composer/wikilink-context.ts +++ b/src/features/chat/application/composer/wikilink-context.ts @@ -1,6 +1,7 @@ +import { parseLinktext } from "obsidian"; + import { codexTextInputWithMentions, type RequestAdditionalContext, type RequestMention } from "../../../../domain/chat/input"; import type { SkillMetadata } from "../../../../domain/catalog/metadata"; -import { parseObsidianWikiLink } from "../../../../shared/obsidian/wikilinks"; export interface ParsedWikiLink { raw: string; @@ -88,8 +89,19 @@ function parsedSkillReferences(text: string): string[] { } function parseWikiLink(raw: string): ParsedWikiLink | null { - const parsed = parseObsidianWikiLink(raw); - return parsed ? { raw, ...parsed } : null; + const trimmed = raw.trim(); + if (!trimmed) return null; + + const separator = trimmed.indexOf("|"); + const linktext = (separator === -1 ? trimmed : trimmed.slice(0, separator)).trim(); + const display = separator === -1 ? "" : trimmed.slice(separator + 1).trim(); + if (!linktext) return null; + + const parsed = parseLinktext(linktext); + const target = parsed.path.trim(); + const subpath = parsed.subpath.trim(); + if (!target) return null; + return { raw, target, subpath, display }; } function firstEnabledSkillByName(skills: readonly SkillMetadata[]): Map { diff --git a/src/shared/obsidian/wikilinks.ts b/src/shared/obsidian/wikilinks.ts deleted file mode 100644 index 08297c35..00000000 --- a/src/shared/obsidian/wikilinks.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { parseLinktext } from "obsidian"; - -export interface ParsedObsidianWikiLink { - target: string; - subpath: string; - display: string; -} - -export function parseObsidianWikiLink(raw: string): ParsedObsidianWikiLink | null { - const trimmed = raw.trim(); - if (!trimmed) return null; - - const separator = trimmed.indexOf("|"); - const linktext = (separator === -1 ? trimmed : trimmed.slice(0, separator)).trim(); - const display = separator === -1 ? "" : trimmed.slice(separator + 1).trim(); - if (!linktext) return null; - - const parsed = parseLinktext(linktext); - const target = parsed.path.trim(); - const subpath = parsed.subpath.trim(); - if (!target) return null; - return { target, subpath, display }; -} diff --git a/tests/scripts/eslint-config.test.ts b/tests/scripts/eslint-config.test.ts index d0277878..f25ab8cb 100644 --- a/tests/scripts/eslint-config.test.ts +++ b/tests/scripts/eslint-config.test.ts @@ -422,9 +422,22 @@ export const status = signal("idle"); expect(messages).toContain("no-restricted-syntax"); }); - it("allows event wiring but not DOM writes in event-only bridge files", async () => { + it("does not treat AbortSignal event wiring as imperative DOM", async () => { const messages = await lintSource( - "src/shared/lifecycle/abortable.ts", + "src/app-server/services/abortable-operation.ts", + ` +export function attach(signal: AbortSignal): void { + signal.addEventListener("abort", () => undefined); +} +`, + ); + + expect(messages).not.toContain("codex-panel/no-imperative-dom"); + }); + + it("still reports DOM writes beside AbortSignal event wiring", async () => { + const messages = await lintSource( + "src/app-server/services/abortable-operation.ts", ` export function attach(signal: AbortSignal, element: HTMLElement): void { signal.addEventListener("abort", () => undefined); @@ -436,12 +449,12 @@ export function attach(signal: AbortSignal, element: HTMLElement): void { expect(messages.filter((message) => message === "codex-panel/no-imperative-dom")).toHaveLength(1); }); - it("allows DOM event wiring in the shared UI event bridge", async () => { + it("does not treat generic EventTarget helpers as DOM event wiring", async () => { const messages = await lintSource( "src/shared/ui/dom-events.ts", ` -export function attach(element: HTMLElement): void { - element.addEventListener("click", () => undefined); +export function attach(target: EventTarget): void { + target.addEventListener("click", () => undefined); } `, );