test: close remaining PR 387 review nits

Author: ndycode

README.md

@@ -199,6 +199,7 @@ If browser launch is blocked, use the alternate login paths in [docs/getting-sta
 | Accounts | `~/.codex/multi-auth/openai-codex-accounts.json` |
 | Flagged accounts | `~/.codex/multi-auth/openai-codex-flagged-accounts.json` |
 | Quota cache | `~/.codex/multi-auth/quota-cache.json` |
+| Runtime observability | `~/.codex/multi-auth/runtime-observability.json` |
 | Logs | `~/.codex/multi-auth/logs/codex-plugin/` |
 | Per-project accounts | `~/.codex/multi-auth/projects/<project-key>/openai-codex-accounts.json` |
 

lib/codex-manager/commands/report.ts

@@ -60,6 +60,20 @@ interface ModelInspection {
 	capabilities: ReturnType<typeof getModelCapabilities>;
 }
 
+function parsePositiveIntegerOption(
+	rawValue: string,
+): number | null {
+	const normalized = rawValue.trim();
+	if (!/^\d+$/.test(normalized)) {
+		return null;
+	}
+	const parsed = Number.parseInt(normalized, 10);
+	if (!Number.isSafeInteger(parsed) || parsed < 1) {
+		return null;
+	}
+	return parsed;
+}
+
 const RETRYABLE_WRITE_CODES = new Set(["EBUSY", "EPERM"]);
 
 export interface ReportCommandDeps {
@@ -163,17 +177,17 @@ function parseReportArgs(args: string[]): ParsedArgsResult<ReportCliOptions> {
 		if (arg === "--max-accounts") {
 			const value = args[i + 1];
 			if (!value) return { ok: false, message: "Missing value for --max-accounts" };
-			const parsed = Number.parseInt(value, 10);
-			if (!Number.isFinite(parsed) || parsed < 1) {
+			const parsed = parsePositiveIntegerOption(value);
+			if (parsed === null) {
 				return { ok: false, message: "--max-accounts must be a positive integer" };
 			}
 			options.maxAccounts = parsed;
 			i += 1;
 			continue;
 		}
 		if (arg.startsWith("--max-accounts=")) {
-			const parsed = Number.parseInt(arg.slice("--max-accounts=".length), 10);
-			if (!Number.isFinite(parsed) || parsed < 1) {
+			const parsed = parsePositiveIntegerOption(arg.slice("--max-accounts=".length));
+			if (parsed === null) {
 				return { ok: false, message: "--max-accounts must be a positive integer" };
 			}
 			options.maxAccounts = parsed;
@@ -182,17 +196,17 @@ function parseReportArgs(args: string[]): ParsedArgsResult<ReportCliOptions> {
 		if (arg === "--max-probes") {
 			const value = args[i + 1];
 			if (!value) return { ok: false, message: "Missing value for --max-probes" };
-			const parsed = Number.parseInt(value, 10);
-			if (!Number.isFinite(parsed) || parsed < 1) {
+			const parsed = parsePositiveIntegerOption(value);
+			if (parsed === null) {
 				return { ok: false, message: "--max-probes must be a positive integer" };
 			}
 			options.maxProbes = parsed;
 			i += 1;
 			continue;
 		}
 		if (arg.startsWith("--max-probes=")) {
-			const parsed = Number.parseInt(arg.slice("--max-probes=".length), 10);
-			if (!Number.isFinite(parsed) || parsed < 1) {
+			const parsed = parsePositiveIntegerOption(arg.slice("--max-probes=".length));
+			if (parsed === null) {
 				return { ok: false, message: "--max-probes must be a positive integer" };
 			}
 			options.maxProbes = parsed;

test/codex-manager-report-command.test.ts

@@ -100,6 +100,22 @@ describe("runReportCommand", () => {
 		);
 	});
 
+	it.each([
+		["--max-accounts", "1.9", "--max-accounts must be a positive integer"],
+		["--max-accounts", "1e3", "--max-accounts must be a positive integer"],
+		["--max-accounts", "2foo", "--max-accounts must be a positive integer"],
+		["--max-probes", "1.9", "--max-probes must be a positive integer"],
+		["--max-probes", "1e3", "--max-probes must be a positive integer"],
+		["--max-probes", "2foo", "--max-probes must be a positive integer"],
+	] as const)("rejects malformed numeric flags %s %s", async (flag, value, message) => {
+		const deps = createDeps();
+
+		const result = await runReportCommand([flag, value], deps);
+
+		expect(result).toBe(1);
+		expect(deps.logError).toHaveBeenCalledWith(message);
+	});
+
 	it("writes json report output when requested", async () => {
 		const deps = createDeps();
 

test/config-explain.test.ts

@@ -146,6 +146,42 @@ describe("getPluginConfigExplainReport", () => {
 		});
 	});
 
+	it("covers the rate-limit explain rows and env sources", async () => {
+		const { getPluginConfigExplainReport } = await import("../lib/config.js");
+		const keys = [
+			"rateLimitDedupWindowMs",
+			"rateLimitStateResetMs",
+			"rateLimitMaxBackoffMs",
+			"rateLimitShortRetryThresholdMs",
+		] as const;
+		const report = getPluginConfigExplainReport();
+		for (const key of keys) {
+			expect(expectEntry(report, key)).toBeDefined();
+		}
+
+		const serialized = JSON.parse(JSON.stringify(report)) as {
+			entries: Array<{ key: string; value: unknown; defaultValue: unknown; source: string }>;
+		};
+		for (const key of keys) {
+			const entry = serialized.entries.find((item) => item.key === key);
+			expect(entry).toBeDefined();
+			expect(entry?.value).toBeDefined();
+			expect(entry?.defaultValue).toBeDefined();
+		}
+
+		vi.resetModules();
+		process.env.CODEX_AUTH_RATE_LIMIT_DEDUP_WINDOW_MS = "3210";
+		process.env.CODEX_AUTH_RATE_LIMIT_STATE_RESET_MS = "654321";
+		process.env.CODEX_AUTH_RATE_LIMIT_MAX_BACKOFF_MS = "12345";
+		process.env.CODEX_AUTH_RATE_LIMIT_SHORT_RETRY_THRESHOLD_MS = "250";
+		const envMod = await import("../lib/config.js");
+		const envReport = envMod.getPluginConfigExplainReport();
+		for (const key of keys) {
+			const entry = expectEntry(envReport, key);
+			expect(entry?.source).toBe("env");
+		}
+	});
+
 	it("reports default and env sources", async () => {
 		const mod = await import("../lib/config.js");
 		let report = mod.getPluginConfigExplainReport();

test/fetch-helpers.test.ts

@@ -525,6 +525,20 @@ describe('Fetch Helpers Module', () => {
 			expect(rateLimit).toBeUndefined();
 		});
 
+		it('prioritizes entitlement over rate-limit when signals conflict', async () => {
+			const body = {
+				error: {
+					code: 'usage_not_included',
+					type: 'usage_limit_reached',
+					message: 'mixed signals',
+				},
+			};
+			const resp = new Response(JSON.stringify(body), { status: 404 });
+			const { response: mapped, rateLimit } = await handleErrorResponse(resp);
+			expect(mapped.status).toBe(403);
+			expect(rateLimit).toBeUndefined();
+		});
+
 		it('leaves non-usage 404 errors unchanged', async () => {
 			const body = { error: { code: 'not_found', message: 'nope' } };
 			const resp = new Response(JSON.stringify(body), { status: 404 });
@@ -1053,6 +1067,15 @@ describe('createEntitlementErrorResponse', () => {
 			expect(rateLimit?.retryAfterMs).toBeGreaterThan(0);
 		});
 
+		it('does not remap 404s with malformed json bodies', async () => {
+			const response = new Response('{ invalid json', { status: 404 });
+
+			const { response: result, rateLimit } = await handleErrorResponse(response);
+
+			expect(result.status).toBe(404);
+			expect(rateLimit).toBeUndefined();
+		});
+
 		it('does not treat non-429 rate-limit text as a cooldown signal', async () => {
 			const response = new Response('rate_limit_exceeded - upstream overloaded', {
 				status: 500,

test/storage.test.ts

@@ -2980,6 +2980,26 @@ describe("storage", () => {
 
 			unlinkSpy.mockRestore();
 		});
+
+		it("throws and does not write reset marker when the wal file cannot be removed", async () => {
+			await fs.writeFile(testStoragePath, "{}", "utf-8");
+			await fs.writeFile(`${testStoragePath}.wal`, "{}", "utf-8");
+			const resetMarkerPath = getIntentionalResetMarkerPath(testStoragePath);
+			const unlinkSpy = vi.spyOn(fs, "unlink").mockImplementation(async (targetPath) => {
+				if (String(targetPath) === `${testStoragePath}.wal`) {
+					const error = Object.assign(new Error("locked"), { code: "EBUSY" });
+					throw error;
+				}
+				return Promise.resolve();
+			});
+
+			try {
+				await expect(clearAccounts()).rejects.toMatchObject({ code: "EBUSY" });
+				expect(existsSync(resetMarkerPath)).toBe(false);
+			} finally {
+				unlinkSpy.mockRestore();
+			}
+		});
 	});
 
 	describe("StorageError with cause", () => {