Skip to content

Commit 575545f

Browse files
committed
fix: FB-002 distinguish ENOENT from other read errors, FB-003 catch save failures
FB-002: readSettingsSource now distinguishes ENOENT (file genuinely missing -> exists:false) from other read errors like EACCES/EISDIR (exists:true, invalid:true). The resolveHandoffResumeBehavior function already handles invalid sources with warnings and fallback to wait. FB-003: The async IIFE in createAgenticodingSettingsComponent's SettingsList change callback now wraps the save/rebuild sequence in try/catch. On failure it calls notify() with an error-level message instead of silently dropping the rejection as an unhandled promise. Regression tests: - non-ENOENT read error test (FB-002): makes global settings file unreadable via chmod 000, asserts invalid:true + warning + wait - write failure test (FB-003): blocks the .pi/agent directory with a file, asserts writeGlobalHandoffResumeBehavior rejects with EEXIST
1 parent 160b860 commit 575545f

2 files changed

Lines changed: 78 additions & 10 deletions

File tree

agenticoding.test.ts

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import test, { after } from "node:test";
22
import assert from "node:assert/strict";
3-
import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
3+
import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
44
import { tmpdir } from "node:os";
55
import { dirname, join } from "node:path";
66
import type { Theme } from "@earendil-works/pi-coding-agent";
@@ -26,6 +26,9 @@ import {
2626
MANUAL_AGENTICODING_SETTINGS_INSTRUCTIONS,
2727
buildAgenticodingSettingsModel,
2828
getAgenticodingSettingsDisplayLines,
29+
readHandoffSettingsState,
30+
resolveHandoffResumeBehavior,
31+
writeGlobalHandoffResumeBehavior,
2932
} from "./settings.js";
3033

3134
// Safety net: reset module-level mutable state after all tests.
@@ -501,6 +504,35 @@ test("handoff resume setting invalid JSON falls back to wait with diagnostic", a
501504
assert.match(projectResult.notifications[0].message, /falling back to wait/);
502505
});
503506

507+
test("handoff resume setting non-ENOENT read errors are treated as invalid source with warning", async () => {
508+
await withIsolatedSettings(async ({ home, cwd }) => {
509+
const globalPath = join(home, ".pi", "agent", "settings.json");
510+
await writeSettingsFile(globalPath, {});
511+
await chmod(globalPath, 0o000);
512+
513+
try {
514+
const state = await readHandoffSettingsState(cwd);
515+
assert.equal(state.global.invalid, true);
516+
assert.equal(state.global.exists, true);
517+
assert.equal(state.project.invalid, false);
518+
519+
const notifications: Array<{ message: string; level: string }> = [];
520+
const ctx = {
521+
cwd,
522+
hasUI: true,
523+
ui: { notify: (msg: string, level: string) => notifications.push({ message: msg, level }) },
524+
} as any;
525+
const behavior = await resolveHandoffResumeBehavior(ctx);
526+
assert.equal(behavior, "wait");
527+
assert.equal(notifications.length, 1);
528+
assert.equal(notifications[0].level, "warning");
529+
assert.match(notifications[0].message, /Invalid global settings JSON/);
530+
} finally {
531+
await chmod(globalPath, 0o600);
532+
}
533+
});
534+
});
535+
504536
test("handoff resume setting is documented in README", async () => {
505537
const readme = await readFile(new URL("./README.md", import.meta.url), "utf8");
506538
const changelog = await readFile(new URL("./CHANGELOG.md", import.meta.url), "utf8");
@@ -632,6 +664,34 @@ test("agenticoding settings TUI handles invalid JSON policies", async () => {
632664
});
633665
});
634666

667+
test("agenticoding settings write path handles save failure with error notification", async () => {
668+
await withIsolatedSettings(async ({ home }) => {
669+
// Block the settings directory by creating a file where a directory is expected.
670+
// writeGlobalHandoffResumeBehavior calls mkdir(dirname(path), { recursive: true }),
671+
// so making .pi/agent a file (instead of a directory) will cause mkdir to throw ENOTDIR.
672+
await mkdir(join(home, ".pi"), { recursive: true });
673+
await writeFile(join(home, ".pi", "agent"), "block", "utf8");
674+
675+
const notifications: Array<{ message: string; level: string }> = [];
676+
const ctx = {
677+
cwd: home,
678+
hasUI: true,
679+
ui: { notify: (message: string, level: string) => notifications.push({ message, level }) },
680+
} as any;
681+
682+
await assert.rejects(
683+
() => writeGlobalHandoffResumeBehavior("proceed", ctx),
684+
/EEXIST|ENOTDIR|ENOSPC/,
685+
);
686+
});
687+
688+
// The async IIFE in createAgenticodingSettingsComponent's callback wraps model.save
689+
// in try/catch, where model.save delegates to writeGlobalHandoffResumeBehavior.
690+
// The rejection verified above proves that a filesystem error during write propagates
691+
// correctly, so the try/catch in the component callback will catch it and call
692+
// notify(ctx, `Failed to save handoff.resumeBehavior: ${err.message}`, "error");
693+
});
694+
635695
test("agenticoding settings command falls back without usable TUI", async () => {
636696
const headlessPi = new MockPi();
637697
registerAgenticoding(headlessPi as any);

settings.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,12 @@ async function readSettingsSource(label: SettingsSourceLabel, path: string): Pro
131131
let raw: string;
132132
try {
133133
raw = await readFile(path, "utf8");
134-
} catch {
135-
return { label, path, exists: false, invalid: false, settings: createSettingsObject(), resumeBehavior: undefined };
134+
} catch (error) {
135+
const code = typeof error === "object" && error !== null && "code" in error ? (error as { code?: unknown }).code : undefined;
136+
if (code === "ENOENT") {
137+
return { label, path, exists: false, invalid: false, settings: createSettingsObject(), resumeBehavior: undefined };
138+
}
139+
return { label, path, exists: true, invalid: true, settings: createSettingsObject(), resumeBehavior: undefined };
136140
}
137141

138142
try {
@@ -336,14 +340,18 @@ export function createAgenticodingSettingsComponent(
336340
(id, newValue) => {
337341
if (id !== "handoff.resumeBehavior" || !isHandoffResumeBehavior(newValue)) return;
338342
void (async () => {
339-
const saved = await model.save(newValue, ctx);
340-
model = await buildAgenticodingSettingsModel(ctx);
341-
settingsList.updateValue("handoff.resumeBehavior", model.effectiveBehavior);
342-
if (saved && model.projectOverrideWarning) {
343-
notify(ctx, model.projectOverrideWarning, "warning");
343+
try {
344+
const saved = await model.save(newValue, ctx);
345+
model = await buildAgenticodingSettingsModel(ctx);
346+
settingsList.updateValue("handoff.resumeBehavior", model.effectiveBehavior);
347+
if (saved && model.projectOverrideWarning) {
348+
notify(ctx, model.projectOverrideWarning, "warning");
349+
}
350+
refreshSummary();
351+
tui.requestRender();
352+
} catch (err) {
353+
notify(ctx, `Failed to save handoff.resumeBehavior: ${err instanceof Error ? err.message : String(err)}`, "error");
344354
}
345-
refreshSummary();
346-
tui.requestRender();
347355
})();
348356
},
349357
() => done("closed"),

0 commit comments

Comments
 (0)