Repository navigation
fix(models): project drifted discovery reads instead of answering catalog_busy - #6333
Conversation
…licy File-backed inventory drift rejected ordinary model reads with catalog_busy. Identical synthetic inventory succeeds on the same cached roster, ruling out row replacement and cache revision churn in the reproduction. Allow read callers to receive a detached new-arrival policy projection. Management renders disabled arrivals; other shared-fetch consumers receive only visible projected rows. Keep retained-sync and direct stale-writer refusal contracts, inventory/revision checks, and persisted merge baselines intact. Verification: eight authorized files, 80 passed / 0 failed; after adding save-safety coverage, runtime-policy and ratchet files, 24 passed / 0 failed. Typecheck and structure checks pass. Full suite and docs build were excluded by the delegated command restrictions.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
|
Maintainer integration into |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7cdebebd5
ℹ️ 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".
| if (publicationConfig !== config && !publishProjection) { | ||
| // Other consumers keep their live config, so only return rows visible under the projection. | ||
| const { filterCatalogVisibleModels } = await import("../../codex/catalog"); | ||
| return filterCatalogVisibleModels(models, publicationConfig); |
There was a problem hiding this comment.
Preserve unfiltered rosters for configuration endpoints
When a file-backed configuration already differs from disk, this filters the shared fetch result using every disabledModels entry and selectedModels allowlist, not only the newly projected arrivals. However, /api/selected-models explicitly uses fetchAllModels to construct its full unfiltered available roster, and the model-preset routes use it to calculate complete candidate sets. During persistent live/disk drift, those endpoints therefore omit models the user needs to re-enable and can apply a preset from a truncated roster; pass the detached config separately to visibility consumers while retaining raw rows for configuration callers.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
Summary
Cross-platform CI on
dev(run 36734548914) failed on Linux, Windows and macOS with one test:tests/codex-integration/model-visibility-management-api.test.ts"manual models replace management rows…" threwCatalogGatherBusyError. The cause is #6331. It marks everyloadConfig()object as file-backed and refuses discovery publication once such a config differs from disk. A management read on a config that differed from disk (unsaved in-memory edits, or another writer changed the file) therefore answeredcatalog_busy503. In production, the dashboard model list would keep failing until the config reloaded.Read consumers now opt into a detached projection when a file-backed config has already drifted. The new-arrival policy is applied to a copy of
modelDiscovery/disabledModels. Management renders new arrivals as disabled from that copy, and other fetch consumers receive only rows visible under it. The live config, disk and save baselines stay untouched, so the #6331 protection holds: no unsaved disable can later be persisted over an operator's on-disk choice. Writers (retained sync) still refuse stale publication.Verification
bun testover model-visibility-management-api, new-model-policy-runtime, new-model-policy, server-new-model-policy-arrival, model-export-new-model-policy, codex-sync-new-model-policy, file-size ratchet and test-layout: 80 pass / 0 fail.bun x tsc --noEmitandbun run structure:checkpass.devtip after this merge.Checklist