Address Obsidian review warnings

This commit is contained in:
murashit 2026-05-13 08:39:00 +09:00
parent c77dab19cd
commit 3e7e09c83c
14 changed files with 110 additions and 34 deletions

View file

@ -337,7 +337,7 @@ export class AppServerClient {
private request<M extends TypedClientRequestMethod>(method: M, params: ClientRequestParams<M>): Promise<ClientResponseByMethod[M]> {
const id = this.nextId++;
const promise = new Promise<ClientResponseByMethod[M]>((resolve, reject) => {
const timeout = globalThis.setTimeout(() => {
const timeout = window.setTimeout(() => {
this.pending.delete(id);
reject(new Error(`Codex app-server request timed out: ${method}`));
}, this.requestTimeoutMs);
@ -354,7 +354,7 @@ export class AppServerClient {
} catch (error) {
const pending = this.pending.get(id);
if (pending) {
globalThis.clearTimeout(pending.timeout);
window.clearTimeout(pending.timeout);
this.pending.delete(id);
}
throw error;
@ -396,7 +396,7 @@ export class AppServerClient {
this.handlers.onLog(`Orphan app-server response: ${JSON.stringify(message)}`);
return;
}
globalThis.clearTimeout(pending.timeout);
window.clearTimeout(pending.timeout);
this.pending.delete(message.id);
if ("error" in message && message.error) {
pending.reject(new Error(message.error.message || "Codex app-server request failed."));
@ -413,7 +413,7 @@ export class AppServerClient {
private rejectAll(error: Error): void {
for (const pending of this.pending.values()) {
globalThis.clearTimeout(pending.timeout);
window.clearTimeout(pending.timeout);
pending.reject(error);
}
this.pending.clear();

View file

@ -26,5 +26,5 @@ export interface PendingRequest {
method: ClientRequestMethod;
reject: (reason: Error) => void;
resolve: (value: unknown) => void;
timeout: ReturnType<typeof setTimeout>;
timeout: ReturnType<Window["setTimeout"]>;
}

5
src/electron.d.ts vendored Normal file
View file

@ -0,0 +1,5 @@
declare module "electron" {
export const shell: {
openPath(path: string): Promise<string>;
};
}

View file

@ -71,10 +71,6 @@ import { renderPendingRequestMessage } from "./view/pending-request-message";
import { bottomScrollTop, captureScrollAnchor, isNearScrollBottom, restoreScrollAnchor } from "./view/scroll";
import { renderToolbar, toolbarSignature, type ToolbarChoice, type ToolbarViewModel } from "./view/toolbar";
interface ElectronShell {
openPath(path: string): Promise<string>;
}
export default class CodexPanelPlugin extends Plugin {
settings: CodexPanelSettings = DEFAULT_SETTINGS;
vaultPath = "";
@ -90,13 +86,13 @@ export default class CodexPanelPlugin extends Plugin {
});
this.addCommand({
id: "open-codex-panel",
id: "open-panel",
name: "Open panel",
callback: () => void this.activateView(),
});
this.addCommand({
id: "new-codex-chat",
id: "new-chat",
name: "New chat",
callback: async () => {
const view = await this.activateView();
@ -134,7 +130,7 @@ export default class CodexPanelPlugin extends Plugin {
}
async loadSettings(): Promise<void> {
const data = await this.loadData();
const data: unknown = await this.loadData();
this.settings = normalizeSettings(data);
if (!settingsMatchNormalizedData(data, this.settings)) {
await this.saveSettings();
@ -255,7 +251,7 @@ class CodexPanelView extends ItemView {
async onOpen(): Promise<void> {
this.registerNoteIndexInvalidation();
this.registerDomEvent(document, "pointerdown", (event) => this.closeToolbarPanelOnOutsidePointer(event));
this.registerDomEvent(activeDocument, "pointerdown", (event) => this.closeToolbarPanelOnOutsidePointer(event));
this.render();
await this.ensureConnected();
}
@ -691,7 +687,7 @@ class CodexPanelView extends ItemView {
}
try {
const shell = (require("electron") as { shell?: ElectronShell }).shell;
const { shell } = await import("electron");
if (!shell) throw new Error("Electron shell is not available.");
const vaultCodexPath = join(this.plugin.vaultPath, ".codex");
@ -910,7 +906,7 @@ class CodexPanelView extends ItemView {
return [
`Model: ${currentModel(snapshot, config) ?? "(from default)"}`,
`Override: ${runtimeOverrideLabel(this.state.requestedModel)}`,
`Provider: ${config.model_provider ?? "(from default)"}`,
`Provider: ${statusValue(config.model_provider, "(from default)")}`,
`Effort: ${currentReasoningEffort(snapshot, config) ?? "(from default)"}`,
`Mode: ${this.collaborationModeLabel()}`,
`Service tier: ${serviceTierLabel(snapshot, config)}`,
@ -1035,7 +1031,7 @@ class CodexPanelView extends ItemView {
workspaceRoot: this.state.activeThreadCwd ?? this.plugin.vaultPath,
openDetails: this.state.openDetails,
onDetailsToggle: () => {
requestAnimationFrame(() => {
window.requestAnimationFrame(() => {
this.state.messagesPinnedToBottom = isNearScrollBottom(messagesEl);
});
},
@ -1075,7 +1071,7 @@ class CodexPanelView extends ItemView {
}
}
requestAnimationFrame(() => {
window.requestAnimationFrame(() => {
if (shouldScrollToBottom) {
messagesEl.scrollTop = bottomScrollTop(messagesEl);
} else {
@ -1121,7 +1117,7 @@ class CodexPanelView extends ItemView {
if (!this.state.messagesPinnedToBottom) return;
const messagesEl = parent.closest<HTMLElement>(".codex-panel__messages");
if (!messagesEl) return;
requestAnimationFrame(() => {
window.requestAnimationFrame(() => {
if (!this.state.messagesPinnedToBottom) return;
messagesEl.scrollTop = bottomScrollTop(messagesEl);
this.state.messagesPinnedToBottom = isNearScrollBottom(messagesEl);
@ -1323,3 +1319,18 @@ class CodexPanelView extends ItemView {
this.registerEvent(this.app.vault.on("modify", invalidate));
}
}
function statusValue(value: unknown, fallback: string): string {
if (typeof value === "string") return value;
if (typeof value === "number" || typeof value === "boolean" || typeof value === "bigint") return String(value);
if (value === null || value === undefined) return fallback;
return jsonPreview(value, fallback);
}
function jsonPreview(value: unknown, fallback: string): string {
try {
return JSON.stringify(value) ?? fallback;
} catch {
return fallback;
}
}

View file

@ -1,15 +1,17 @@
export type ClassifiedAppServerLog = { kind: "plain"; text: string } | { kind: "error"; text: string } | null;
const ANSI_PATTERN = new RegExp("\\u001B\\[[0-?]*[ -/]*[@-~]", "g");
export function classifyAppServerLog(message: string): ClassifiedAppServerLog {
const normalized = stripAnsi(message).trimEnd();
if (!normalized || isMcpTokenRefreshLog(normalized)) return null;
const parsed = parseAppServerLog(normalized);
if (!parsed) return null;
const level = String(parsed.level ?? "").toUpperCase();
const level = logString(parsed.level).toUpperCase();
const fields = parsed.fields && typeof parsed.fields === "object" ? (parsed.fields as Record<string, unknown>) : {};
const text = stripAnsi(String(fields.message ?? normalized));
const target = String(parsed.target ?? "");
const text = stripAnsi(logString(fields.message, normalized));
const target = logString(parsed.target);
if (target.includes("rmcp::transport::worker") && isMcpTokenRefreshLog(text)) return null;
if (target.includes("codex_core::tools::router") && text.includes("apply_patch verification failed")) return null;
if (level === "ERROR") return { kind: "error", text };
@ -30,5 +32,16 @@ function isMcpTokenRefreshLog(message: string): boolean {
}
function stripAnsi(message: string): string {
return message.replace(/\u001b\[[0-?]*[ -/]*[@-~]/g, "");
return message.replace(ANSI_PATTERN, "");
}
function logString(value: unknown, fallback = ""): string {
if (typeof value === "string") return value;
if (typeof value === "number" || typeof value === "boolean" || typeof value === "bigint") return String(value);
if (value === null || value === undefined) return fallback;
try {
return JSON.stringify(value) ?? fallback;
} catch {
return fallback;
}
}

View file

@ -70,14 +70,14 @@ export async function generateThreadTitleWithCodex(
let namingThreadId: string | null = null;
let expectedTurnId: string | null = null;
let completed = false;
let timeout: ReturnType<typeof setTimeout> | null = null;
let timeout: ReturnType<Window["setTimeout"]> | null = null;
let rejectCompletedTurn: ((error: Error) => void) | null = null;
let handleNamingNotification: (notification: ServerNotification) => void = () => undefined;
const completedItems: ThreadItem[] = [];
const completedTurn = new Promise<Turn>((resolve, reject) => {
rejectCompletedTurn = reject;
timeout = globalThis.setTimeout(() => {
timeout = window.setTimeout(() => {
if (completed) return;
completed = true;
reject(new Error("Timed out while generating a Codex thread title."));
@ -133,7 +133,7 @@ export async function generateThreadTitleWithCodex(
return titleFromNamingTurn(turn);
} finally {
completed = true;
if (timeout) globalThis.clearTimeout(timeout);
if (timeout) window.clearTimeout(timeout);
client.disconnect();
}
}

View file

@ -105,10 +105,12 @@ export function syncComposerHeight(composer: HTMLTextAreaElement | null): void {
const style = getComputedStyle(composer);
const minHeight = parseCssPixels(style.minHeight, 76);
const maxHeight = composerMaxHeight(style.maxHeight);
composer.style.height = "auto";
composer.setCssProps({ height: "auto" });
const nextHeight = Math.min(Math.max(composer.scrollHeight, minHeight), maxHeight);
composer.style.height = `${nextHeight}px`;
composer.style.overflowY = composer.scrollHeight > maxHeight ? "auto" : "hidden";
composer.setCssProps({
height: `${nextHeight}px`,
"overflow-y": composer.scrollHeight > maxHeight ? "auto" : "hidden",
});
}
function parseCssPixels(value: string, fallback: number): number {

View file

@ -8,11 +8,12 @@ export function shortSignature(value: string): string {
export function renderTextWithWikiLinks(parent: HTMLElement, text: string, openLink: (target: string) => void): void {
const wikilinkPattern = /\[\[([^\]\n]+?)\]\]/g;
const doc = parent.ownerDocument;
let lastIndex = 0;
for (const match of text.matchAll(wikilinkPattern)) {
const index = match.index ?? 0;
if (index > lastIndex) {
parent.appendChild(document.createTextNode(text.slice(lastIndex, index)));
parent.appendChild(doc.createTextNode(text.slice(lastIndex, index)));
}
const rawLink = match[1] ?? "";
@ -21,7 +22,7 @@ export function renderTextWithWikiLinks(parent: HTMLElement, text: string, openL
const label = (separator === -1 ? rawLink : rawLink.slice(separator + 1)).trim() || target;
if (target.length === 0) {
parent.appendChild(document.createTextNode(match[0]));
parent.appendChild(doc.createTextNode(match[0]));
} else {
const link = parent.createEl("a", {
cls: "internal-link codex-panel__wikilink",
@ -40,6 +41,6 @@ export function renderTextWithWikiLinks(parent: HTMLElement, text: string, openL
}
if (lastIndex < text.length) {
parent.appendChild(document.createTextNode(text.slice(lastIndex)));
parent.appendChild(doc.createTextNode(text.slice(lastIndex)));
}
}

View file

@ -345,7 +345,7 @@ function renderThreadRenameRow(parent: HTMLElement, thread: ToolbarThreadRow, ac
}
};
window.setTimeout(() => {
if (document.activeElement !== input) {
if (input.ownerDocument.activeElement !== input) {
input.focus();
input.select();
}

View file

@ -1,4 +1,4 @@
import { describe, expect, it } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { AppServerClient } from "../src/app-server/client";
import type { AppServerTransport, AppServerTransportHandlers } from "../src/app-server/transport";
@ -57,6 +57,13 @@ async function connectedClient(): Promise<{ client: AppServerClient; transport:
}
describe("AppServerClient", () => {
beforeEach(() => {
vi.stubGlobal("window", {
clearTimeout,
setTimeout,
});
});
it("routes responses, notifications, and server requests", async () => {
let transport: FakeTransport;
const getTransport = () => transport;

View file

@ -27,6 +27,21 @@ describe("app-server log classification", () => {
});
});
it("renders structured JSON log fields without object stringification", () => {
expect(
classifyAppServerLog(
JSON.stringify({
level: "ERROR",
fields: { message: { error: "boom" } },
target: { crate: "codex" },
}),
),
).toEqual({
kind: "error",
text: '{"error":"boom"}',
});
});
it("suppresses structured apply_patch router verification logs", () => {
expect(
classifyAppServerLog(

View file

@ -1,4 +1,4 @@
import { describe, expect, it, vi } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { AppServerClient } from "../src/app-server/client";
import { ConnectionManager, StaleConnectionError } from "../src/app-server/connection-manager";
@ -38,6 +38,13 @@ class SilentTransport implements AppServerTransport {
}
describe("ConnectionManager", () => {
beforeEach(() => {
vi.stubGlobal("window", {
clearTimeout,
setTimeout,
});
});
it("disconnects clients whose initialization fails", async () => {
let transport!: SilentTransport;
const manager = new ConnectionManager(

View file

@ -11,6 +11,7 @@ declare global {
createEl<K extends keyof HTMLElementTagNameMap>(tag: K, options?: ElementOptions): HTMLElementTagNameMap[K];
createSpan(options?: ElementOptions): HTMLSpanElement;
empty(): void;
setCssProps(props: Record<string, string>): void;
setAttr(name: string, value: string): void;
}
}
@ -236,6 +237,14 @@ function ensureElementHelpers(): void {
};
}
if (!HTMLElement.prototype.setCssProps) {
HTMLElement.prototype.setCssProps = function setCssProps(props: Record<string, string>): void {
for (const [key, value] of Object.entries(props)) {
this.style.setProperty(key, value);
}
};
}
if (!HTMLElement.prototype.createEl) {
HTMLElement.prototype.createEl = function createEl<K extends keyof HTMLElementTagNameMap>(
tag: K,

View file

@ -28,6 +28,7 @@ declare global {
createSpan(options?: { cls?: string; text?: string; attr?: Record<string, string> }): HTMLSpanElement;
empty(): void;
hide(): void;
setCssProps(props: Record<string, string>): void;
setAttr(name: string, value: string): void;
show(): void;
}
@ -75,6 +76,11 @@ beforeEach(() => {
HTMLElement.prototype.setAttr = function setAttr(this: HTMLElement, name: string, value: string): void {
this.setAttribute(name, value);
};
HTMLElement.prototype.setCssProps = function setCssProps(this: HTMLElement, props: Record<string, string>): void {
for (const [key, value] of Object.entries(props)) {
this.style.setProperty(key, value);
}
};
HTMLElement.prototype.scrollIntoView = vi.fn();
globalThis.createDiv = ((options: { cls?: string; text?: string; attr?: Record<string, string> } = {}) =>
document.body.createDiv(options)) as typeof globalThis.createDiv;