Move single-use helpers to owning boundaries

This commit is contained in:
murashit 2026-06-21 19:23:06 +09:00
parent 4582104d08
commit 2de0dc8399
9 changed files with 75 additions and 100 deletions

View file

@ -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),

View file

@ -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) {

View file

@ -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<T>(promise: Promise<T>, signal: AbortSignal | undefined, abortError: () => Error): Promise<T> {
export function abortableOperation<T>(promise: Promise<T>, signal: AbortSignal | undefined, abortError: () => Error): Promise<T> {
if (!signal) return promise;
throwIfAbortSignalAborted(signal, abortError);
throwIfSignalAborted(signal, abortError);
return new Promise<T>((resolve, reject) => {
const onAbort = (): void => {
reject(abortError());

View file

@ -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<T>(promise: Promise<T>, signal: AbortSignal | undefined, abortError: () => Error): Promise<T> {
return abortablePromise(promise, signal, abortError);
return abortableOperation(promise, signal, abortError);
}
function ephemeralStructuredTurnAbortError(message: string | undefined): Error {

View file

@ -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 };
}

View file

@ -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 };
}

View file

@ -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<string, SkillMetadata> {

View file

@ -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 };
}

View file

@ -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);
}
`,
);