fix(coding-agent): report settings diagnostic paths

This commit is contained in:
Vegard Stikbakke
2026-08-19 13:07:53 +02:00
parent 1e1a6e27be
commit 913bcf3391
3 changed files with 75 additions and 12 deletions
@@ -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<string>();
return diagnostics.filter((diagnostic) => {
const key = `${diagnostic.type}\0${diagnostic.message}`;
if (seen.has(key)) return false;
seen.add(key);
return true;
});
}
+3 -12
View File
@@ -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<string | undefined> {
});
}
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}`,
@@ -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" },
]);
});
});