Skip to content

Commit 5562a43

Browse files
committed
refactor(lib): dedupe drifted isRecord guards into the canonical utils helper
Sweep follow-up to the runtime-current-account isRecord fix: of the nine local isRecord definitions in lib/, two had drifted from the canonical array-rejecting contract in lib/utils.ts (both accepted arrays): - lib/codex-manager/commands/rotation.ts (app helper status file parse) - lib/refresh-lease.ts (lease lock/result payload parse) Both local copies are deleted in favor of importing the canonical helper. Unlike the runtime-current-account case, the drift here was latent: downstream per-field validation already yielded all-null/invalid results for array payloads, so there is no observable behavior change. New tests pin the array-rejection contract at both call sites (array lock/result files never become a lease or follower result; an array helper status file reads as "not running"). The six remaining local isRecord copies already match the canonical semantics and are left in place. https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
1 parent 98d9819 commit 5562a43

4 files changed

Lines changed: 54 additions & 8 deletions

File tree

lib/codex-manager/commands/rotation.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
import { isRateLimitedMarker } from "../rate-limit-markers.js";
5050
import type { PluginConfig } from "../../types.js";
5151
import type { AccountMetadataV3, AccountStorageV3 } from "../../storage.js";
52+
import { isRecord } from "../../utils.js";
5253

5354
type LoadedStorage = AccountStorageV3 | null;
5455

@@ -500,10 +501,6 @@ function formatEnvOverride(): string {
500501
return parsed ? "enabled" : "disabled";
501502
}
502503

503-
function isRecord(value: unknown): value is Record<string, unknown> {
504-
return typeof value === "object" && value !== null;
505-
}
506-
507504
function readOptionalNumber(record: Record<string, unknown>, key: string): number | null {
508505
const value = record[key];
509506
return typeof value === "number" && Number.isFinite(value) ? value : null;

lib/refresh-lease.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { createLogger } from "./logger.js";
66
import { getCodexMultiAuthDir } from "./runtime-paths.js";
77
import { safeParseTokenResult } from "./schemas.js";
88
import type { TokenResult } from "./types.js";
9+
import { isRecord } from "./utils.js";
910

1011
const log = createLogger("refresh-lease");
1112

@@ -66,10 +67,6 @@ function hashRefreshToken(refreshToken: string): string {
6667
return createHash("sha256").update(refreshToken).digest("hex");
6768
}
6869

69-
function isRecord(value: unknown): value is Record<string, unknown> {
70-
return value !== null && typeof value === "object";
71-
}
72-
7370
function parseLeasePayload(raw: unknown): LeaseFilePayload | null {
7471
if (!isRecord(raw)) return null;
7572
const tokenHash = typeof raw.tokenHash === "string" ? raw.tokenHash : "";

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,25 @@ describe("codex-multi-auth rotation command", () => {
446446
expect(infos.join("\n")).toContain("Codex app helper: not running");
447447
});
448448

449+
it("treats an array helper status file as not running", async () => {
450+
// Pins the canonical isRecord contract (lib/utils.ts): a status file
451+
// whose top-level JSON value is an array must read as "no status", not
452+
// as a record with all-null fields.
453+
const root = await createTempRoot("codex-rotation-helper-array-");
454+
process.env.CODEX_MULTI_AUTH_DIR = root;
455+
await mkdir(root, { recursive: true });
456+
await writeFile(
457+
join(root, "runtime-rotation-app-helper.json"),
458+
"[]\n",
459+
"utf8",
460+
);
461+
const { deps, infos } = createDeps({ storage: null });
462+
463+
await expect(runRotationCommand(["status"], deps)).resolves.toBe(0);
464+
465+
expect(infos.join("\n")).toContain("Codex app helper: not running");
466+
});
467+
449468
it("handles overlapping enable commands without dropping saves or app binds", async () => {
450469
const {
451470
deps,

test/refresh-lease.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,39 @@ describe("RefreshLeaseCoordinator", () => {
521521
expect(handle.role).toBe("bypass");
522522
});
523523

524+
it("rejects array JSON payloads in result and lock files", async () => {
525+
// Pins the canonical isRecord contract (lib/utils.ts): a top-level JSON
526+
// array must never be treated as a lease or result record, even though
527+
// arrays satisfy `typeof value === "object"`.
528+
const refreshToken = "token-array-payload";
529+
const tokenHash = hashToken(refreshToken);
530+
const lockPath = join(leaseDir, `${tokenHash}.lock`);
531+
const resultPath = join(leaseDir, `${tokenHash}.result.json`);
532+
await mkdir(leaseDir, { recursive: true });
533+
534+
// An array result file must not turn the caller into a follower.
535+
await writeFile(resultPath, "[]\n", "utf8");
536+
const coordinator = new RefreshLeaseCoordinator({
537+
enabled: true,
538+
leaseDir,
539+
leaseTtlMs: 2_000,
540+
waitTimeoutMs: 120,
541+
pollIntervalMs: 20,
542+
resultTtlMs: 2_000,
543+
});
544+
const owner = await coordinator.acquire(refreshToken);
545+
expect(owner.role).toBe("owner");
546+
await owner.release();
547+
548+
// An array lock file is an invalid payload: never owned, never stale, so
549+
// the acquire times out and bypasses instead of stealing the lock.
550+
await writeFile(lockPath, "[]\n", "utf8");
551+
const blocked = await coordinator.acquire(refreshToken);
552+
expect(blocked.role).toBe("bypass");
553+
await blocked.release();
554+
await expect(readFile(lockPath, "utf8")).resolves.toBe("[]\n");
555+
});
556+
524557
it("prunes stale artifacts while keeping non-file entries", async () => {
525558
await mkdir(leaseDir, { recursive: true });
526559
const staleLock = join(leaseDir, "stale.lock");

0 commit comments

Comments
 (0)