The saveConfig() function only includes a hardcoded subset of properties when writing the config file. Any property not explicitly listed (like disableResponseStorage) will be dropped. I have added `disableResponseStorage` to the `configToSave` object as the immediate fix. [Linking Issue this fixes.](https://github.com/openai/codex/issues/726)
This commit is contained in:
@@ -294,6 +294,12 @@ if (!apiKey && !NO_API_KEY_REQUIRED.has(provider.toLowerCase())) {
|
|||||||
process.exit(1);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const flagPresent = Object.hasOwn(cli.flags, "disableResponseStorage");
|
||||||
|
|
||||||
|
const disableResponseStorage = flagPresent
|
||||||
|
? Boolean(cli.flags.disableResponseStorage) // value user actually passed
|
||||||
|
: (config.disableResponseStorage ?? false); // fall back to YAML, default to false
|
||||||
|
|
||||||
config = {
|
config = {
|
||||||
apiKey,
|
apiKey,
|
||||||
...config,
|
...config,
|
||||||
@@ -303,10 +309,7 @@ config = {
|
|||||||
(cli.flags.reasoning as ReasoningEffort | undefined) ?? "high",
|
(cli.flags.reasoning as ReasoningEffort | undefined) ?? "high",
|
||||||
flexMode: Boolean(cli.flags.flexMode),
|
flexMode: Boolean(cli.flags.flexMode),
|
||||||
provider,
|
provider,
|
||||||
disableResponseStorage:
|
disableResponseStorage,
|
||||||
cli.flags.disableResponseStorage !== undefined
|
|
||||||
? Boolean(cli.flags.disableResponseStorage)
|
|
||||||
: config.disableResponseStorage,
|
|
||||||
};
|
};
|
||||||
|
|
||||||
// Check for updates after loading config. This is important because we write state file in
|
// Check for updates after loading config. This is important because we write state file in
|
||||||
|
|||||||
@@ -323,6 +323,22 @@ export const loadConfig = (
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (
|
||||||
|
storedConfig.disableResponseStorage !== undefined &&
|
||||||
|
typeof storedConfig.disableResponseStorage !== "boolean"
|
||||||
|
) {
|
||||||
|
if (storedConfig.disableResponseStorage === "true") {
|
||||||
|
storedConfig.disableResponseStorage = true;
|
||||||
|
} else if (storedConfig.disableResponseStorage === "false") {
|
||||||
|
storedConfig.disableResponseStorage = false;
|
||||||
|
} else {
|
||||||
|
log(
|
||||||
|
`[codex] Warning: 'disableResponseStorage' in config is not a boolean (got '${storedConfig.disableResponseStorage}'). Ignoring this value.`,
|
||||||
|
);
|
||||||
|
delete storedConfig.disableResponseStorage;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const instructionsFilePathResolved =
|
const instructionsFilePathResolved =
|
||||||
instructionsPath ?? INSTRUCTIONS_FILEPATH;
|
instructionsPath ?? INSTRUCTIONS_FILEPATH;
|
||||||
const userInstructions = existsSync(instructionsFilePathResolved)
|
const userInstructions = existsSync(instructionsFilePathResolved)
|
||||||
@@ -372,7 +388,7 @@ export const loadConfig = (
|
|||||||
instructions: combinedInstructions,
|
instructions: combinedInstructions,
|
||||||
notify: storedConfig.notify === true,
|
notify: storedConfig.notify === true,
|
||||||
approvalMode: storedConfig.approvalMode,
|
approvalMode: storedConfig.approvalMode,
|
||||||
disableResponseStorage: storedConfig.disableResponseStorage ?? false,
|
disableResponseStorage: storedConfig.disableResponseStorage === true,
|
||||||
reasoningEffort: storedConfig.reasoningEffort,
|
reasoningEffort: storedConfig.reasoningEffort,
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -488,6 +504,7 @@ export const saveConfig = (
|
|||||||
provider: config.provider,
|
provider: config.provider,
|
||||||
providers: config.providers,
|
providers: config.providers,
|
||||||
approvalMode: config.approvalMode,
|
approvalMode: config.approvalMode,
|
||||||
|
disableResponseStorage: config.disableResponseStorage,
|
||||||
reasoningEffort: config.reasoningEffort,
|
reasoningEffort: config.reasoningEffort,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
93
codex-cli/tests/disableResponseStorage.agentLoop.test.ts
Normal file
93
codex-cli/tests/disableResponseStorage.agentLoop.test.ts
Normal file
@@ -0,0 +1,93 @@
|
|||||||
|
/**
|
||||||
|
* codex-cli/tests/disableResponseStorage.agentLoop.test.ts
|
||||||
|
*
|
||||||
|
* Verifies AgentLoop's request-building logic for both values of
|
||||||
|
* disableResponseStorage.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, it, expect, vi } from "vitest";
|
||||||
|
import { AgentLoop } from "../src/utils/agent/agent-loop";
|
||||||
|
import type { AppConfig } from "../src/utils/config";
|
||||||
|
import { ReviewDecision } from "../src/utils/agent/review";
|
||||||
|
|
||||||
|
/* ─────────── 1. Spy + module mock ─────────────────────────────── */
|
||||||
|
const createSpy = vi.fn().mockResolvedValue({
|
||||||
|
data: { id: "resp_123", status: "completed", output: [] },
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.mock("openai", () => ({
|
||||||
|
default: class {
|
||||||
|
public responses = { create: createSpy };
|
||||||
|
},
|
||||||
|
APIConnectionTimeoutError: class extends Error {},
|
||||||
|
}));
|
||||||
|
|
||||||
|
/* ─────────── 2. Parametrised tests ─────────────────────────────── */
|
||||||
|
describe.each([
|
||||||
|
{ flag: true, title: "omits previous_response_id & sets store:false" },
|
||||||
|
{ flag: false, title: "sends previous_response_id & allows store:true" },
|
||||||
|
])("AgentLoop with disableResponseStorage=%s", ({ flag, title }) => {
|
||||||
|
/* build a fresh config for each case */
|
||||||
|
const cfg: AppConfig = {
|
||||||
|
model: "o4-mini",
|
||||||
|
provider: "openai",
|
||||||
|
instructions: "",
|
||||||
|
disableResponseStorage: flag,
|
||||||
|
notify: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
it(title, async () => {
|
||||||
|
/* reset spy per iteration */
|
||||||
|
createSpy.mockClear();
|
||||||
|
|
||||||
|
const loop = new AgentLoop({
|
||||||
|
model: cfg.model,
|
||||||
|
provider: cfg.provider,
|
||||||
|
config: cfg,
|
||||||
|
instructions: "",
|
||||||
|
approvalPolicy: "suggest",
|
||||||
|
disableResponseStorage: flag,
|
||||||
|
additionalWritableRoots: [],
|
||||||
|
onItem() {},
|
||||||
|
onLoading() {},
|
||||||
|
getCommandConfirmation: async () => ({ review: ReviewDecision.YES }),
|
||||||
|
onLastResponseId() {},
|
||||||
|
});
|
||||||
|
|
||||||
|
await loop.run([
|
||||||
|
{
|
||||||
|
type: "message",
|
||||||
|
role: "user",
|
||||||
|
content: [{ type: "input_text", text: "hello" }],
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
expect(createSpy).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
const call = createSpy.mock.calls[0];
|
||||||
|
if (!call) {
|
||||||
|
throw new Error("Expected createSpy to have been called at least once");
|
||||||
|
}
|
||||||
|
const payload: any = call[0];
|
||||||
|
|
||||||
|
if (flag) {
|
||||||
|
/* behaviour when ZDR is *on* */
|
||||||
|
expect(payload).not.toHaveProperty("previous_response_id");
|
||||||
|
if (payload.input) {
|
||||||
|
payload.input.forEach((m: any) => {
|
||||||
|
expect(m.store === undefined ? false : m.store).toBe(false);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
/* behaviour when ZDR is *off* */
|
||||||
|
expect(payload).toHaveProperty("previous_response_id");
|
||||||
|
if (payload.input) {
|
||||||
|
payload.input.forEach((m: any) => {
|
||||||
|
if ("store" in m) {
|
||||||
|
expect(m.store).not.toBe(false);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
43
codex-cli/tests/disableResponseStorage.test.ts
Normal file
43
codex-cli/tests/disableResponseStorage.test.ts
Normal file
@@ -0,0 +1,43 @@
|
|||||||
|
/**
|
||||||
|
* codex/codex-cli/tests/disableResponseStorage.test.ts
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, it, expect, beforeAll, afterAll } from "vitest";
|
||||||
|
import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from "node:fs";
|
||||||
|
import { join } from "node:path";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
|
||||||
|
import { loadConfig, saveConfig } from "../src/utils/config";
|
||||||
|
import type { AppConfig } from "../src/utils/config";
|
||||||
|
|
||||||
|
const sandboxHome: string = mkdtempSync(join(tmpdir(), "codex-home-"));
|
||||||
|
const codexDir: string = join(sandboxHome, ".codex");
|
||||||
|
const yamlPath: string = join(codexDir, "config.yaml");
|
||||||
|
|
||||||
|
describe("disableResponseStorage persistence", () => {
|
||||||
|
beforeAll((): void => {
|
||||||
|
// mkdir -p ~/.codex inside the sandbox
|
||||||
|
rmSync(codexDir, { recursive: true, force: true });
|
||||||
|
mkdirSync(codexDir, { recursive: true });
|
||||||
|
|
||||||
|
// seed YAML with ZDR enabled
|
||||||
|
writeFileSync(yamlPath, "model: o4-mini\ndisableResponseStorage: true\n");
|
||||||
|
});
|
||||||
|
|
||||||
|
afterAll((): void => {
|
||||||
|
rmSync(sandboxHome, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps disableResponseStorage=true across load/save cycle", async (): Promise<void> => {
|
||||||
|
// 1️⃣ explicitly load the sandbox file
|
||||||
|
const cfg1: AppConfig = loadConfig(yamlPath);
|
||||||
|
expect(cfg1.disableResponseStorage).toBe(true);
|
||||||
|
|
||||||
|
// 2️⃣ save right back to the same file
|
||||||
|
await saveConfig(cfg1, yamlPath);
|
||||||
|
|
||||||
|
// 3️⃣ reload and re-assert
|
||||||
|
const cfg2: AppConfig = loadConfig(yamlPath);
|
||||||
|
expect(cfg2.disableResponseStorage).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user