[codex] Refresh Codex cache after provider changes - #3
Closed
Ingwannu wants to merge 1 commit into
Closed
Conversation
Intent: keep Codex Desktop model pickers in sync immediately after GUI provider changes. Root-cause: adding a provider through /api/providers updated opencodex config but did not rebuild the Codex catalog or invalidate models_cache.json, so Codex could keep showing its stale native-only cache. Changed: centralize catalog refresh/cache invalidation, call it from CLI sync/start and GUI model-affecting management endpoints, and invalidate any time a catalog file exists. Tested: bun run typecheck; bun test tests/codex-refresh.test.ts tests/codex-catalog.test.ts tests/codex-inject.test.ts; bun test tests.
Ingwannu
marked this pull request as ready for review
June 20, 2026 12:49
lidge-jun
approved these changes
Jun 20, 2026
lidge-jun
left a comment
Owner
There was a problem hiding this comment.
LGTM — squash-merged onto dev.
코어 로직 정확하고 버그 수정 올바릅니다. 몇 가지 minor suggestion:
1. Redundant delete-then-write (codex-refresh.ts:44-45)
invalidateCodexModelsCache() → syncCodexModelsCacheFromCatalog() 순서로 호출하는데, atomicWriteFile이 이미 기존 파일을 원자적으로 교체하므로 사전 삭제가 불필요합니다. 삭제~쓰기 사이 Codex Desktop이 missing cache를 읽을 수 있는 미세한 race도 생깁니다. invalidateCodexModelsCache() 호출을 제거하고 syncCodexModelsCacheFromCatalog만 유지하면 됩니다.
2. Double disk read (codex-refresh.ts:33-34)
syncCatalogModels가 방금 카탈로그를 디스크에 썼는데, syncCodexModelsCacheFromCatalog에서 다시 readFileSync로 읽습니다. syncCatalogModels 반환값으로 content를 전달하면 I/O를 줄일 수 있습니다 (minor).
둘 다 correctness 이슈는 아니므로 blocking 아닙니다. 좋은 PR 감사합니다!
lidge-jun
added a commit
that referenced
this pull request
Jun 20, 2026
Rebuild Codex catalog after GUI provider additions (not only removal/toggle). Add shared catalog refresh helper that overwrites models_cache.json from the materialized catalog. Keep ocx start alive after startup sync; start GUI proxy in a detached process. Co-authored-by: Ingwannu <Ingwannu@users.noreply.github.com>
Owner
|
Merged into main via squash merge (commit 0157ffe). Released as v2.0.1 → v2.0.2. |
lidge-jun
added a commit
that referenced
this pull request
Jul 12, 2026
…p audit BLOCKER fixes: - outbound.ts: detached pump pattern for SSE streaming — frames now reach the client as they arrive instead of buffering until upstream EOF; cancel calls reader.cancel() instead of the locked stream (#1) - system-env.ts: transactional launchctl injection with incremental tracking and rollback on mid-flight failure (#2) MAJOR fixes: - system-env.ts: POSIX single-quote escaping for all user-controlled values in the generated shell file, preventing command injection (#3) - system-env.ts: exported applySystemEnvToggle for runtime ON/OFF lifecycle (#7) - claude-messages.ts: 120s hard timeout on native passthrough upstream fetch using AbortSignal.timeout, returning Anthropic 504 on expiry (#4) - management-api.ts: reject non-plain-object PUT bodies (null, array, primitive) with 400; strict validation for blockedSkills, tierModels, modelMap, and autoCompactWindow (integer, range 100k-1M) (#5, #6) - management-api.ts: call applySystemEnvToggle after systemEnv config toggle (#7) - cli/claude.ts: override stale loopback ANTHROPIC_BASE_URL with live port; warn on gateway-cache and agent-sync failures instead of swallowing (#8, #10) - gateway-cache.ts: distinguish discovery failure (keep cache) from authoritative empty success (write {models:[]}) (#9) Test fixes: updated assertions for single-quote shell escaping, empty cache semantics, validation messages, incremental tracking writes, and PUT response shape.
3 tasks
dev-shinyu
added a commit
to dev-shinyu/opencodex
that referenced
this pull request
Jul 24, 2026
…row fallback Four fixes from the joint post-implementation audit (terra-high + qwen-ultimate), all verified against the code before applying: - lidge-jun#1 (blocker): when an auto-restart fired, tick()'s restart hook synchronously nulled the global `running` (defaultRestart calls stopMemoryWatchdog before draining), and probeOnce's trailing `scheduleNextProbe(generation, running.cfg.intervalMs)` then evaluated `running.cfg` on null — an unhandled TypeError at the exact moment of every real auto-restart (surfacing as crash-guard noise, and a dead loop for injected restart hooks). probeOnce now pins its instance in a local `current` and re-checks identity against the global after capture, after tick, and before rescheduling: a stopped/replaced watchdog is left untouched and never rescheduled — the restart owns the exit. - lidge-jun#2: an unexpected exception escaping capture() no longer silences the whole cycle; probeOnce evaluates exactly once on a new exported rssFallbackSnapshot(now, "capture-threw") instead of skipping evaluation. - lidge-jun#3: the PowerShell probe now emits labeled values (P=/C=/L=/A=) parsed by the new pure parseWindowsProbeOutput() — a $null first value previously rendered as a blank line that trim() removed, shifting positional indexes and misassigning commit values to privateBytes. Order-independent, loss-explicit, unit-tested; first-resolution-wins/timeout/kill handling untouched. - lidge-jun#4: applyWatchdogRuntimeConfig now finite-guards systemCommitHighWater and maxRestarts (same pattern as resolveWatchdogConfig): a programmatic partial carrying undefined/NaN would otherwise make `fraction >= NaN` permanently false and silently disable the commit warning. Regression tests: auto-restart-stops-watchdog mid-tick (validated to fail on the pre-fix code via bun's unhandled-rejection surfacing), capture-throw RSS fallback, NaN/undefined live re-clamp, and label-parser unit cases. 77 pass.
This was referenced Jul 26, 2026
tizerluo
pushed a commit
to tizerluo/opencodex
that referenced
this pull request
Jul 28, 2026
…aliases, DNS noun Address Wibias fourth CHANGES_REQUESTED review: 1. Deadline abort classification (required lidge-jun#1): - Replace 0.95 wall-clock heuristic with signal.reason.name === 'TimeoutError' - deadlineSignal declared outside try so catch can check deadlineSignal.aborted - Submit catch returns timeout error when deadlineSignal fires (not generic abort) 2. Don't deliver pruned artifact paths (required lidge-jun#2): - After batch prune, scan fulfilled results and rewrite any whose path no longer exists to ok:false with explanatory error 3. Wire video aliases (required lidge-jun#3): - planVideoBridge seeds toolNames with VIDEO_GEN_TOOL_NAME + any request function tools matching isVideoGenName (image-bridge parity) - core.ts filter only strips unnamespaced video_gen aliases (namespaced MCP tools are left alone) 4. Fix test fixtures (required lidge-jun#4): - authMode 'api_key' → 'key' (valid OcxProviderConfig value) 5. DNS error noun (small): - resolvePublicAddresses accepts optional noun param (default 'image') - downloadVideoToArtifact passes 'video'
Wibias
added a commit
that referenced
this pull request
Jul 29, 2026
* feat(videos): add Grok video bridge for non-OpenAI models
Extends the image bridge to support asynchronous video generation via xAI
Grok Imagine Video. Video generation uses a submit→poll→download pattern
(POST /v1/videos/generations → GET /v1/videos/{id}) with heartbeat forwarding
to keep the SSE stream alive during the 30-180s generation window.
New files:
- src/images/xai-video-client.ts: submitVideoJob + pollVideoJob
- src/images/fulfill-video.ts: arg parsing, async polling generator, result builder
- tests/videos/: 32 tests (xai-video-client, fulfill-video, plan-video)
- docs-site/.../video-bridge.md: user guide
Modified files:
- src/images/loop.ts: unified image+video fulfillment, per-turn call caps
- src/images/types.ts: VideoBridgePlan/VideoCallResult shared types
- src/images/synthetic-tool.ts: buildVideoTool, VIDEO_GEN_TOOL_NAME
- src/images/plan.ts: planVideoBridge (opt-in via videoBridgeEnabled)
- src/images/artifacts.ts: downloadVideoToArtifact (200MB cap, SSRF-protected)
- src/server/responses/core.ts: video bridge wiring + tool dedup
- src/types.ts: videoGeneration flag, video config fields
Video bridge is opt-in (videoBridgeEnabled defaults to false). All upstream
image-bridge hardening preserved (SSRF, size caps, abort linking, pinned HTTPS).
* fix(videos): address Codex P1/P2 + CodeRabbit review findings
P1 fixes:
- Non-streaming requests no longer 400 when video bridge is enabled
(only image bridge requires stream=true; video-only skips bridge for
non-stream requests)
- Restore globalThis.fetch after video client tests (afterEach cleanup)
P2 fixes:
- Use clamped vidPlan.timeoutMs instead of raw config value
- Honor videoMaxRounds when both image+video bridges active (tighter wins)
- guessVideoExtFromMagic throws on unrecognized magic (was defaulting to mp4)
- Poll 4xx permanent failures (except 429) fail fast instead of retrying
- buildVideoResult uses pathToFileURL for cross-platform markdown links
- Initial poll interval restored to 5s (was 200ms)
- done-without-videoUrl returns error instead of spinning until timeout
- VideoBudget charges every streamed chunk (was only first chunk)
- downloadVideoToArtifact: reader cleanup on file-open failure
- readBoundedText: cancel reader body on size cap breach
- Namespaced video_gen tools preserved in dedup filter
- Fix JSON missing closing brace in docs
- Clarify API key auth requirement in docs prerequisites
* fix(videos): encode requestId, defer paidVideoCalls, guard plan! assertion
- Encode requestId in poll URL to prevent path injection (CodeRabbit)
- Move paidVideoCalls++ past arg validation so malformed calls don't burn budget
- Add early guard for undefined plan in image branch, removing non-null assertions
* fix(videos): batch-aware artifact pruning + clarify provider key docs
- Move pruneArtifacts from per-download to post-batch so multi-video turns
don't delete earlier videos before tool results are injected (Codex P2)
- Document that video bridge requires providers.xai with apiKey authMode,
not OAuth from 'ocx login xai' (Codex P2)
* fix(videos): web search coexistence, unified timeout budget, VideoBudget ceiling
Address Wibias CHANGES_REQUESTED review:
1. Web search coexistence (P1):
- When wsPlan is active, media bridge is skipped (existing behavior)
- Now emits console.warn so the user sees the skip instead of silent loss
- Documented priority rule in video-bridge.md
2. Timeout budget (P2):
- Start deadline BEFORE submitVideoJob, not after
- Poll receives remaining budget (deadline - now), min 5s floor
- Submit (60s) + poll now share one videoTimeoutMs deadline
3. VideoBudget aggregate ceiling (optional, addressed):
- VideoBudget now has a cap (600 MiB = 3 × single-download max)
- chargeVideoBudget() enforces the ceiling per-chunk during streaming
- Exceeding the budget throws mid-download (partial file is cleaned up)
4. New tests: done-without-videoUrl, permanent 4xx poll stop, requestId encoding
* fix(videos): gate tool_choice on imgPlan, shared deadline signal, buffer magic sniff
Address Wibias second CHANGES_REQUESTED review:
Required:
B. Gate image tool_choice rewriting on imgPlan — video-only turns no longer
rewrite image_generation/image_gen aliases to an undeclared tool (core.ts)
C. Shared timeout complete — deadline-bound AbortSignal passed into submitVideoJob;
if budget expired after submit, poll is skipped with timeout error (no 5s floor)
Preferred:
A. Buffer ≥12 bytes before magic-byte sniff — accumulate chunks until sniff minimum
so short first reads don't crash guessVideoExtFromMagic (artifacts.ts)
Optional:
D. Docs: scope web-search priority to runnable sidecar / non-runTurn path
* fix(videos): pass deadline-bound signal into poll generator
Pass linkedDeadline (not raw client signal) into pollVideoWithHeartbeats so
in-flight pollVideoJob fetches and sleep() calls abort when the wall-clock
budget expires. Abort error messages now distinguish deadline-expiry from
client-cancel based on elapsed time vs timeoutMs threshold.
* fix(videos): deadline abort classification, pruned-path guard, video aliases, DNS noun
Address Wibias fourth CHANGES_REQUESTED review:
1. Deadline abort classification (required #1):
- Replace 0.95 wall-clock heuristic with signal.reason.name === 'TimeoutError'
- deadlineSignal declared outside try so catch can check deadlineSignal.aborted
- Submit catch returns timeout error when deadlineSignal fires (not generic abort)
2. Don't deliver pruned artifact paths (required #2):
- After batch prune, scan fulfilled results and rewrite any whose path
no longer exists to ok:false with explanatory error
3. Wire video aliases (required #3):
- planVideoBridge seeds toolNames with VIDEO_GEN_TOOL_NAME + any request
function tools matching isVideoGenName (image-bridge parity)
- core.ts filter only strips unnamespaced video_gen aliases (namespaced
MCP tools are left alone)
4. Fix test fixtures (required #4):
- authMode 'api_key' → 'key' (valid OcxProviderConfig value)
5. DNS error noun (small):
- resolvePublicAddresses accepts optional noun param (default 'image')
- downloadVideoToArtifact passes 'video'
* fix(videos): complete pruned-path guard, skip namespaced aliases, mock sleep in tests
Address Wibias fifth CHANGES_REQUESTED review:
1. Finish pruned-path guard:
- Filter files[] through existsSync, not just path
- If all pruned: ok:false with no markdown/path
- If some survive: refresh path/files/count/markdown from survivors
2. Skip namespaced tools when seeding videoPlan.toolNames:
- if (t.namespace) continue — matches core.ts filter rule
3. Fix ubuntu CI timeout:
- Mock setTimeout in heartbeat test so 5s poll interval resolves instantly
- Suite now runs in ~156ms (was ~5s)
* fix(videos): stop hanging ubuntu CI with global setTimeout mock
cf76fb1 mocked globalThis.setTimeout to skip the 5s poll sleep. That races
parallel Bun workers and cancelled ubuntu-latest at the 12m job cap
(938f25e was green; cf76fb1 hung). Inject sleep/poll seams instead and drop
mock.module so video tests no longer poison each other.
---------
Co-authored-by: Your Name <your.email@example.com>
Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com>
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
models_cache.jsonfrom the materialized catalog when available.ocx startalive after the async startup sync completes, and starts the GUI proxy in a detached process whenocx guihas to launch it.Why
Adding a provider through the dashboard updated
~/.opencodex/config.json, but Codex Desktop could keep reading a stale~/.codex/models_cache.jsoncontaining only native GPT models. The catalog file could already contain routed provider models while the app still showed the old 5-model cache.Deleting the cache is not always enough for Desktop/App flows, so the refresh path now writes the same model catalog content into
models_cache.jsonas well.Validation
bun run typecheckbun test tests/codex-refresh.test.ts tests/codex-catalog.test.ts tests/codex-inject.test.tsbun test testsmodels_cache.jsonboth contained 16 models after Neuralwatt sync, andcodex debug modelsreported 16 models.