From 5ec402fc10ad486981a48745ef86bf16fc1ca825 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 14 Jul 2026 20:47:46 +0000 Subject: [PATCH] fix(catalog): force codex refresh for authoritative pruning Built-in discovery skipped the OAuth refresh whenever a fresh authoritative cache existed, so an openai-codex user with an expired access token never got the model manager constructed and stale bundled models (e.g. gpt-5.4-nano) stayed selectable for the full cache TTL. Force the refresh for authoritative providers and forward the registry fetch through the Codex manager so discovery honors the configured transport. Fixes #5364 --- packages/catalog/CHANGELOG.md | 1 + packages/catalog/src/discovery/codex.ts | 4 +- .../catalog/src/provider-models/special.ts | 5 +- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/config/model-registry.ts | 28 +++++++++-- .../coding-agent/test/model-discovery.test.ts | 47 +++++++++++++++++++ 6 files changed, 79 insertions(+), 7 deletions(-) diff --git a/packages/catalog/CHANGELOG.md b/packages/catalog/CHANGELOG.md index 62437ba3f..2017e588b 100644 --- a/packages/catalog/CHANGELOG.md +++ b/packages/catalog/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed OpenAI Codex discovery to replace stale bundled models with the authenticated account catalog, preventing unsupported models from remaining selectable. ([#5364](https://github.com/can1357/oh-my-pi/issues/5364)) +- Fixed OpenAI Codex discovery ignoring the caller-supplied `fetch`, so it always hit the global network instead of the configured (proxy/extra-CA/test) fetch. ([#5364](https://github.com/can1357/oh-my-pi/issues/5364)) ## [16.4.3] - 2026-07-11 diff --git a/packages/catalog/src/discovery/codex.ts b/packages/catalog/src/discovery/codex.ts index 33dad7763..a9031458e 100644 --- a/packages/catalog/src/discovery/codex.ts +++ b/packages/catalog/src/discovery/codex.ts @@ -1,5 +1,5 @@ import { type } from "arktype"; -import type { ModelSpec } from "../types"; +import type { FetchImpl, ModelSpec } from "../types"; import { discoveryFetch } from "../utils"; import { CODEX_BASE_URL, CODEX_CLIENT_VERSION, OPENAI_HEADER_VALUES, OPENAI_HEADERS } from "../wire/codex"; @@ -60,7 +60,7 @@ export interface CodexModelDiscoveryOptions { /** Abort signal for network request cancellation. */ signal?: AbortSignal; /** Optional fetch implementation override for tests. */ - fetchFn?: typeof fetch; + fetchFn?: FetchImpl; } /** diff --git a/packages/catalog/src/provider-models/special.ts b/packages/catalog/src/provider-models/special.ts index 2bd937130..e747f6de6 100644 --- a/packages/catalog/src/provider-models/special.ts +++ b/packages/catalog/src/provider-models/special.ts @@ -13,19 +13,20 @@ export interface OpenAICodexModelManagerConfig { accessToken?: string; accountId?: string; clientVersion?: string; + fetch?: FetchImpl; } export function openaiCodexModelManagerOptions( config: OpenAICodexModelManagerConfig = {}, ): ModelManagerOptions<"openai-codex-responses"> { - const { accessToken, accountId, clientVersion } = config; + const { accessToken, accountId, clientVersion, fetch } = config; return { providerId: "openai-codex", dynamicModelsAuthoritative: true, ...(accessToken ? { fetchDynamicModels: async () => { - const result = await fetchCodexModels({ accessToken, accountId, clientVersion }); + const result = await fetchCodexModels({ accessToken, accountId, clientVersion, fetchFn: fetch }); return result?.models ?? null; }, } diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9fe2494f3..75d6ef564 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -43,6 +43,7 @@ - Fixed launch tool rendering stacking a stale pending header over a bare `✓ Launch` line and raw text: the tool now uses a merged registry renderer with one per-op status header (op, target, `state · pid · uptime` meta), stripped log cursor suffixes, capped collapsed log/list previews, and a launch tool glyph - Fixed confusing launch start/wait results when readiness timed out with the log pattern already matched (readiness needs log AND port): the result printed a contradictory `Ready: ` next to `Readiness timed out` without naming the failing condition. Daemon snapshots now carry the unmet conditions (`readyPending`), and start/wait results state exactly what never happened (e.g. `port 3100 on 127.0.0.1 never accepted connections`); the TUI shows a `waiting on port` badge on starting daemons - Fixed the in-process `stat` builtin mangling BSD-style invocations like `stat -f "%Sm %N" file` (macOS muscle memory): GNU `-f` means `--file-system`, so the format string was treated as a file operand — printing filesystem info for the real operands and erroring with `cannot read file system information for '%Sm %N'`. A `-f` whose format value contains `%` is now detected as BSD syntax and translated to the GNU equivalent (`%Sm`→`%y`, `%N`→`%n`, `%z`→`%s`, epoch/`S`-form times, owner/group/permission and `H`/`L` sub-field directives, `-L`/`-n`/`-q`/`-F` flag clusters, with `%n`/`%t` as literal newline/tab); directives with no GNU counterpart fail with a clear `unsupported BSD format directive` error +- Fixed authoritative providers (e.g. `openai-codex`) keeping unsupported bundled models selectable when a fresh model cache and an expired OAuth token coincided: built-in discovery now forces the OAuth refresh so the provider's model manager is constructed and prunes stale bundled entries (e.g. `gpt-5.4-nano`) instead of waiting out the cache TTL. ([#5364](https://github.com/can1357/oh-my-pi/issues/5364)) - Fixed the remaining GNU-flavored shell builtins that broke under macOS/BSD muscle memory, using the same unambiguous-detection approach as the `stat` fix (only invocations that are invalid or nonsensical under GNU semantics are reinterpreted; unsupported BSD forms fail loudly instead of producing wrong output): `date -r ` formats the epoch when no such file exists (GNU `-r FILE` mtime preserved), signed `date -v±N` adjustments translate to `-d` relative dates and `-j` is accepted (`-j -f` strptime parse mode and field-set `-v` error clearly); `sed -i '' 's/…/…/' file` drops the BSD empty backup-suffix token instead of treating it as the script; `mktemp -t prefix` without X's creates `$TMPDIR/prefix.XXXXXXXXXX` (the GNU `too few X's` error path); `tail -r` reverses input by delegating to `tac` (with `-n`/`-c`/`-f` combinations erroring clearly); `find -E` maps to `-regextype posix-extended` ahead of the expression; `base64 -D` decodes as an alias of `-d`; and `ln -sfh` works via a `-h` alias of `--no-dereference` (clap's `-h` help short is dropped to match real GNU/BSD ln; `--help` unchanged) ## [16.4.8] - 2026-07-12 diff --git a/packages/coding-agent/src/config/model-registry.ts b/packages/coding-agent/src/config/model-registry.ts index 7fb1491b6..976c89b60 100644 --- a/packages/coding-agent/src/config/model-registry.ts +++ b/packages/coding-agent/src/config/model-registry.ts @@ -1592,6 +1592,7 @@ export class ModelRegistry { providerId: string, strategy: ModelRefreshStrategy, cacheProviderId: string, + authoritative: boolean, ): Promise { const peekedKey = await this.#peekApiKeyForProvider(providerId); if (isAuthenticated(peekedKey) || strategy === "offline") { @@ -1601,7 +1602,13 @@ export class ModelRegistry { if (oauthCredentials.length === 0) { return peekedKey; } - if (strategy === "online-if-uncached") { + // Authoritative providers prune bundled models only when their manager is + // actually constructed, which needs an authenticated key. A fresh cache does + // not let us skip the refresh here: with an expired OAuth token peekedKey is + // undefined, the manager is never added, and stale bundled models survive the + // full cache TTL. So only take the no-refresh shortcut for non-authoritative + // providers, whose bundled models stay visible regardless. + if (strategy === "online-if-uncached" && !authoritative) { // Mirror shouldFetchRemoteSources: built-in managers use the catalog's // default TTL, so only refresh when the manager will actually fetch. const cache = readModelCache( @@ -1633,11 +1640,13 @@ export class ModelRegistry { ): Promise[]> { const specialProviderDescriptors: Array<{ providerId: string; + authoritative: boolean; resolveKey: (value: string | undefined) => string | undefined; createOptions: (key: string) => ModelManagerOptions; }> = [ { providerId: "google-antigravity", + authoritative: false, resolveKey: extractGoogleOAuthToken, createOptions: oauthToken => googleAntigravityModelManagerOptions({ @@ -1648,6 +1657,7 @@ export class ModelRegistry { }, { providerId: "google-gemini-cli", + authoritative: false, resolveKey: extractGoogleOAuthToken, createOptions: oauthToken => googleGeminiCliModelManagerOptions({ @@ -1658,12 +1668,14 @@ export class ModelRegistry { }, { providerId: "openai-codex", + authoritative: true, resolveKey: value => value, createOptions: accessToken => { const accountId = resolveOAuthAccountIdForAccessToken(this.authStorage, "openai-codex", accessToken); return openaiCodexModelManagerOptions({ accessToken, accountId, + fetch: this.#fetch, }); }, }, @@ -1688,12 +1700,22 @@ export class ModelRegistry { const cacheProviderId = descriptor.createModelManagerOptions({ baseUrl: discoveryBaseUrl, fetch: this.#fetch }) .cacheProviderId ?? descriptor.providerId; - return this.#resolveBuiltInDiscoveryApiKey(descriptor.providerId, strategy, cacheProviderId); + return this.#resolveBuiltInDiscoveryApiKey( + descriptor.providerId, + strategy, + cacheProviderId, + descriptor.dynamicModelsAuthoritative ?? false, + ); }), ); const specialKeys = await Promise.all( enabledSpecialProviderDescriptors.map(descriptor => - this.#resolveBuiltInDiscoveryApiKey(descriptor.providerId, strategy, descriptor.providerId), + this.#resolveBuiltInDiscoveryApiKey( + descriptor.providerId, + strategy, + descriptor.providerId, + descriptor.authoritative, + ), ), ); const options: ModelManagerOptions[] = []; diff --git a/packages/coding-agent/test/model-discovery.test.ts b/packages/coding-agent/test/model-discovery.test.ts index d700be529..eb7d4492f 100644 --- a/packages/coding-agent/test/model-discovery.test.ts +++ b/packages/coding-agent/test/model-discovery.test.ts @@ -279,6 +279,53 @@ describe("ModelRegistry runtime discovery", () => { expect(authStorage.getOAuthCredential("anthropic")?.access).toBe("sk-ant-oat-expired-anthropic"); }); + test("online-if-uncached refreshes expired OAuth for authoritative providers even when the cache is fresh", async () => { + // Regression for #5364: openai-codex is authoritative, so its bundled + // models are pruned only when the manager is actually constructed — which + // needs an authenticated key. With an expired OAuth token peekApiKey + // returns undefined; the fresh-cache shortcut must NOT skip the refresh, or + // the manager is never added and unsupported bundled ids (gpt-5.4-nano) + // remain selectable for the whole cache TTL. + const { refreshCalls } = await useAuthStorageWithRefreshTracker(); + await authStorage.set("openai-codex", { + type: "oauth", + access: "expired-openai-codex", + refresh: "refresh-openai-codex", + expires: Date.now() - 60_000, + }); + // Fresh + authoritative, but written against no static fingerprint so the + // constructed manager still performs the account-scoped fetch. + writeModelCache("openai-codex", Date.now() - 60_000, [], true, "", cacheDbPath); + let modelListCalls = 0; + const fetchMock: FetchImpl = async (input, init) => { + const url = String(input); + if (url.startsWith("https://chatgpt.com/backend-api") && url.includes("/models")) { + modelListCalls++; + expect(new Headers(init?.headers).get("Authorization")).toBe("Bearer fresh-openai-codex"); + return Response.json({ + models: [ + { + slug: "gpt-5.6-terra", + display_name: "GPT-5.6 Terra", + context_window: 372_000, + supported_in_api: true, + input_modalities: ["text", "image"], + }, + ], + }); + } + throw new Error(`Unexpected URL: ${url}`); + }; + const registry = new ModelRegistry(authStorage, modelsJsonPath, { fetch: fetchMock }); + + await registry.refreshProvider("openai-codex", "online-if-uncached"); + + expect(refreshCalls).toEqual(["openai-codex"]); + expect(modelListCalls).toBe(1); + expect(registry.find("openai-codex", "gpt-5.6-terra")).toBeDefined(); + expect(registry.find("openai-codex", "gpt-5.4-nano")).toBeUndefined(); + }); + test("configured discovery suppresses built-in special OAuth discovery", async () => { await authStorage.set("google-gemini-cli", { type: "oauth",