From f86743490d68401330495ec23b08917a555ae531 Mon Sep 17 00:00:00 2001 From: murashit Date: Tue, 9 Jun 2026 21:25:57 +0900 Subject: [PATCH] Keep app-server scheduling out of DOM bridge lint allowlist --- eslint.config.mjs | 59 +++++++++++++------ src/app-server/structured-ephemeral-turn.ts | 35 ++++++----- src/shared/lifecycle/abortable.ts | 17 ++++++ .../structured-ephemeral-turn.test.ts | 18 ++++++ 4 files changed, 96 insertions(+), 33 deletions(-) create mode 100644 src/shared/lifecycle/abortable.ts diff --git a/eslint.config.mjs b/eslint.config.mjs index 72828019..52787e2e 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -9,22 +9,6 @@ const typeScriptFiles = ["src/**/*.{ts,tsx}", "tests/**/*.{ts,tsx}"]; const nodeJavaScriptFiles = ["*.mjs", "scripts/**/*.mjs"]; const typeScriptConfigFiles = ["*.config.ts"]; const lintedTypeScriptFiles = [...typeScriptFiles, ...typeScriptConfigFiles]; -const imperativeDomRestrictions = [ - { - selector: - "CallExpression[callee.property.name=/^(createEl|createDiv|createSpan|appendChild|replaceChildren|insertBefore|removeChild|append|prepend|before|after|replaceWith|remove|insertAdjacentHTML|insertAdjacentElement|insertAdjacentText|setAttr|empty)$/]", - message: "Keep imperative DOM writes in an explicit bridge module or Obsidian-owned UI boundary.", - }, - { - selector: - "AssignmentExpression[left.type='MemberExpression'][left.property.name=/^(innerHTML|outerHTML|textContent|value|checked|onclick|ondblclick|oninput|onchange|onkeydown|onkeyup|onmousedown|onmouseup|onmousemove|onpointerdown|onpointerup|onblur|onfocus|onselect|onscroll)$/]", - message: "Keep imperative DOM writes in an explicit bridge module or Obsidian-owned UI boundary.", - }, - { - selector: "CallExpression[callee.property.name=/^(addEventListener|removeEventListener)$/]", - message: "Keep imperative DOM event wiring in an explicit bridge module or Obsidian-owned UI boundary.", - }, -]; const preactFormRestrictions = [ { selector: "JSXAttribute[name.name=/^(defaultValue|defaultChecked)$/]", @@ -43,6 +27,25 @@ const unsafeIteratorRestrictions = [ message: "Avoid reading iterator.next().value directly; use for...of or inspect the typed IteratorResult first.", }, ]; +const imperativeDomWriteRestrictions = [ + { + selector: + "CallExpression[callee.property.name=/^(createEl|createDiv|createSpan|appendChild|replaceChildren|insertBefore|removeChild|append|prepend|before|after|replaceWith|remove|insertAdjacentHTML|insertAdjacentElement|insertAdjacentText|setAttr|empty)$/]", + message: "Keep imperative DOM writes in an explicit bridge module or Obsidian-owned UI boundary.", + }, + { + selector: + "AssignmentExpression[left.type='MemberExpression'][left.property.name=/^(innerHTML|outerHTML|textContent|value|checked|onclick|ondblclick|oninput|onchange|onkeydown|onkeyup|onmousedown|onmouseup|onmousemove|onpointerdown|onpointerup|onblur|onfocus|onselect|onscroll)$/]", + message: "Keep imperative DOM writes in an explicit bridge module or Obsidian-owned UI boundary.", + }, +]; +const imperativeDomEventRestrictions = [ + { + selector: "CallExpression[callee.property.name=/^(addEventListener|removeEventListener)$/]", + message: "Keep imperative DOM event wiring in an explicit bridge module or Obsidian-owned UI boundary.", + }, +]; +const imperativeDomRestrictions = [...imperativeDomWriteRestrictions, ...imperativeDomEventRestrictions]; const pureChatModelRestrictions = [ { selector: "CallExpression[callee.object.name='Date'][callee.property.name='now']", @@ -79,7 +82,6 @@ const chatImperativeDomBridgeFiles = [ "src/features/chat/ui/turn-diff.tsx", ]; const nonChatImperativeDomBridgeFiles = [ - "src/app-server/structured-ephemeral-turn.ts", "src/features/selection-rewrite/popover.tsx", "src/features/selection-rewrite/runner.ts", "src/features/thread-picker/modal.ts", @@ -92,6 +94,7 @@ const nonChatImperativeDomBridgeFiles = [ "src/shared/ui/textarea-caret.ts", "src/shared/ui/ui-root.tsx", ]; +const nonUiEventListenerFiles = ["src/shared/lifecycle/abortable.ts"]; const codexPanelEslintPlugin = { rules: { "no-self-referential-initializer-callback": { @@ -259,9 +262,11 @@ export default defineConfig([ HTMLTextAreaElement: "readonly", KeyboardEvent: "readonly", NodeJS: "readonly", + clearTimeout: "readonly", console: "readonly", document: "readonly", requestAnimationFrame: "readonly", + setTimeout: "readonly", }, }, rules: { @@ -314,7 +319,7 @@ export default defineConfig([ }, { files: ["src/**/*.{ts,tsx}"], - ignores: ["src/features/chat/**/*.{ts,tsx}", ...nonChatImperativeDomBridgeFiles], + ignores: ["src/features/chat/**/*.{ts,tsx}", ...nonChatImperativeDomBridgeFiles, ...nonUiEventListenerFiles], rules: { "no-restricted-syntax": [ "error", @@ -362,6 +367,18 @@ export default defineConfig([ ], }, }, + { + files: nonUiEventListenerFiles, + rules: { + "no-restricted-syntax": [ + "error", + ...removedChatStateEscapeHatchRestrictions, + ...unsafeIteratorRestrictions, + ...imperativeDomWriteRestrictions, + ...preactFormRestrictions, + ], + }, + }, { files: ["src/features/chat/chat-state.ts", "src/features/chat/display/**/*.{ts,tsx}"], rules: { @@ -404,5 +421,11 @@ export default defineConfig([ ], }, }, + { + files: ["src/app-server/**/*.{ts,tsx}"], + rules: { + "obsidianmd/prefer-window-timers": "off", + }, + }, eslintConfigPrettier, ]); diff --git a/src/app-server/structured-ephemeral-turn.ts b/src/app-server/structured-ephemeral-turn.ts index 69d8ad54..103efea8 100644 --- a/src/app-server/structured-ephemeral-turn.ts +++ b/src/app-server/structured-ephemeral-turn.ts @@ -4,6 +4,7 @@ import { structuredTurnRunMatches, transitionStructuredTurnRunLifecycle, } from "./structured-turn-run-lifecycle"; +import { abortablePromise, throwIfAbortSignalAborted } from "../shared/lifecycle/abortable"; import type { InitializeResponse } from "../generated/app-server/InitializeResponse"; import type { RequestId } from "../generated/app-server/RequestId"; import type { ReasoningEffort } from "../generated/app-server/ReasoningEffort"; @@ -24,6 +25,18 @@ interface StructuredTurnRuntimeOverride { type StructuredTurnProgressEvent = { type: "agent-message-delta"; delta: string } | { type: "reasoning-activity" }; +interface StructuredEphemeralTurnTimers { + setTimeout(callback: () => void, delayMs: number): unknown; + clearTimeout(timer: unknown): void; +} + +const DEFAULT_STRUCTURED_EPHEMERAL_TURN_TIMERS: StructuredEphemeralTurnTimers = { + setTimeout: (callback, delayMs) => setTimeout(callback, delayMs), + clearTimeout: (timer) => { + clearTimeout(timer as ReturnType); + }, +}; + export interface StructuredEphemeralTurnClient { connect(): Promise; disconnect(): void; @@ -68,18 +81,20 @@ export interface RunStructuredEphemeralTurnOptions { signal?: AbortSignal | undefined; onProgress?: (event: StructuredTurnProgressEvent) => void; clientFactory?: StructuredEphemeralTurnClientFactory | undefined; + timers?: StructuredEphemeralTurnTimers | undefined; } export async function runStructuredEphemeralTurn(options: RunStructuredEphemeralTurnOptions): Promise { throwIfAborted(options.signal, options.abortMessage); let state = createStructuredEphemeralTurnState(); - let timeout: number | undefined; + const timers = options.timers ?? DEFAULT_STRUCTURED_EPHEMERAL_TURN_TIMERS; + let timeout: unknown; let rejectCompletedTurn: ((error: Error) => void) | null = null; let handleNotification: (notification: ServerNotification) => void = () => undefined; const completedTurn = new Promise((resolve, reject) => { rejectCompletedTurn = reject; - timeout = window.setTimeout(() => { + timeout = timers.setTimeout(() => { if (state.lifecycle.kind === "completed") return; state = completeStructuredEphemeralTurnState(state); reject(new Error(options.timedOutMessage)); @@ -149,7 +164,7 @@ export async function runStructuredEphemeralTurn(options: RunStructuredEphemeral : await abortable(completedTurn, options.signal, options.abortMessage); } finally { state = completeStructuredEphemeralTurnState(state); - if (timeout !== undefined) window.clearTimeout(timeout); + if (timeout !== undefined) timers.clearTimeout(timeout); client.disconnect(); } } @@ -216,21 +231,11 @@ function turnWithCollectedItems(turn: Turn, completedItems: readonly ThreadItem[ } function throwIfAborted(signal: AbortSignal | undefined, message: string | undefined): void { - if (signal?.aborted) throw structuredEphemeralTurnAbortError(message); + throwIfAbortSignalAborted(signal, () => structuredEphemeralTurnAbortError(message)); } function abortable(promise: Promise, signal: AbortSignal | undefined, message: string | undefined): Promise { - if (!signal) return promise; - throwIfAborted(signal, message); - return new Promise((resolve, reject) => { - const onAbort = (): void => { - reject(structuredEphemeralTurnAbortError(message)); - }; - signal.addEventListener("abort", onAbort, { once: true }); - promise.then(resolve, reject).finally(() => { - signal.removeEventListener("abort", onAbort); - }); - }); + return abortablePromise(promise, signal, () => structuredEphemeralTurnAbortError(message)); } function structuredEphemeralTurnAbortError(message: string | undefined): Error { diff --git a/src/shared/lifecycle/abortable.ts b/src/shared/lifecycle/abortable.ts new file mode 100644 index 00000000..beefd089 --- /dev/null +++ b/src/shared/lifecycle/abortable.ts @@ -0,0 +1,17 @@ +export function throwIfAbortSignalAborted(signal: AbortSignal | undefined, abortError: () => Error): void { + if (signal?.aborted) throw abortError(); +} + +export function abortablePromise(promise: Promise, signal: AbortSignal | undefined, abortError: () => Error): Promise { + if (!signal) return promise; + throwIfAbortSignalAborted(signal, abortError); + return new Promise((resolve, reject) => { + const onAbort = (): void => { + reject(abortError()); + }; + signal.addEventListener("abort", onAbort, { once: true }); + promise.then(resolve, reject).finally(() => { + signal.removeEventListener("abort", onAbort); + }); + }); +} diff --git a/tests/app-server/structured-ephemeral-turn.test.ts b/tests/app-server/structured-ephemeral-turn.test.ts index a3278183..91377187 100644 --- a/tests/app-server/structured-ephemeral-turn.test.ts +++ b/tests/app-server/structured-ephemeral-turn.test.ts @@ -76,6 +76,24 @@ describe("runStructuredEphemeralTurn", () => { expect(remove).toHaveBeenCalledTimes(add.mock.calls.length); }); + it("uses injected timers for structured turn timeout cleanup", async () => { + const timers = { + setTimeout: vi.fn((_callback: () => void, _delayMs: number) => 123), + clearTimeout: vi.fn(), + }; + const { clientFactory } = fakeStructuredTurnClientFactory((fake) => { + fake.startStructuredTurnImpl = async () => ({ turn: turn([agentMessage("answer", '{"ok":true}')]) }); + }); + + await runStructuredEphemeralTurn({ + ...runOptions(clientFactory), + timers, + }); + + expect(timers.setTimeout).toHaveBeenCalledWith(expect.any(Function), 10_000); + expect(timers.clearTimeout).toHaveBeenCalledWith(123); + }); + it("rejects server requests with the configured message", async () => { const { clientFactory, client } = fakeStructuredTurnClientFactory((fake) => { fake.connectImpl = async () => {