feat(codex): fetch and persist available models

[lsm]

Fetch the authenticated Codex model catalog for API-key and ChatGPT OAuth accounts, persist a credential-scoped cache, and route discovered models through the existing Responses bridge while retaining curated metadata as an overlay and fallback.

Focused provider, model-service, and Responses bridge tests pass. Static checks pass. Live credential verification was blocked because the available Codex token was rejected with HTTP 401.

---

[lsm]

🤖 Review by GLM-5.3 (Z.ai)

Model: GLM-5.3 | Client: HyperNeo | Provider: Z.ai
Diff: +964/−142 across 4 files — code 564 | tests 400 | comments 0 | other 0

Implements the task well: authenticated model discovery on both Codex endpoints, credential-scoped persistence with ETag/304, curated metadata overlay with bundled fallback, and dynamic Responses routing through the existing getModels() → model-service → picker seam (no UI change needed; verified no hardcoded codex list in web). Test coverage is strong and assertions are meaningful. The profile is majority production code (564 code vs 400 test lines), so the integration surface got the most scrutiny.

Note: the first pushed head broke 5 tests in provider-auth-lifecycle-matrix.test.ts (they still asserted the old POST /responses probe; the fix existed only uncommitted in the worktree). Commit 756f703 pushes that fix verbatim — thank you. The 1-core shard is still pending on CI; please confirm it goes green before merge (I don't run tests as reviewer).

Four change requests, all in the provider (production files unchanged by the fix commit):

1. P2 — bridge servers leak per catalog generation. Servers are keyed responses:${authKey}:${catalogGeneration} and the eviction loop skips every same-auth key, while nothing in production ever calls stopAllBridgeServers. Every generation bump permanently leaks a listening HTTP server (port, fd, per-session maps) — triggered by the bundled→remote swap, any model add/reorder at TTL refresh, and account switching. Keeping the old server alive mid-stream is right; never reaping it is not.
2. P2 — remote/cache parser bounds diverge (found independently by two reviewers). parseCodexRemoteModel has no length caps while parseCachedRemoteModel rejects displayName > 500 / description > 4000 — and parseModelCache is all-or-nothing. One oversized upstream field → cache persists → every restart rejects the whole file → dynamic catalog permanently unloadable while online fetches keep re-poisoning it. Apply identical bounds at both ends.
3. P2 — OAuth restart hydration gap. The persisted cache is activated only inside getModels() (constructor activation is env-API-key only), and app.ts skips initializeModels() for non-Anthropic users — so a codex-only user's first query with a persisted dynamic model hits buildSdkConfig → Unknown Codex model → the catch in getProviderEnvVars silently strips the codex env vars and the session fails with a misleading auth error. Activate the cache for file credentials too (or lazily on lookup miss), and add an OAuth restart test.
4. P2 — all-or-nothing remote parse. parseRemoteModelList requires every entry to parse; the codex models[] shape is third-party and live-unverified (token rejected 401), so one anomalous entry (a new visibility value, a missing priority) silently reverts the whole feature to the bundled board for first-run users. Skip invalid entries and keep the existing non-empty/≥1-visible guards.

Passing observations (not blocking): cross-scope modelRefresh join race (transient, self-heals on next call); clearModelCache() revalidates rather than clears — the ETag survives, so it cannot bust a bad-but-stable ETag, and the name diverges from every peer provider; health checks now treat network-offline as healthy (401/403 still throw — deliberate probe removal); stale-TTL getModels blocks up to 5 s offline with no negative caching (no worse than the old probe); 'none' visibility is routed identically to 'hide'; capabilities.maxContextWindow stays hardcoded at 1_050_000; live verification of the endpoint shape remains blocked — worth a follow-up online test once a valid credential is available.

Recommendation: REQUEST_CHANGES

[lsm]

🤖 Review by glm-5.3[1m] (z.ai)

Model: glm-5.3[1m] | Client: HyperNeo | Provider: z.ai
Diff: +1333/−163 across 6 files — code 653 | tests 680 | comments 0 | other 0

Recommendation: REQUEST_CHANGES — 1×P1, 3×P2

Reviewed at d9a452e (including 578dfa8c in-place bridge catalogs, 9860f4f queued forced refreshes, d9a452e hidden tier defaults). Production-code-major diff (653 added code lines vs 680 test lines): the risk sits in the new discovery/persistence/routing state machine, which is well covered by the new tests except where noted.

Verified resolved this round

All 16 resolved threads re-verified at this head: undrained response bodies; client_version=0.148.0 compat; scope-keyed refreshes with stale-completion discard; synchronous OAuth cache hydration on restart; 401-only credential rejection (403 keeps bundled catalog); picker-visible tier selection; bounded parser fields; per-model thinkingModes; in-place bridge catalog updates (no generation retirement, no port churn, no 5-minute kill of in-flight turns); forced refreshes queued behind in-flight fetches (with regression test); hidden/none models excluded from SDK tier defaults via getModelForTier (with visibility: 'none' coverage).

Findings

P1 — buildSdkConfig has no graceful path when the catalog no longer matches the session or the credentials (inline at the throw; includes open thread #PRRT_kwDORDDLj86avoQv, endorsed)

• (a) Dropped model: with the catalog now dynamic, a persisted session.config.model that falls out of catalogEntries (server-side removal, account switch) makes buildSdkConfig throw Unknown Codex model. The query path surfaces session.error and needs manual model re-selection; worse, getProviderEnvVars (provider-service.ts:336) and the title-config path (:404) swallow the throw and return {} env — the SDK then launches with no ANTHROPIC_BASE_URL/key and fails with a misleading auth error. The peer dynamic provider degrades instead: openrouter's buildSdkConfig routes ownsModel(modelId) ? modelId : DEFAULT_MODEL.
• (b) Scope drift: setCredentials() swaps auth immediately but leaves catalogEntries on the previous account's scope until the next getModels(); a synchronous buildSdkConfig in that window resolves old-account models through a bridge keyed by the new credentials.
  Minimal fix: on lookup failure or scope mismatch, fall back to getModelForTier('default') with a logger.warn (openrouter pattern), plus a regression test pinning the policy.

P2 — newly admitted non-gpt- ids misroute on provider-less inference (inline at the admission regex)
isOpenAIResponsesModel now admits o3/o4-mini/codex-*/ft:* into api-key catalogs (the suite asserts o3 is listed), but inferProviderForModel (registry.ts) maps only the gpt- prefix to anthropic-codex — its findProviderForModel fallback only helps once the dynamic catalog is resident (bundled fallback does not contain these ids). Provider-less paths (model-switch-handler.ts:117, space inferPersistableProviderForModel) then wire such ids to the anthropic provider, which rejects them; the web family heuristic has the same gap (cosmetic mis-grouping). Fix: teach daemon inference and the web heuristic the same admission prefixes, or restrict admission to prefixes inference knows.

P2 — 304 revalidation path has zero test coverage (inline at the 304 branch)
The if-none-match round-trip (304 → fetchedAt bump, catalog retained, cache rewrite, activateCachedCatalog re-activation) is new behavior with no test — a regression treating 304 as !response.ok (falling back to the bundled catalog) would pass CI. Cheap to test by hand-writing a stale cache file with an etag and returning 304 from the mocked fetch. Worth bundling: constructor hydration with a corrupt/oversized cache file (the catch-all has no coverage), and rejection of a cache persisted under a different clientVersion.

P2 — refreshed catalogs cannot remove models (open thread #PRRT_kwDORDDLj86avoQz, endorsed after verification)
refreshModels in model-service.ts:325-328 keeps previousModels whenever the merged list shrinks and re-stamps it fresh. With dynamic codex catalogs this is not one stale cycle: every subsequent refresh shrinks again and re-keeps previousModels, so a removed model never leaves the picker — and selecting it walks straight into the P1 throw. The refresh integration needs to distinguish an authoritative shrink (successful fetch for that provider) from a partial outage.

Passing observations (non-blocking)

• No models-array count/payload cap alongside the per-field caps (slug ≤256, name ≤500, description ≤4000); a MAX_MODELS bound would be consistent with the PR's own hardening. Realistic exposure is low (trusted TLS origin / same-user cache file).
• visibility is validated via String(record.visibility) but stored via as cast — a non-string like ['list'] passes validation and is then never listed (conservative direction; a typeof check closes it).
• No backoff on failed discovery: past TTL with a dead upstream, each getModels awaits a fresh 5s-timeout fetch inline (bounded in practice by model-service's cache).
• cacheMatchesScope re-implements sameScope field-for-field plus clientVersion; return cache.clientVersion === CODEX_COMPAT_CLIENT_VERSION && this.sameScope(cache.scope, scope) collapses it.
• Pre-existing, adjacent (not this PR): the Responses bridge's Bun.serve binds all interfaces rather than loopback — worth a follow-up one-liner hostname: '127.0.0.1', since the bridge attaches the user's bearer token to upstream calls.
• Live credential verification was blocked (401 token) per the PR description — QA should validate discovery against a real API-key and a real ChatGPT account before release.

[lsm]

🤖 Review by glm-5.3 (Z.ai)

Model: glm-5.3 | Client: HyperNeo | Provider: Z.ai
Diff (delta d9a452e…c38b843aa, both fix waves): +962/−83 across 15 files — code 248 | tests 714 | comments 0 | other 0

Verdict: REQUEST_CHANGES — 0 P0 / 0 P1 / 2 P2. Recommendation: REQUEST_CHANGES

The head moved from abc493e20 to c38b843aa mid-review, so this round covers both fix waves since the last reviewed head d9a452ece (commits 8cfeaabed, 17fd0844b, 157452cc7, abc493e20, c38b843aa). The delta is test-dominated (+714 of +962 lines) with meaningful assertions. CI is green on c38b843aa (39 pass / 4 skipping).

Verified fixed — the earlier 10 open threads plus awvIF; the coder may resolve these 11 with the dispositions below

Thread	Fixed in	Verification
avoQv — catalog scope on credential swap	8cfeaab	ensureCatalogForScope activates cached/bundled catalog on setCredentials (both auth branches) and revalidates in buildSdkConfig; scope-isolation test proves old-account models stop routing
avoQz — authoritative catalog shrink	8cfeaab	retainFailedProviderModels replaces the shorter-list-restores-previous heuristic; healthy-provider removals stick, only failed providers retain stale entries
av3_r (P1) — graceful model fallback	8cfeaab	buildSdkConfig falls back to getModelForTier('default') with a warning instead of throwing; the empty-env failure mode is unreachable because catalogEntries always keeps a list-visible entry
av3_w — provider inference prefixes	8cfeaab	daemon + web recognize gpt-/oN/codex-/ft:; ordering safe (openrouter /, ollama-specific, ownsModel lookups precede; ft: precedes the web colon-to-ollama rule); both sides lowercase first
av3_0 — 304/coverage gaps	8cfeaab	304 test asserts if-none-match + rewritten fetchedAt + retained catalog; corrupt-cache and client-version-mismatch tests added
av6vP — thinking modes in routing	17fd084	getModelThinkingMode + sessionModelIds clear bridge thinking for off models; upstream body asserted free of reasoning
av6vT — discovery outages vs health	17fd084	forced healthCheck() probe throws on transport/5xx while getModels() keeps the fallback; both RPC paths wired; handler tests
awQy- — swallowed provider failures	157452c	Anthropic propagates SDK failures; allSettled tracking retains only failed providers' stale entries; every production getModels() caller verified safe
awQzC — proactive OAuth refresh propagation	157452c	updateAuth covers all ResolvedResponsesAuth fields; account-scoped key is token-stable; port-stability + refreshed-bearer tests
awdAT — unusable payloads as unhealthy	abc493e	malformed JSON / bad envelope / no-usable-models set the discovery error while retaining the catalog; cleared on 304 and success; parameterized tests
awvIF — per-model reasoning levels	c38b843	efforts persisted (tolerant legacy-cache hydration), propagated into the bridge incl. aliases; clamp walks downward to the highest advertised level and omits reasoning when nothing at-or-below is advertised; server + provider + restart-hydration tests assert the clamped/disabled bodies

Open findings — P2, blocking

1. PRRT_kwDORDDLj86awvIO — sessionModelIds is last-write-wins between primary and fallback contexts (endorsed, verified). query-options-builder.ts:300-307 builds the fallback by spreading the same session config — same sessionId — with model swapped, so the second createContext to buildSdkConfig overwrites sessionModelIds[sessionId] with the fallback model. setSessionThinkingConfig then applies capability handling for the fallback regardless of which model actually serves: an off-primary with a reasoning-fallback sends an unsupported reasoning on primary requests (the new bridge clamp cannot help — the off primary has no reasoningEffortsByModelId entry, so the legacy path emits the effort), while a reasoning-primary with an off-fallback silently drops thinking. Fix per the thread: track both models, or make capability handling per-request (for example emit an empty supported_reasoning_efforts for off models so the bridge drops reasoning per request).

2. PRRT_kwDORDDLj86awvIW — updateBridgeAuth derives the lookup key from the refreshed auth (endorsed, verified). bridgeAuthCacheKey includes accountId plus the fedramp flag; when a refresh changes either, the bridgeServers lookup misses the still-live bridge created under the old key, leaving it on the expiring token and stale headers. With refresh-token rotation the bridge's internal 401-refresh closure holds the already-consumed token, so the next request through the issued bridge URL can hard-fail instead of self-healing. Fix per the thread: locate the bridge by the key it was created under, or update the active OAuth bridge before rekeying it.

Passing observations (non-blocking, no action requested)

• discoveryError is global while refresh dedupe is per-scope: a concurrent cross-scope fetch can stale-set or steal the forced-refresh flag around a healthCheck — microtask-window races on a user-clicked probe that self-heal on the next probe.
• The silent default-tier fallback plus thinking suppression can forward an SDK-supplied thinking block if a session's model disappears and the curated default is absent and the first listed model is non-reasoning — compound edge, loud 400, primary paths are tested.
• registry.getProviderInfo wraps getModels() in bare Promise.all and would now reject wholesale on an Anthropic SDK failure — currently no production callers; worth allSettled if ever wired up.
• Socket-dependent tests are isBun-gated per file convention while CI runs daemon units under vitest/node — a pre-existing limitation; the non-gated new tests (health probe, 304, shrink/retain, inference, handlers) are CI-covered.

[lsm]

🤖 Review by glm-5.3[1m] (Z.ai)

Model: glm-5.3[1m] | Client: HyperNeo | Provider: Z.ai
Diff: +3025/−242 across 18 files — code 1015 | tests 2010 | comments 0 | other 0

Recommendation: REQUEST_CHANGES

Verdict: REQUEST_CHANGES — P0: 0 · P1: 0 · P2: 1. Risk profile: two-thirds of the added lines are tests (2010 of 3025); the 1015 production lines center on the Codex catalog discovery/cache/bridge path, which carries the review weight.

Whole-PR review at 665a4f9 (the review began at a586c05; the final delta commit fix(codex): keep logout authoritative was reviewed in full and is clean — an in-flight OAuth refresh can no longer resurrect credentials on disk after logout, with a meaningful CI-runnable regression test). All 38 prior threads verified resolved at head, including the a586c05 rich-catalog reasoning fallback — parseCodexRemoteModel reuses bundledReasoningEfforts when supported_reasoning_levels is absent, preserving the gpt-5.4-mini high clamp, with the focused bridge test present at head.

Verified clean

• Goal alignment: fetch + persist + route discovered Codex models with curated overlay/fallback is complete against the ask; no acceptance criterion unaddressed; the OAuth refresh coalescing is load-bearing for concurrent discovery, not scope creep.
• Security: no secrets reach disk or logs (scope.credentialId is a sha256 hash; test asserts the API key absent from the cache file); no new credential-receiving origins (same two upstream bases as the pre-PR probe, /models instead of /responses); the slug charset validator blocks env-var injection into ANTHROPIC_DEFAULT_*/CLAUDE_CODE_AUTO_COMPACT_WINDOW; no prototype pollution (fresh literals + Object.fromEntries, __proto__ slugs rejected by the charset); atomic 0600 tmp+rename cache writes; all new regexes are anchored and linear.
• Compatibility: refreshModels/healthCheck are optional on Provider (all 12 implementations audited; 9 unchanged via the getModels() fallback); updateAuth/updateModels are required only on the single bridge factory implementer; every inferProviderForModel caller (6 daemon sites + web mirror) handles 'anthropic-codex' for the newly-routed o3/codex-*/ft:* IDs, and inferPersistableProviderForModel's other-owner gate still holds.
• Correctness spot-checks: constructor hydration ordering (no TDZ), 401-retry-then-throw, 403 fallback retention, 304 ETag revalidation, scope guards on every mutating terminal path of fetchModelCatalog, OAuth refresh coalescing with pre/post-save credentialsStillMatch re-checks and file restore, and the ≥1-listed-entry invariant that keeps getModelForTier/buildSdkConfig throw branches unreachable.

Finding (P2 — anchored inline)

modelRefreshes join breaks under failure: duplicate concurrent catalog fetches and premature map eviction. Details in the inline comment; minimal fix is a guarded delete mirroring the oauthRefreshes idiom ten lines away, plus re-reading the map after the join await.

Passing observations (below the change-request bar — no action requested)

• Bun.serve binds all interfaces (no hostname) — pre-existing, untouched by this diff; the bridge already forwarded with live tokens pre-PR. Worth a separate hardening follow-up.
• Bridge HTTP-roundtrip tests (describe.skipIf(!isBun), incl. the updateModels/updateAuth/reasoning-clamp coverage) skip in the vitest-based daemon CI — the file's standing convention (no Bun.serve shim exists under Node); they run under local bun test. A Node shim or exporting the pure mapThinkingToReasoningEffort for direct unit tests would be a good follow-up.
• Partial-failure retry-on-read spawns an Anthropic SDK subprocess per refresh cycle during outages — this is the accepted design from the resolved retention threads; the rate is activity-bound, not a tight loop.
• discoveryError is not scope-tagged, so a mid-flight credential flip can make one strict refresh throw the previous scope's error — one self-healing provider-failed cycle; transient and rare.
• Shared IDs (gpt-5.4 etc.) resolve to anthropic-codex over anthropic-copilot by registration order — consistent with the resolved specific-ownership decision; copilot-exclusive IDs keep copilot.
• ETag revalidation is mostly bypassed in practice because model-service refreshes are forced (refreshModels() → unconditional fetch); the 304 path mainly serves direct getModels() callers. Efficiency note only.
• Empty-but-successful catalogs drop the Claude fallback entries — the accepted authoritative-shrink semantics from the resolved threads.

[lsm]

🤖 Review by glm-5.3 (Z.ai)

Model: glm-5.3 | Client: HyperNeo | Provider: Z.ai
Diff: +3609/−379 across 18 files — code 1075 | tests 2534 | comments 0 | other 0

Verdict: REQUEST_CHANGES — 0 P0 · 2 P1 · 2 P2.

Recommendation: REQUEST_CHANGES

Code changes: 1075 added lines across 9 production files; test changes: 2534 added lines across 9 test files. Mostly-tests risk profile — the production delta is concentrated in the Codex bridge provider.

Round-1 whole-PR review of head d1b64b679. Note: the branch advanced twice during this review (2864c0729 → 580d773b5 → d1b64b679); the final head is what was reviewed. The late commits resolved two issues my earlier passes had identified: the getCachedModels() sync fallback removes the duplicate fallback discovery (double subprocess spawn) after a strict-refresh failure, and the simplified registry inference removes credential-dependent test ordering. The same-account 401 rotation retry and logout-abort in d1b64b679 were re-verified and are sound.

Verified as sound (re-derived, not inherited from prior threads):

• Credential-scoped fetch/persist/activate is coherent: strict cache validation (schemaVersion, per-field caps, fail-closed on any invalid entry), atomic 0600 tmp+rename writes, scope-keyed in-flight refreshes with correct join semantics (forced flags consumed only when a forced fetch starts), logout-authoritative credential handling, in-place bridge auth/model updates that preserve ports and in-flight requests, and per-request reasoning-effort clamping including the empty-list-disables-reasoning path.
• Security posture of the new code is clean: no secret/token logging on any new path, no prototype pollution, no injection sinks (env values/JSON only), no ReDoS in the new patterns, and no credential material in the cache file (sha256 scope ids only).
• The 46 resolved threads hold up under re-derivation at this head; concurrency windows in modelRefreshes/oauthRefreshes/updateBridgeAuth/replaceCatalog were specifically attacked and are bounded.
• Interface additions (refreshModels?, getCachedModels?, healthCheck?) are additive; all implementors and both call sites guard optionality. No new dependencies. Commit hygiene (26 conventional commits) checks out.

Blocking findings (inline comments attached):

1. P1 — no failure backoff in the global model cache (model-service.ts). Partial-failure refreshes delete cacheTimestamps, so every hot-path read starts a full refresh cycle for as long as any provider fails — and each cycle now spawns the Anthropic SDK discovery subprocess (the strict refreshModels hook bypasses the provider cache by design) plus a forced no-etag Codex catalog fetch. On dev, partial success still stamped the cache, so this churn did not exist. Please add a bounded failure cooldown; the automatic-rertry semantics the earlier threads asked for are preserved, only the rate gets bounded.
2. P1 — two unit tests read and write the developer's real ~/.hyperneo (anthropic-to-codex-bridge-provider.test.ts:2875 and :2945). They construct the provider with undefined authDir, so the constructor reads the real auth.json and a successful mocked discovery writes the real openai-models-cache.json. Cross-run contamination can silently change which production path later runs exercise.
3. P2 — the remote catalog array is unbounded (parseRemoteModelList). Every field is capped (slug ≤256, name ≤500, description ≤4000) but the models/data array length is not, so a hostile or compromised upstream can exhaust memory/disk even within the 5s timeout. An entry cap would complete the existing validation regime.
4. P2 — two new behaviors have no test: (a) modelRefreshes join-loop rejection propagation — concurrent non-strict getModels() callers under a 401 both reject (the 401 test is single-caller; the coalescing test uses a network error, which resolves without throwing); (b) triggerBackgroundRefresh's empty-catalog install branch is only covered via the explicit refreshModels() path.

Passing observations (no action requested this round): the two 401-retry blocks in fetchModelCatalog could collapse into one with the intentional updateBridgeAuth asymmetry made explicit (~−15 lines); sameScope/cacheMatchesScope/scopeKey re-list the same fields three ways; the codex-family regex is now mirrored across daemon and web (hoisting it to @hyperneo/shared would be a natural follow-up); MODELS_SUPPORTING_XHIGH_REASONING and CODEX_XHIGH_MODEL_IDS duplicate the same six ids and must be kept in sync manually. Pre-existing and untouched by this PR, but worth a separate hardening task: the Responses bridge binds all interfaces without authentication, and sessionModelAliasOverrides grows without bound.

Happy to re-review the delta once these are addressed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce4687d25e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Exclude rejected providers from catalog expiry

When an expired API-key catalog is revalidated and /models returns 401, the provider remains isAvailable() === true and its retained cache still reports the expired deadline, so this loop propagates that past timestamp to the global cache. Because definitive rejections are classified as rejected rather than failed, no refresh cooldown is recorded; consequently every subsequent models.list request sees the cache as stale and retries the rejected key. Exclude rejected providers from expiry calculation, clear their deadline, or apply the failure cooldown.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Match the rejected credential before suppressing imports

When a HyperNeo OAuth credential is rejected while ~/.codex/auth.json contains a different valid credential, this helper records the unrelated Codex credential as rejected merely because the file exists. After logout, importFromCodexAuth() suppresses that valid fallback until the file changes, leaving the provider unavailable. Record the import key only when the Codex file actually corresponds to the credential that received the definitive rejection.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Persist rejected-import suppression across restarts

When an imported Codex credential is definitively rejected and the daemon later restarts, this in-memory field resets, so the unchanged token in ~/.codex/auth.json is imported and retried again. Fresh evidence after the prior suppression fix is that no rejection marker is loaded from or written to persistent state; the promised suppression therefore lasts only for the current process rather than until the source credential changes.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a9856a343e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Restore provider availability snapshots after login

When a provider is logged out and later authenticated again, the logout handler leaves its snapshot set to false, while the successful auth.login callback only clears the model cache and never marks the provider available. Until model discovery happens, synchronous consumers such as createCustomAgentInit() therefore continue excluding the now-authenticated owner and can route GPT model overrides to anthropic-codex instead—for example, after logging back into Copilot when no client subsequently calls models.list. Update or clear the snapshot when credentials are installed or otherwise avoid treating the stale false value as authoritative.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve provider caches when publishing health changes

When a health check successfully discovers a changed catalog, this call clears not only the global model list but every provider-local cache that the health check just populated. For Codex, clearModelsCache() sets forceModelRefresh, and for providers such as Anthropic and OpenRouter it discards the loaded catalog outright; the providers.changed event then causes the model picker to issue another remote discovery request before it can rebuild the global cache. Invalidate only the global cache entry here so the freshly verified provider catalogs can be reused.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5d3ecfc821

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Normalize imported credentials before matching rejection

When ~/.codex/auth.json omits the optional tokens.account_id but its access-token JWT contains the account ID, importFromCodexAuth() enriches the stored credentials with that extracted value, while codexImportHash() hashes the source with an empty account ID. This comparison therefore rejects the match, so a definitive 401 logs out without recording suppression and the next availability check reimports the same rejected token indefinitely. Fresh evidence beyond the persisted-marker fix is that the source and stored hashes are computed from differently normalized account fields; compare the rejected token to the source before enrichment or normalize both inputs identically.

Useful? React with 👍 / 👎.

[lsm]

Fixed in ebfa832: codexImportHash() now derives the account ID the same way importFromCodexAuth() enriches stored credentials — JWT chatgpt_account_id first, file tokens.account_id as fallback — so both hashes normalize identically and a rejected token is suppressed even when the file omits the account ID. Covered by normalizes JWT-derived account IDs when matching rejected imports.

[chatgpt-codex-connector]

Advance the auth generation after 400 fallback retries

When credentials change while a request is awaiting a 400 response, this fallback correctly sends the rebuilt body with activeAuth, but it leaves requestAuthGeneration at the generation captured for the original credentials. If the retry succeeds, the stream callbacks therefore discard its continuation and encrypted reasoning state as stale; the analogous reasoning-item fallback has the same problem. Fresh evidence beyond the 401 generation fix is that these non-401 retries adopt current authentication without updating the callback generation snapshot.

Useful? React with 👍 / 👎.

[lsm]

Fixed in ebfa832: both 400 fallback retries (previous_response_id rebuild and reasoning-item drop) now assign requestAuthGeneration = authGeneration after re-sending with the current auth, mirroring the 401 retry paths, so a successful retry's continuation and reasoning callbacks are no longer discarded as stale.

[chatgpt-codex-connector]

Reject removed models instead of silently substituting

When a persisted or concurrently selected model is absent from the newly authoritative remote catalog, this branch silently runs the catalog's default model while the session configuration continues to name the requested model. This also combines with isValidModel()'s static Codex metadata fallback, which can approve a curated model that the current account's catalog no longer contains, so experiments may unknowingly run against a different model rather than receiving an actionable validation error. Preserve the previous error behavior or update the visible session model when an explicit fallback is intended.

Useful? React with 👍 / 👎.

[lsm]

Fixed in ebfa832 along both axes:

• buildSdkConfig() now tracks whether the active catalog is the bundled fallback or an authoritative fetched/hydrated one. Substitution to the tier default only applies while the bundled fallback is active (pre-PR behavior for unknown IDs); once an authoritative catalog is installed, a model absent from it throws Unknown Codex model, restoring the pre-PR error path. Covered by rejects models removed from an authoritative fetched catalog.
• isValidModel() gates the static Codex metadata fallback on the provider's current ownsModel(), so a curated model the account catalog no longer contains fails validation instead of being approved and then silently substituted. Covered by existing isValidModel tests plus the new ownership test.

Bundled-fallback substitution (never-fetched or scope-changed-before-first-fetch) is retained deliberately so persisted selections keep working across restarts and credential switches before the first successful fetch.

[lsm]

Fixed in 5d3ecfc: rejected providers excluded from catalog expiry, import suppression matched/persisted, availability snapshots cleared on credential change, provider caches preserved on health change.

[chatgpt-codex-connector]

💡 Codex Review

HyperNeo/packages/daemon/src/lib/model-service.ts

Line 223 in 7e23fd0

refreshInProgress.delete(cacheKey);

Delete only the refresh promise that is finishing

When clearModelsCache(cacheKey) cancels an in-flight refresh and a replacement refresh starts before the old promise settles, this unconditional deletion removes the replacement promise from refreshInProgress. A third caller can then start another refresh with the same generation, allowing duplicate catalog requests and late results to overwrite each other; only delete the entry when it still refers to this finishing promise. The foreground refresh has the same unconditional cleanup.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Keep logged-out providers marked unavailable

When a provider retains an ownership catalog after logout, as Copilot does with dynamicModelsCache, deleting this snapshot makes availabilitySnapshots.get(provider.id) return undefined, which passes the registry's !== false ownership filter. Synchronous model inference can therefore route a new Space agent to the logged-out provider until another discovery runs. Fresh evidence after the earlier logout-snapshot fix is that the latest change replaced markProviderAvailability(providerId, false) with this deletion; keep the explicit false state on logout and clear it only when credentials are installed.

Useful? React with 👍 / 👎.

[lsm]

Fixed in ebfa832: auth.logout marks the provider explicitly unavailable again (markProviderAvailability(providerId, false)), so a retained ownership catalog like Copilot's dynamicModelsCache cannot pass the !== false inference filter after logout. The snapshot is cleared only on the auth.login credential-install path, per the suggested split. Covered by keeps a logged-out provider marked unavailable for model inference and clears the availability snapshot when login installs credentials.

[chatgpt-codex-connector]

Invalidate in-flight generations with the cache entry

When a health check or catalog-scope change calls this helper while a global refresh is in flight, the refresh can already have captured the old provider models but still passes its unchanged cacheGeneration check. It then reinstalls that old catalog after this invalidation, and an event-triggered models.list can join the old refresh and treat its result as fresh. Advance the cache generation here so work started before the provider change cannot repopulate the entry.

Useful? React with 👍 / 👎.

[lsm]

Addressed in ebfa832 by routing the callers rather than changing invalidateModelsCacheEntry itself:

• Provider create/update/delete now go through full clearModelsCache(), which bumps cacheGeneration for in-flight keys and clears provider-level caches, so a refresh started before a provider mutation can no longer repopulate the old model set.
• Health-check paths (providers.test / providers.healthCheck) use the keyed clearModelsCache('global'), which also advances the generation for the global key while preserving provider-level caches per the earlier fix in 7e23fd0.
• invalidateModelsCacheEntry itself stays generation-neutral because the Codex catalog-scope notification fires from inside the very refresh that produced the replacement catalog — bumping the generation there makes that refresh discard its own fresh result and empties the cache (keeps the replacement catalog when a provider switches scope mid-refresh fails), and any provider read after the scope change returns the new catalog anyway.

[chatgpt-codex-connector]

Inherit context metadata for fine-tuned models

When API-key discovery returns a fine-tuned model such as ft:gpt-5.4:org:custom, the model-list response supplies only its ID, so this fallback assigns 128K even though the recognized base model has a 272K context window in MODEL_CONTEXT_WINDOWS (and GPT-5.6 bases are larger still). The provider already extracts the base ID for capability detection, but the resulting incorrect value is exported to the picker and CLAUDE_CODE_AUTO_COMPACT_WINDOW, causing fine-tuned sessions to compact substantially earlier than their base model requires. Resolve known base-model metadata before using the generic 128K default.

Useful? React with 👍 / 👎.

[lsm]

Fixed in ebfa832: resolveCodexBridgeModelId() now unwraps ft:<base>:… to its base ID, and codexRemoteModelInfo() prefers the resolved base-model window over the generic 128K default (remote-explicit context still wins over the default for slugs with no known base). ft:gpt-5.4:org:custom from the API-key catalog now reports 272000, and buildSdkConfig OAuth compaction follows the same resolution. Covered by inherits known base-model context windows for fine-tuned models.

[lsm]

Fixed in 5b6c517. and now only delete the entry when it still refers to the promise that is finishing, so a replacement refresh started after is not evicted by the old promise's finally block.

[lsm]

Fixed in 5b6c517. Background and foreground refresh now only delete the refreshInProgress entry when it still refers to the promise that is finishing, so a replacement refresh started after clearModelsCache is not evicted by the old promise finally block.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b6c51794f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Recheck credentials before logging out after refresh failure

When credential A's refresh returns a definitive failure, recordRejectedCodexImport() performs filesystem awaits before this unconditional logout. If an OAuth callback installs credential B during that interval, logout() clears B from memory and deletes the newly written auth file, so a successful login is immediately lost. Revalidate credentialKey after recording the rejection and before logging out; the same race also exists in clearUnrefreshableOauthCredentials().

Useful? React with 👍 / 👎.

[lsm]

Fixed in ebfa832: both logout sites now re-validate credentials after recordRejectedCodexImport()'s filesystem awaits and skip the logout when they changed — performOauthCredentialRefresh rechecks credentialsStillMatch(credentialKey) and clearUnrefreshableOauthCredentials reloads and compares the credential key. Covered by preserves credentials installed while a rejection is being recorded, which interposes setCredentials inside the rejection-marker rename.

[chatgpt-codex-connector]

Preserve dynamic catalogs when legacy providers fall back

When a provider without refreshModels() handles a transient discovery failure by returning a fallback catalog, this branch treats that result as an authoritative success. For example, Copilot catches an expired clientCache.listModels() failure and returns its static list while retaining dynamicModelsCache, and Ollama returns fallback models after a fetch failure; the global refresh consequently removes their previously published dynamic models instead of preserving them as it does for failed providers. Persisted selections then disappear from models.list until discovery recovers, so these providers need a refresh contract that reports failure or the service must distinguish their fallback result from a successful catalog replacement.

Useful? React with 👍 / 👎.

[lsm]

Disputing this one as out of scope for this PR — the behavior is unchanged from before it, not introduced by it. Pre-PR, loadModelsFromProviders treated every successful getModels() return as an authoritative replacement, so a provider that swallows a transient discovery failure and returns its static list (Copilot, Ollama) already replaced its previously published dynamic models in the global cache. This PR only adds preservation for providers that actually report failure (refreshModels\ throwing, definitiveAuthFailure`), which strictly improves on that baseline. Making legacy providers report failure would require a new refresh contract across Copilot/Ollama provider internals, which is a separate change from Codex catalog fetching; happy to follow up in its own PR if wanted.

[chatgpt-codex-connector]

💡 Codex Review

HyperNeo/packages/daemon/src/lib/providers/anthropic-to-codex-bridge-provider.ts

Lines 1985 to 1990 in ebfa832

	if (refreshToken) {
	const refreshed = await this.tryRefreshCodexToken(refreshToken);
	if (refreshed.ok) {
	accessToken = refreshed.token.access_token;
	refreshToken = refreshed.token.refresh_token || refreshToken;
	expires = Date.now() + refreshed.token.expires_in * 1000;

Preserve import identity across the eager token refresh

When ~/.codex/auth.json contains a refresh token and this eager refresh succeeds, HyperNeo stores the newly issued access token (and potentially a rotated refresh token), while the source file still contains the original values. If the imported credential is then definitively rejected, recordRejectedCodexImport() hashes the refreshed stored credential and compares it with the unchanged source hash, so it refuses to persist suppression; the next availability check imports and refreshes the same source again. Fresh evidence beyond the earlier normalization and unrelated-credential fixes is this deterministic token transformation during import; retain the original import hash as credential provenance so a rejection can suppress its actual source.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Classify probe authentication failures as definitive

When catalog discovery succeeds but the subsequent /responses probe returns 401, this throws an ordinary error without definitiveAuthFailure. The provider handlers therefore invalidate the global catalog only if its serialized models or availability changed; with an unchanged API-key catalog, the health check reports unhealthy but leaves the rejected provider in models.list. Fresh evidence beyond the earlier discovery-401 finding is that this newly added probe is a separate authentication-failure path; classify its 401 as definitive so the picker is invalidated.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Clear stale unavailability when credentials are updated

When a provider such as Copilot is logged out, its snapshot is set to false; installing replacement credentials through providers.update or re-enabling the provider calls syncProviderToRegistry() but never clears that snapshot. Fresh evidence beyond the fixed auth.login path is the credential-hydration route in provider-handlers.ts, so an immediate synchronous inferProviderForModel() call still excludes the newly authenticated owner until asynchronous model discovery runs and can route a Space agent to another provider.

Useful? React with 👍 / 👎.

[lsm]

Closing: provider discovery and model curation landed generically (#3253, #2847 through #2886) and this branch is 1,100+ commits behind with 220 review comments. Re-slice from dev if live Codex catalog fetch is still wanted. Branch retained.