fix(cron): normalize agentId filter through the same agent-id normalizer

lonexreb · lonexreb · commit 363137161a7e · 2026-05-04T23:37:41.000-05:00
Codex follow-up on PR #77191 noted: cron job ownership is normalized via normalizeOptionalAgentId/normalizeAgentId on create/update paths, but the new agentId filter compared raw trimmed strings. As a result, requests like cron.list({ agentId: 'MAIN' }) missed jobs stored as 'main', and implicit default-agent jobs could also be dropped when the configured default-agent id casing differed from caller input — even though the same IDs are treated as equivalent in create/update paths. Route both the requested filter and the resolved default-agent id through normalizeOptionalAgentId, and normalize each job's agentId the same way when computing the effective owner. Add a regression test covering the 'MAIN' vs 'main' casing scenario and a whitespace+case variant.
diff --git a/src/cron/service.list-page-agent-filter.test.ts b/src/cron/service.list-page-agent-filter.test.ts
@@ -104,4 +104,32 @@ describe("cron listPage agentId filter (#77118)", () => {
     ]);
     expect(mainPage.total).toBe(2);
   });
+
+  it("normalizes the agentId filter so case-only mismatches still match (#77118)", async () => {
+    // Regression for the codex follow-up on PR #77191: cron job ownership is
+    // normalized via `normalizeOptionalAgentId`/`normalizeAgentId` (lowercased
+    // + path-safe) on create/update paths. Before the fix, the filter
+    // compared raw trimmed strings, so requests like
+    // `cron.list({ agentId: "MAIN" })` against jobs stored as `"main"` (or
+    // implicit default-agent jobs whose default-agent id was uppercase)
+    // dropped valid jobs even though the same IDs are treated as equivalent
+    // elsewhere.
+    const jobs = [
+      createBaseJob({ id: "main-1", name: "lowercase main", agentId: "main" }),
+      createBaseJob({ id: "default-implicit", name: "implicit default", agentId: undefined }),
+      createBaseJob({ id: "alpha-only", name: "alpha", agentId: "alpha" }),
+    ];
+    const state = createMockCronStateForJobs({ jobs, defaultAgentId: "MAIN" });
+
+    // Caller uses uppercase but the filter normalizes through the same
+    // `normalizeAgentId` used by job create paths, so `MAIN` matches `main`
+    // and the implicit default-agent job (since defaultAgentId normalizes the
+    // same way).
+    const upperPage = await listPage(state, { agentId: "MAIN" });
+    expect(upperPage.jobs.map((j) => j.id).toSorted()).toEqual(["default-implicit", "main-1"]);
+
+    // Whitespace+casing variant resolves the same way.
+    const messyPage = await listPage(state, { agentId: "  Main  " });
+    expect(messyPage.jobs.map((j) => j.id).toSorted()).toEqual(["default-implicit", "main-1"]);
+  });
 });
diff --git a/src/cron/service/ops.ts b/src/cron/service/ops.ts
@@ -30,6 +30,7 @@ import type {
   CronSortDir,
 } from "./list-page-types.js";
 import { locked } from "./locked.js";
+import { normalizeOptionalAgentId } from "./normalize.js";
 import type { CronServiceState } from "./state.js";
 import { ensureLoaded, persist, warnIfDisabled } from "./store.js";
 import {
@@ -277,13 +278,18 @@ export async function listPage(state: CronServiceState, opts?: CronListPageOptio
     await ensureLoadedForRead(state);
     const query = normalizeLowercaseStringOrEmpty(opts?.query);
     const enabledFilter = resolveEnabledFilter(opts);
-    const agentIdFilter =
-      typeof opts?.agentId === "string" && opts.agentId.trim() ? opts.agentId.trim() : null;
+    // Normalize the requested agentId through the same agent-id normalizer
+    // used by job create/update paths (`normalizeOptionalAgentId` →
+    // `normalizeAgentId`) so case-only mismatches like
+    // `cron.list({ agentId: "MAIN" })` against jobs stored as `"main"` are
+    // treated as equivalent. Returns `undefined` when no filter was requested.
+    const agentIdFilter = normalizeOptionalAgentId(opts?.agentId) ?? null;
     // Cron jobs are allowed to omit `agentId` for the default agent — delivery
     // and session-cleanup code already treats missing agentId as the default.
     // Mirror that here so `cron.list({ agentId: defaultAgentId })` returns
-    // default-agent jobs that omit the field, not an empty page. See #77118.
-    const defaultAgentId = state.deps.defaultAgentId?.trim() || undefined;
+    // default-agent jobs that omit the field, not an empty page. Normalize the
+    // default-agent id too so the comparison is symmetric. See #77118.
+    const defaultAgentId = normalizeOptionalAgentId(state.deps.defaultAgentId);
     const sortBy = opts?.sortBy ?? "nextRunAtMs";
     const sortDir = opts?.sortDir ?? "asc";
     const source = state.store?.jobs ?? [];
@@ -295,7 +301,8 @@ export async function listPage(state: CronServiceState, opts?: CronListPageOptio
         return false;
       }
       if (agentIdFilter !== null) {
-        const effectiveOwner = (job.agentId ?? "").trim() || defaultAgentId || "";
+        const normalizedJobOwner = normalizeOptionalAgentId(job.agentId);
+        const effectiveOwner = normalizedJobOwner ?? defaultAgentId ?? "";
         if (effectiveOwner !== agentIdFilter) {
           return false;
         }
