From 913bcf3391be680a41fd6cb9acfe5d0fbfb3f7d9 Mon Sep 17 00:00:00 2001 From: Vegard Stikbakke Date: Wed, 19 Aug 2026 12:55:36 +0200 Subject: [PATCH] fix(coding-agent): report settings diagnostic paths --- .../src/core/settings-diagnostics.ts | 25 ++++++++++ packages/coding-agent/src/main.ts | 15 ++---- .../test/settings-diagnostics.test.ts | 47 +++++++++++++++++++ 3 files changed, 75 insertions(+), 12 deletions(-) create mode 100644 packages/coding-agent/src/core/settings-diagnostics.ts create mode 100644 packages/coding-agent/test/settings-diagnostics.test.ts diff --git a/packages/coding-agent/src/core/settings-diagnostics.ts b/packages/coding-agent/src/core/settings-diagnostics.ts new file mode 100644 index 000000000..8dfec3899 --- /dev/null +++ b/packages/coding-agent/src/core/settings-diagnostics.ts @@ -0,0 +1,25 @@ +import type { AgentSessionRuntimeDiagnostic } from "./agent-session-services.ts"; +import type { SettingsManager } from "./settings-manager.ts"; + +export function collectSettingsDiagnostics(settingsManager: SettingsManager): AgentSessionRuntimeDiagnostic[] { + return settingsManager.drainErrors().map(({ scope, path, error }) => ({ + type: "warning", + message: path ? `Invalid settings file ${path}: ${error.message}` : `Invalid ${scope} settings: ${error.message}`, + })); +} + +/** + * Remove duplicate type/message diagnostics while preserving their first occurrence. + * Startup and runtime settings managers can report the same file error. + */ +export function deduplicateDiagnostics( + diagnostics: readonly AgentSessionRuntimeDiagnostic[], +): AgentSessionRuntimeDiagnostic[] { + const seen = new Set(); + return diagnostics.filter((diagnostic) => { + const key = `${diagnostic.type}\0${diagnostic.message}`; + if (seen.has(key)) return false; + seen.add(key); + return true; + }); +} diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 85d952359..f9f626a9e 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -56,6 +56,7 @@ import { type SessionCwdIssue, } from "./core/session-cwd.ts"; import { assertValidSessionId, SessionManager } from "./core/session-manager.ts"; +import { collectSettingsDiagnostics } from "./core/settings-diagnostics.ts"; import { SettingsManager } from "./core/settings-manager.ts"; import { printTimings, resetTimings, time } from "./core/timings.ts"; import { hasTrustRequiringProjectResources, ProjectTrustStore } from "./core/trust-manager.ts"; @@ -92,16 +93,6 @@ async function readPipedStdin(): Promise { }); } -function collectSettingsDiagnostics( - settingsManager: SettingsManager, - context: string, -): AgentSessionRuntimeDiagnostic[] { - return settingsManager.drainErrors().map(({ scope, error }) => ({ - type: "warning", - message: `(${context}, ${scope} settings) ${error.message}`, - })); -} - function reportDiagnostics(diagnostics: readonly AgentSessionRuntimeDiagnostic[]): void { for (const diagnostic of diagnostics) { const color = diagnostic.type === "error" ? chalk.red : diagnostic.type === "warning" ? chalk.yellow : chalk.dim; @@ -656,7 +647,7 @@ export async function main(args: string[], options?: MainOptions) { time("runMigrations"); const startupSettingsManager = SettingsManager.create(cwd, agentDir); - reportDiagnostics(collectSettingsDiagnostics(startupSettingsManager, "startup session lookup")); + reportDiagnostics(collectSettingsDiagnostics(startupSettingsManager)); // Experimental first-time setup: theme choice and analytics opt-in. // Runs before any runtime services are created so the chosen settings apply everywhere. @@ -784,7 +775,7 @@ export async function main(args: string[], options?: MainOptions) { const diagnostics: AgentSessionRuntimeDiagnostic[] = [ ...projectTrustDiagnostics, ...services.diagnostics, - ...collectSettingsDiagnostics(settingsManager, "runtime creation"), + ...collectSettingsDiagnostics(settingsManager), ...resourceLoader.getExtensions().errors.map(({ path, error }) => ({ type: "error" as const, message: `Failed to load extension "${path}": ${error}`, diff --git a/packages/coding-agent/test/settings-diagnostics.test.ts b/packages/coding-agent/test/settings-diagnostics.test.ts new file mode 100644 index 000000000..9bda0fd6a --- /dev/null +++ b/packages/coding-agent/test/settings-diagnostics.test.ts @@ -0,0 +1,47 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { collectSettingsDiagnostics, deduplicateDiagnostics } from "../src/core/settings-diagnostics.ts"; +import { SettingsManager, type SettingsStorage } from "../src/core/settings-manager.ts"; + +describe("settings diagnostics", () => { + it("includes the settings file path for file-backed storage", () => { + const tempDir = mkdtempSync(join(tmpdir(), "pi-settings-diagnostics-")); + const agentDir = join(tempDir, "agent"); + const settingsPath = join(agentDir, "settings.json"); + mkdirSync(agentDir); + writeFileSync(settingsPath, "{"); + + try { + const diagnostics = collectSettingsDiagnostics(SettingsManager.create(tempDir, agentDir)); + + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0]?.type).toBe("warning"); + expect(diagnostics[0]?.message).toContain(`Invalid settings file ${settingsPath}:`); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it("falls back to the settings scope for storage without file paths", () => { + const storage: SettingsStorage = { + withLock(scope, fn) { + if (scope === "global") throw new Error("backend failed"); + fn(undefined); + }, + }; + const diagnostics = collectSettingsDiagnostics(SettingsManager.fromStorage(storage)); + + expect(diagnostics).toEqual([{ type: "warning", message: "Invalid global settings: backend failed" }]); + }); + + it("deduplicates diagnostics by type and message", () => { + const warning = { type: "warning" as const, message: "Invalid settings file /tmp/settings.json" }; + + expect(deduplicateDiagnostics([warning, warning, { ...warning, type: "error" }])).toEqual([ + warning, + { ...warning, type: "error" }, + ]); + }); +});