diff --git a/gui/src/pages/Models.tsx b/gui/src/pages/Models.tsx index 7d099bd877f..0dabe8ea505 100644 --- a/gui/src/pages/Models.tsx +++ b/gui/src/pages/Models.tsx @@ -296,9 +296,8 @@ export default function Models({ apiBase, restartEpoch = 0, catalogSyncedAt, rep pickerFlight.current?.controller.abort(); pickerFlight.current?.clear(); pickerFlight.current = null; - cancelAppServerRead(); }; - }, [apiBase, catalogActive, cancelAppServerRead]); + }, [apiBase, catalogActive]); useLayoutEffect(() => { // Pin inferred Custom before any late GET can switch mode and unmount its draft. if (catalogActive && pickerDraft === null && pickerMode === "custom") setPickerDraft("custom"); diff --git a/gui/tests/models-status-toast.test.tsx b/gui/tests/models-status-toast.test.tsx index 66bb641359c..0622e41023a 100644 --- a/gui/tests/models-status-toast.test.tsx +++ b/gui/tests/models-status-toast.test.tsx @@ -613,6 +613,31 @@ test("leaving Models aborts its pending picker save", async () => { expect(container.querySelector(".action-toast")).toBeNull(); }); +test("changing Models tabs preserves the pending app-server status read", async () => { + const baseFetch = globalThis.fetch; + let statusSignal: AbortSignal | null | undefined; + let releaseStatus!: (response: Response) => void; + globalThis.fetch = (async (input, init) => { + if (String(input).endsWith("/api/system/codex-app-server")) { + statusSignal = init?.signal; + return new Promise(resolve => { releaseStatus = resolve; }); + } + return baseFetch(input, init); + }) as typeof fetch; + + await mountModelsForRefreshWarning(); + await waitForModelsFeedback(() => releaseStatus !== undefined); + const combosTab = [...container.querySelectorAll('[role="tab"]')] + .find(button => button.textContent?.startsWith("Combos")); + expect(combosTab).toBeDefined(); + await act(async () => { combosTab!.click(); }); + + expect(statusSignal?.aborted).toBe(false); + await act(async () => { releaseStatus(Response.json({ state: "stale", runningCount: 1 })); }); + await waitForModelsFeedback(() => container.querySelector(".codex-stale-banner") !== null); + expect(container.querySelector(".codex-stale-banner")).not.toBeNull(); +}); + function holdPostSaveAppServerRead() { const baseFetch = globalThis.fetch; diff --git a/src/routing/history/indexer.ts b/src/routing/history/indexer.ts index 186860fa943..3ac745ebfba 100644 --- a/src/routing/history/indexer.ts +++ b/src/routing/history/indexer.ts @@ -22,6 +22,7 @@ import { getConfigDir } from "../../config"; import { recordOwnedConfigPath } from "../../lib/config-ownership"; import { currentUsageLogRevision, + encodePersistedRequestedModel, normalizeUsageEntryForTest, usageLogPath, type PersistedUsageEntry, @@ -510,7 +511,9 @@ function queryRows( }; if (filters.provider !== undefined) add("provider = ?", filters.provider); if (filters.model !== undefined) add("model = ?", filters.model); - if (filters.requestedModel !== undefined) add("requested_model = ?", filters.requestedModel); + // Rows store the bounded encoded form, so the lookup value must be encoded the + // same way — short selectors encode to themselves and still match verbatim. + if (filters.requestedModel !== undefined) add("requested_model = ?", encodePersistedRequestedModel(filters.requestedModel)); if (filters.status !== undefined) add("status = ?", filters.status); if (filters.conversationId !== undefined) add("conversation_id = ?", filters.conversationId); if (filters.surface !== undefined) add("surface = ?", filters.surface); diff --git a/src/usage/log.ts b/src/usage/log.ts index 6955db2073a..bf54a0727f3 100644 --- a/src/usage/log.ts +++ b/src/usage/log.ts @@ -222,8 +222,33 @@ export interface PersistedRequestSpend extends RequestSpendTotals { } const MAX_PERSISTED_MOVE_REASONS = 8; +// Model selectors are NOT length-bound at admission: configured and discovered +// model ids reach MODEL_DISCOVERY_MAX_MODEL_ID_LENGTH, and the wire `model` +// field is raw client input. Persisting a plain prefix would merge selectors +// that share it, so over-long selectors persist as prefix + a digest of the +// FULL selector — bounded, deterministic, and still exact-matchable. +const MAX_PERSISTED_REQUESTED_MODEL_LEN = 130; +const REQUESTED_MODEL_DIGEST_HEX_LEN = 16; const LOGICAL_REQUEST_ID_RE = /^[A-Za-z0-9_.:-]{1,64}$/; +/** + * Persisted form of the wire model selector. Selectors within the bound persist + * verbatim; longer selectors persist as a prefix plus a short digest of the full + * value, so two distinct selectors that share the prefix never collapse into one + * persisted identity. Exact-match readers (`requested_model = ?`) must encode + * lookup input through this same function. Idempotent — encoded forms fit the + * bound — which matters because rows are normalized again on read. + */ +export function encodePersistedRequestedModel(selector: string): string { + if (selector.length <= MAX_PERSISTED_REQUESTED_MODEL_LEN) return selector; + const digest = createHash("sha256") + .update(selector) + .digest("hex") + .slice(0, REQUESTED_MODEL_DIGEST_HEX_LEN); + const prefixLen = MAX_PERSISTED_REQUESTED_MODEL_LEN - REQUESTED_MODEL_DIGEST_HEX_LEN - 1; + return `${selector.slice(0, prefixLen)}~${digest}`; +} + export function isLogicalRequestId(value: unknown): value is string { return typeof value === "string" && LOGICAL_REQUEST_ID_RE.test(value); } @@ -856,7 +881,9 @@ function normalizeUsageEntry(entry: PersistedUsageEntry): PersistedUsageEntry { ? { conversationId: entry.conversationId.trim().slice(0, 128) } : {}), ...(entry.resolvedModel ? { resolvedModel: entry.resolvedModel } : {}), - ...(entry.requestedModel ? { requestedModel: entry.requestedModel } : {}), + ...(typeof entry.requestedModel === "string" && entry.requestedModel + ? { requestedModel: encodePersistedRequestedModel(entry.requestedModel) } + : {}), ...(shadowCallRewrittenFrom ? { shadowCallRewrittenFrom } : {}), ...(typeof entry.requestedEffort === "string" && entry.requestedEffort ? { requestedEffort: capMetadataString(entry.requestedEffort) } diff --git a/tests/usage/request-history-index.test.ts b/tests/usage/request-history-index.test.ts index 240fd391c59..c376d6ae7fb 100644 --- a/tests/usage/request-history-index.test.ts +++ b/tests/usage/request-history-index.test.ts @@ -300,6 +300,29 @@ describe("request-history index (RI-02)", () => { expect(byRange.rows.map(row => row.requestId)).toEqual(["f2"]); }); + test("requestedModel filter matches the encoded form of over-long selectors", async () => { + // Two valid selectors sharing the first 130 chars must stay distinguishable: + // the persisted form is prefix + digest, and the filter encodes identically. + const sharedPrefix = `a/${"m".repeat(200)}`; + const selectorA = `${sharedPrefix}-alpha`; + const selectorB = `${sharedPrefix}-omega`; + appendUsageEntry(entry("sel-a", 1000, "a", "m1", { requestedModel: selectorA })); + appendUsageEntry(entry("sel-b", 2000, "a", "m1", { requestedModel: selectorB })); + + const pageA = await queryRequestHistory({ requestedModel: selectorA }, undefined, 10); + expect(pageA.rows.map(row => row.requestId)).toEqual(["sel-a"]); + const pageB = await queryRequestHistory({ requestedModel: selectorB }, undefined, 10); + expect(pageB.rows.map(row => row.requestId)).toEqual(["sel-b"]); + + // Rows surface the bounded persisted form; filtering by that displayed value + // round-trips because the encoding is idempotent. + const persistedA = pageA.rows[0]!.requestedModel!; + expect(persistedA).not.toBe(selectorA); + expect(persistedA.length).toBeLessThanOrEqual(130); + const roundTrip = await queryRequestHistory({ requestedModel: persistedA }, undefined, 10); + expect(roundTrip.rows.map(row => row.requestId)).toEqual(["sel-a"]); + }); + test("row-by-id returns the canonical entry and unknown ids 404 through the API", async () => { appendUsageEntry(entry("target-id", 1234)); const row = await requestHistoryRowById("target-id"); diff --git a/tests/usage/usage-log.test.ts b/tests/usage/usage-log.test.ts index 02c3b346750..fc43c30931e 100644 --- a/tests/usage/usage-log.test.ts +++ b/tests/usage/usage-log.test.ts @@ -6,6 +6,7 @@ import { join } from "node:path"; import { appendUsageEntry, currentUsageLogRevision, + encodePersistedRequestedModel, normalizeUsageEntryForTest, normalizeClaudeCompatibilityUsageLog, normalizePersistedUsageRow, @@ -150,6 +151,47 @@ describe("usage log", () => { expect(normalized.attempts).toEqual([]); }); + test("bounds requested model selectors before appending usage rows", () => { + const requestedModel = `policy/${"x".repeat(1024 * 1024)}`; + appendUsageEntry({ + requestId: "ocx-bounded-selector", + timestamp: 1, + provider: "unknown", + model: "unknown", + requestedModel, + status: 404, + durationMs: 1, + usageStatus: "unreported", + }); + + const raw = readFileSync(usageLogPath(), "utf8"); + const persisted = JSON.parse(raw) as PersistedUsageEntry; + expect(persisted.requestedModel).toBe(encodePersistedRequestedModel(requestedModel)); + expect(persisted.requestedModel!.length).toBeLessThanOrEqual(130); + expect(raw.length).toBeLessThan(1024); + }); + + test("keeps over-long selectors that share the bounded prefix distinguishable", () => { + // Selectors are not length-bound at admission, so two valid selectors can + // agree past the persistence bound; they must not collapse into one identity. + const sharedPrefix = `provider/${"m".repeat(200)}`; + const selectorA = `${sharedPrefix}-alpha`; + const selectorB = `${sharedPrefix}-omega`; + expect(selectorA.slice(0, 130)).toBe(selectorB.slice(0, 130)); + + const encodedA = encodePersistedRequestedModel(selectorA); + const encodedB = encodePersistedRequestedModel(selectorB); + expect(encodedA).not.toBe(encodedB); + expect(encodedA.length).toBeLessThanOrEqual(130); + expect(encodedB.length).toBeLessThanOrEqual(130); + + // Short selectors persist verbatim, and re-normalizing a persisted row is a + // no-op — normalizeUsageEntry also runs on every read. + const short = "provider/model"; + expect(encodePersistedRequestedModel(short)).toBe(short); + expect(encodePersistedRequestedModel(encodedA)).toBe(encodedA); + }); + test("preserves only valid non-PII Codex account log labels", () => { const normalized = normalizeUsageEntryForTest({ requestId: "ocx-account-label",