[codex] Add marketplace remove command and shared logic#17752
Conversation
xli-oai
added
the
oai
ChatGPT Codex Connector
xli-oai
changed the title
xli-oai
force-pushed
the
xli-codex/marketplace-remove-shared
branch
from
8ca2338 to
908d13c
Compare
|
@codex review |
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 856511e999
ℹ️ 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 !root.exists() { | ||
| return Ok(None); |
There was a problem hiding this comment.
remove_marketplace_root uses Path::exists(), which returns false on metadata errors (e.g. permission denied). That path is then treated as “not installed”, and removal may still report success after config edit, leaving undeleted marketplace files. Use try_exists() and surface I/O errors instead of silently skipping removal.
Useful? React with 👍 / 👎.
| if !root.exists() { | ||
| return Ok(None); |
There was a problem hiding this comment.
remove_marketplace_root uses Path::exists(), which returns false on metadata errors (for example, permission denied). The command can then skip deletion and still report success after config removal, leaving marketplace files behind. Use try_exists() so I/O failures are surfaced as errors instead of being treated as “not installed”.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
will be a todo, very unlikely
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e53c56038d
ℹ️ 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 !root.exists() { | ||
| return Ok(None); |
There was a problem hiding this comment.
remove_marketplace_root checks root.exists() before symlink_metadata. exists() follows symlinks, so a dangling symlink under .tmp/marketplaces/<name> is treated as missing and never removed. This leaves corrupt install entries behind, and removal can incorrectly return "not configured or installed" when only a broken symlink remains.
Useful? React with 👍 / 👎.
2 participants
Summary
Move the marketplace remove implementation into shared core logic so both the CLI command and follow-up app-server RPC can reuse the same behavior.
This change:
codex_core::plugins::remove_marketplace(...)flowValidation
just fmt