[codex] allow shared config reads in app-server queue#21340
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed7a129ecc
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed7a129ecc
ℹ️ 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".
|
@codex review, skip comments any issues that you already commented |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
skills/list,config/readandplugin/listas shared reads for nowWhy
skills/listandplugin/listare read-only config-family requests, but the app-server queue currently treats every config request as exclusive. That means one longskills/listcan make a laterplugin/listwait even though the two requests do not mutate config.This change keeps the existing queue order but lets adjacent reads overlap. If a write is already waiting, later reads still stay behind it, so writes do not starve.
Scope
This intentionally keeps the first pass narrow:
skills/list,plugin/listplugin/install,marketplace/*,skills/config/write,config/*write,config/read, and the rest of the config familyValidation
just fmtcargo test -p codex-app-server-protocolcargo test -p codex-app-serverjust fix -p codex-app-server-protocoljust fix -p codex-app-serverDesktop verification
I ran the dev desktop app against this branch's built binary with the existing UI timing logs enabled. The app did use
/Users/xli/code/codex_6/codex-rs/target/debug/codex.The new scheduler behavior works, but this narrow change does not remove every cold-start delay: in the observed trace, an earlier exclusive
config/readwas already queued ahead of the laterskills/listandplugin/listrequests, so the page-open plugin requests still waited behind that earlier exclusive config-family request before they could run together.That means this PR is the scheduler primitive needed for shared reads, not the complete end-to-end latency fix by itself.
Not run
app-server-protocol