Repository navigation
🗑️ feat: Expose Skill Deletion in Management API - #17005
dustinhealy merged 6 commits into
Conversation
|
@codex review |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ebf9d9053
ℹ️ 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. You're on a roll. Reviewed commit: ℹ️ 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ 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". |
c22f847 to
9483cd7
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The OpenAPI response incorrectly makes the guaranteed cleanupComplete field optional.
1 open finding
What changed in this PR
Adds machine-authenticated whole-Skill deletion with existing authorization, tenant isolation, and cleanup behavior.
Changes:
- Adds and documents
DELETE /api/agents/v1/skills/{id}. - Normalizes deletion responses and hides storage details.
- Adds handler, routing, authorization, and OpenAPI contract tests.
| File | Description |
|---|---|
packages/api/src/skills/management.ts |
Adds authorized deletion adapter and response schema. |
packages/api/src/skills/management.spec.ts |
Tests deletion, permissions, isolation, and cleanup responses. |
packages/api/src/openapi/skills.ts |
Registers the deletion contract. |
packages/api/openapi/agents.openapi.json |
Adds generated endpoint and response definitions. |
docs/skills-management-api.md |
Documents deletion and incomplete cleanup behavior. |
api/server/routes/agents/skills.js |
Registers the DELETE route. |
api/server/routes/agents/__tests__/skills.spec.js |
Tests route dispatch and authentication ordering. |
api/server/routes/agents/__tests__/openapi.contract.spec.js |
Validates the live HTTP deletion contract. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c2a94b038
ℹ️ 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. 🚀 Reviewed commit: ℹ️ 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". |
6c2a94b to
226522d
Compare
This reverts commit 2a3c2da.
226522d to
9fdaef1
Compare

Summary
The machine-authenticated Skills management API can create and update Skills, but it cannot delete a whole Skill. Callers must switch to the browser API to complete that lifecycle. This PR adds
DELETE /api/agents/v1/skills/{id}for inline Skills, using the existing browser deletion handler and requiring the same DELETE resource permission, Skills mutation role permissions, and tenant boundary as other management mutations.This PR is stacked on #16878 and should be retargeted to
devafter that PR merges. A follow-up PR will add durable deletion cleanup; merge both before including this route in a release.How it works
The handler retains its current cleanup behavior. A response with
cleanupComplete: falsemeans the Skill was removed but dependent cleanup did not finish; this PR does not add durable cleanup retries.Type of change
Testing
Tested environments/configuration: Local Node workspace with the repository's disposable MongoDB test server and machine-authenticated contract fixture.
Automated tests: Focused management handler, Express route, and HTTP contract tests;
npx tsc --noEmitin the changed TypeScript workspaces;openapi:check; affected static checks; Lighthouse on port 3098 (passed, median LCP 3.61 s against the 4.5 s budget). The first Lighthouse attempt could not start because port 3080 was occupied.Screenshots / recordings
No user-facing UI change.
Risk / compatibility
This adds a management route without changing the underlying cleanup semantics or adding a migration. Only inline Skills can be mutated through this API. The known incomplete-cleanup result remains visible to callers and is documented rather than being presented as a guaranteed retry. The dependent durable-cleanup PR must be merged before a release containing this route is deployed; old instances and in-flight uploads should be drained before invoking whole-Skill deletion through the management API.
Checklist