codex: address skills path review feedback#25098
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/openai/codex/blob/8a70c65eab09628f5ddc16ef26b9c37af2aa69ec/codex-rs/core-skills/src/manager.rs#L157-L163
When local exec is disabled, skills/list now passes env_path: None but still uses skills_for_cwd; because this cache key only stores None plus the local filesystem, all requested cwd values share one cached outcome when forceReload is false. In that remote-only/no-local-env context, two cwds with different skill config/plugin roots or enablement can return the first cwd's skills until a forced reload, despite skills/list being documented as cached per cwd.
ℹ️ 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".
starr-openai
changed the base branch from
starr/environment-path-ref-core
to
main
starr-openai
force-pushed
the
starr/environment-path-ref-review-fixes
branch
from
8a70c65 to
b338a20
Compare
starr-openai
force-pushed
the
starr/environment-path-ref-review-fixes
branch
from
b338a20 to
d51337b
Compare
## Summary - add public `codex_exec_server::EnvironmentPathRef` - bind an absolute path to its owning executor filesystem - keep path operations in the next review slice ## Stack - 1/5 in the skills path authority stack extracted from #25098 ## Validation - `cd /Users/starr/code/codex-worktrees/pr-25098-restack4/codex-rs && just fmt` - GitHub CI pending on rewritten head
## Summary - add executor filesystem canonicalization as a bound-path operation - route remote canonicalization through the exec-server filesystem RPC surface - keep path normalization attached to the filesystem that owns the path ## Stack - 2/5 in the skills path authority stack extracted from #25098 - follows merged #25121 ## Validation - `cd /Users/starr/code/codex-worktrees/pr-25098-restack-review-pr1b/codex-rs && just fmt` - Not run: tests/checks (not requested) - GitHub CI pending on rewritten head
1 participant
Summary
EnvironmentPathRefand read injections through the bound source pathskills/list(environmentId)app-server supportTesting
cd codex-rs && just test -p codex-core-skillscd codex-rs && just write-app-server-schema && just test -p codex-app-server-protocolcd codex-rs && RUSTUP_TOOLCHAIN=1.95.0-aarch64-apple-darwin just test -p codex-app-servercd codex-rs && RUSTUP_TOOLCHAIN=1.95.0-aarch64-apple-darwin just test -p codex-core(failed locally after compiling: local core integration harness failures around missingtest_stdio_serverplus exec/MCP timeouts)cd codex-rs && RUSTUP_TOOLCHAIN=1.95.0-aarch64-apple-darwin just fix -p codex-core-skillscd codex-rs && RUSTUP_TOOLCHAIN=1.95.0-aarch64-apple-darwin just fix -p codex-tuicd codex-rs && RUSTUP_TOOLCHAIN=1.95.0-aarch64-apple-darwin just fix -p codex-core-skills -p codex-core-plugins -p codex-core -p codex-app-server-protocol -p codex-app-server; local process was terminated by signal 15 before completion without surfacing a branch diagnostic