Remove skills list extra roots#21485
Conversation
918933d to
9183997
Compare
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
codex/sdk/python/src/codex_app_server/generated/v2_all.py
Lines 3662 to 3667 in 8835b5b
The Rust v2 SkillsListParams and server handling no longer include perCwdExtraUserRoots, but the generated Python SDK still exposes and documents it. Python clients can keep sending this field and believe extra user roots are scanned, while the server now ignores it, so SDK behavior/documentation is out of sync with the API.
ℹ️ 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 |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ 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". |
43496a4 to
8835b5b
Compare
Summary
perCwdExtraUserRoots/SkillsListExtraRootsForCwdfrom theskills/listapp-server API.codex-core-skillsextra-root plumbing so skill scans are keyed by the normal cwd/user/plugin roots only.Validation
just write-app-server-schemajust fmtcargo test -p codex-app-server-protocolcargo test -p codex-core-skillsjust fix -p codex-app-server-protocoljust fix -p codex-core-skillsjust fix -p codex-app-serverjust fix -p codex-tuiNotes
cargo test -p codex-app-server --test all skills_listran the edited skills-list cases, but the full filtered run ended on existingskills_changed_notification_is_emitted_after_skill_changetimeout after a websocket401.cargo test -p codex-tui --libcompiled the changed TUI callers, then failed two unrelated status permission tests because local/etc/codex/requirements.tomlforbidsDangerFullAccess.