fix(cli): report a completed whole-file reset even when the follow-up read fails

The config file is already deleted when the override report runs, so a
setting that can't be resolved at that point is printed as a note on
stderr instead of turning a finished reset into a failure. Without such a
setting the output is unchanged.

Refs #2528

Co-Authored-By: Claude Opus 5.5 <[email protected]>

packages/cli/src/commands/config.ts

@@ -1,11 +1,19 @@
 import { Command } from "commander";
-import { ConfigStore, VALID_KEYS, type ResolvedSetting, type ValidKey } from "../config-store.js";
+import { ConfigStore, ENV_MAP, VALID_KEYS, type ResolvedSetting, type ValidKey } from "../config-store.js";
 import { initWorkspaceCommand } from "./config-init-workspace.js";
 
 function formatRow(key: string, value: unknown): string {
   return `${key.padEnd(28)} ${value}`;
 }
 
+/** The environment variable that supplied a key's value: its primary name, or the legacy
+ *  alias when only that is set (the order resolution reads them in). */
+function envVariableFor(key: ValidKey): string {
+  const { primary, legacy } = ENV_MAP[key];
+  const value = process.env[primary];
+  return legacy && (value === undefined || value === "") ? legacy : primary;
+}
+
 function summarizeSettings(store: ConfigStore): Record<ValidKey, ResolvedSetting> {
   return store.resolveAllWithSource();
 }
@@ -192,8 +200,15 @@ Precedence: CLI flag > environment variable > config file > default`)
     .description("Set a config value")
     .action(async (key: string, value: string) => {
       try {
-        store.set(key, value);
-        console.log(`${key} = ${store.get(key)}`);
+        const saved = store.set(key, value);
+        const effective = store.resolveWithSource(key);
+        if (effective.source === "file") {
+          console.log(`${key} = ${effective.value}`);
+        } else if (effective.source === "env") {
+          console.log(`${key} = ${saved} saved; ${envVariableFor(key as ValidKey)} overrides it in this environment, so ${effective.value} is in effect.`);
+        } else {
+          console.log(`${key} = ${saved} saved; the default ${effective.value} is in effect.`);
+        }
         // SWEEP-c: boot-only key + running daemon = stale-until-restart; say so loudly.
         if (isBootOnlyKey(key) && (await probeDaemonRunning())) {
           console.error(`note: '${key}' is read at daemon BOOT — the running daemon keeps its current value; this takes effect on the next daemon restart (rig daemon stop && rig daemon start).`);
@@ -212,9 +227,26 @@ Precedence: CLI flag > environment variable > config file > default`)
       try {
         store.reset(key);
         if (key) {
-          console.log(`${key} reset to default (${store.get(key)}).`);
+          const effective = store.resolveWithSource(key);
+          if (effective.source === "env") {
+            console.log(`${key} file override cleared; ${envVariableFor(key as ValidKey)} still sets it to ${effective.value} in this environment (default ${effective.defaultValue}).`);
+          } else {
+            console.log(`${key} reset to default (${effective.value}).`);
+          }
         } else {
-          console.log("Config reset to defaults.");
+          // The file is already gone: a setting that can't be resolved now is a note, not a failed reset.
+          let overridden: string[] = [];
+          let unresolved: string | null = null;
+          try {
+            const all = store.resolveAllWithSource();
+            overridden = VALID_KEYS.filter((k) => all[k].source === "env").map((k) => `${k} (${envVariableFor(k)})`);
+          } catch (err) {
+            unresolved = (err as Error).message;
+          }
+          console.log(overridden.length === 0
+            ? "Config reset to defaults."
+            : `Config reset to defaults, except where the environment overrides them: ${overridden.join(", ")}.`);
+          if (unresolved) console.error(`note: ${unresolved}`);
         }
       } catch (err) {
         console.error((err as Error).message);

packages/cli/src/config-store.ts

@@ -1521,7 +1521,8 @@ export class ConfigStore {
     return this.resolveWithSource(key).value;
   }
 
-  set(key: string, value: string): void {
+  /** Save a value to the config file and return it as saved (coerced). */
+  set(key: string, value: string): string | number | boolean {
     const removedMessage = removedContextSettingMessage(key);
     if (removedMessage) throw new Error(removedMessage);
     // OPR.0.4.4.15: the registered dynamic class is accepted; every other
@@ -1534,7 +1535,7 @@ export class ConfigStore {
       mkdirSync(dirname(this.configPath), { recursive: true });
       this.writeConfig(JSON.stringify(fcDyn, null, 2) + "\n");
       this.verifyPersisted(["feed", "subscriptions", feedHost.hostId, "enabled"], coercedDyn);
-      return;
+      return coercedDyn;
     }
     if (!isValidKey(key)) {
       throw new Error(`Unknown config key "${key}". Valid keys: ${VALID_KEYS.join(", ")}`);
@@ -1557,6 +1558,7 @@ export class ConfigStore {
     mkdirSync(dirname(this.configPath), { recursive: true });
     this.writeConfig(JSON.stringify(fileConfig, null, 2) + "\n");
     this.verifyPersisted(KEY_TO_PATH[key], coerced);
+    return coerced;
   }
 
   /**

packages/cli/test/config-env-override-report.test.ts

@@ -0,0 +1,108 @@
+// `rig config set` and `rig config reset` describe the config-file change they made. When an
+// environment variable still supplies the value in effect, they name it instead of calling that
+// value the default or printing it as the saved value.
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import * as fs from "node:fs";
+import * as os from "node:os";
+import * as path from "node:path";
+import { ENV_MAP } from "../src/config-store.js";
+import { configCommand } from "../src/commands/config.js";
+
+// Every config variable, plus the removed context-packs variable that ConfigStore still refuses when it's set.
+const ENV_NAMES = [...new Set([...Object.values(ENV_MAP).flatMap((e) => (e.legacy ? [e.primary, e.legacy] : [e.primary])),
+  "OPENRIG_CONTEXT_PACKS_ROOT"])];
+
+let dir: string;
+let configPath: string;
+let savedEnv: Record<string, string | undefined>;
+
+async function run(args: string[]): Promise<string> {
+  const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
+  try {
+    await configCommand(configPath, { probeDaemonRunning: async () => false })
+      .parseAsync(["node", "rig", ...args]);
+    return logSpy.mock.calls.map((c) => c.join(" ")).join("\n");
+  } finally {
+    logSpy.mockRestore();
+  }
+}
+
+describe("rig config set and reset beside an environment override", () => {
+  beforeEach(() => {
+    dir = fs.mkdtempSync(path.join(os.tmpdir(), "config-env-override-"));
+    configPath = path.join(dir, "config.json");
+    savedEnv = Object.fromEntries(ENV_NAMES.map((name) => [name, process.env[name]]));
+    for (const name of ENV_NAMES) delete process.env[name];
+    process.exitCode = undefined;
+  });
+
+  afterEach(() => {
+    for (const name of ENV_NAMES) {
+      if (savedEnv[name] === undefined) delete process.env[name];
+      else process.env[name] = savedEnv[name];
+    }
+    vi.restoreAllMocks();
+    process.exitCode = undefined;
+    fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  it("a whole-file reset still reports success when a removed setting's variable is set, and names it as a note", async () => {
+    fs.writeFileSync(configPath, JSON.stringify({ daemon: { port: 9000 } }));
+    process.env["OPENRIG_CONTEXT_PACKS_ROOT"] = "/old/packs";
+    const error = vi.spyOn(console, "error").mockImplementation(() => {});
+    expect(await run(["reset"])).toBe("Config reset to defaults.");
+    expect(process.exitCode).toBeUndefined();
+    expect(fs.existsSync(configPath)).toBe(false);
+    expect(error.mock.calls.map((c) => c.join(" ")).join("\n")).toContain("note: OPENRIG_CONTEXT_PACKS_ROOT was removed");
+  });
+
+  it("reset of one key names the variable that still sets it instead of calling its value the default", async () => {
+    fs.writeFileSync(configPath, JSON.stringify({ daemon: { port: 9000 } }));
+    process.env["OPENRIG_PORT"] = "8123";
+    expect(await run(["reset", "daemon.port"])).toBe(
+      "daemon.port file override cleared; OPENRIG_PORT still sets it to 8123 in this environment (default 7433).",
+    );
+    expect(JSON.parse(fs.readFileSync(configPath, "utf8"))).toEqual({});
+  });
+
+  it("names a legacy variable when only it is set", async () => {
+    vi.spyOn(console, "warn").mockImplementation(() => {});
+    fs.writeFileSync(configPath, JSON.stringify({ daemon: { port: 9000 } }));
+    process.env["RIGGED_PORT"] = "8123";
+    expect(await run(["reset", "daemon.port"])).toBe(
+      "daemon.port file override cleared; RIGGED_PORT still sets it to 8123 in this environment (default 7433).",
+    );
+  });
+
+  it("set reports the saved value and the variable that overrides it", async () => {
+    process.env["OPENRIG_RUNTIME_READINESS_TIMEOUT_SECONDS"] = "75";
+    expect(await run(["set", "runtime.readiness_timeout_seconds", "60"])).toBe(
+      "runtime.readiness_timeout_seconds = 60 saved; OPENRIG_RUNTIME_READINESS_TIMEOUT_SECONDS overrides it in this environment, so 75 is in effect.",
+    );
+    expect(JSON.parse(fs.readFileSync(configPath, "utf8"))).toEqual({ runtime: { readinessTimeoutSeconds: 60 } });
+  });
+
+  it("set says when a default is in effect instead of the saved value", async () => {
+    vi.spyOn(process.stderr, "write").mockImplementation(() => true);
+    process.env["OPENRIG_HEALTH_CONTEXT_PRESSURE_WARNING_PERCENT"] = "98";
+    expect(await run(["set", "health.context_pressure.critical_percent", "97"])).toBe(
+      "health.context_pressure.critical_percent = 97 saved; the default 99 is in effect.",
+    );
+  });
+
+  it("a whole-file reset names the overrides that remain", async () => {
+    fs.writeFileSync(configPath, JSON.stringify({ daemon: { port: 9000 } }));
+    process.env["OPENRIG_PORT"] = "8123";
+    expect(await run(["reset"])).toBe(
+      "Config reset to defaults, except where the environment overrides them: daemon.port (OPENRIG_PORT).",
+    );
+    expect(fs.existsSync(configPath)).toBe(false);
+  });
+
+  it("without an override, the messages are unchanged", async () => {
+    fs.writeFileSync(configPath, JSON.stringify({ daemon: { port: 9000 } }));
+    expect(await run(["reset", "daemon.port"])).toBe("daemon.port reset to default (7433).");
+    expect(await run(["set", "daemon.port", "7555"])).toBe("daemon.port = 7555");
+    expect(await run(["reset"])).toBe("Config reset to defaults.");
+  });
+});