🔌 feat: OpenAPI spec and docs for the public Agents API - #15928
Conversation
|
👋 Thanks for the contribution! LibreChat merges all changes into Nothing is needed from you; your commits, reviews and discussion are unchanged. If the diff now shows files you did not touch, rebase onto git remote add upstream https://github.com/danny-avila/LibreChat.git
git fetch upstream dev
git rebase upstream/dev
git push --force-with-leaseMaintainers: apply the |
|
@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. |
![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: d06bcd7539
ℹ️ 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".
d06bcd7 to
7f82260
Compare
- adapter: mark JSON request bodies as required - openapi.js: move gate, config read and spec serving into packages/api as an injected router factory; leave /api as wiring only - docs: compute URLs in-browser and use a relative servers url so docs work under a base path (e.g. /chat) - build: copy the generated spec into dist so the multi-stage image ships it - agents: reuse the enforced list envelope for the docs so the AgentList schema cannot drift from the runtime cursor and strictness
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dda9176ed1
ℹ️ 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".
Second review round on the public Agents API spec. Three cases where the
published contract described something the server does not do, so generated
clients would fail against the live API:
- Upload form: document the `purpose` field (enum: file_search, execute_code,
context) that POST /agents/{id}/files actually requires, instead of the
internal-only `tool_resource`; requests following the old form were rejected
with 400 because `purpose` was absent.
- Auth: drop the `apiKeyBearer` scheme from the management and skill endpoints.
They gate on OIDC/M2M via requireAgentManagementAuth and deliberately reject
the API-key fallback; the API key belongs to the out-of-scope
OpenAI-compatible endpoints and returns when those are documented.
- Skill update: document the 409 conflict PATCH /skills/{id} returns on a stale
expectedVersion. The agent update path collapses its internal 409 to 400, so
it correctly keeps no 409.
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e97fc6d47
ℹ️ 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".
Third review round on the public Agents API spec.
- Experimental server: mount routes.openapi in api/server/experimental.js before its /api 404 handler. Without it, npm run backend:experimental returns 404 for /api/docs and /api/openapi.json even when openapi.enabled is true, because that server never mounted the router.
- Auth failure: document the 401 that requireAgentManagementAuth returns on a missing, expired, malformed, or unbound token. Its body is { "error": "Unauthorized" }, a flatter shape than the management error schema, so add a dedicated UnauthorizedError schema and a shared 401 response to the agent and skill contracts rather than reusing the standard error schema.
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cafdc81b16
ℹ️ 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 again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82cf5e59b2
ℹ️ 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".
| const errorResponses = [ | ||
| { status: 400, description: 'Invalid request', schema: agentManagementErrorSchema }, | ||
| { status: 401, description: 'Authentication failed', schema: unauthorizedResponseSchema }, | ||
| { status: 403, description: 'Permission denied', schema: agentManagementErrorSchema }, |
There was a problem hiding this comment.
When BAN_VIOLATIONS is enabled and the authenticated principal or IP is banned, checkBan runs before every documented Agent and Skill handler and returns 403 with the flat body { "message": ... }. The contract instead promises the nested agentManagementErrorSchema, so generated response validation fails specifically for banned callers; make the 403 schema accept the ban envelope as well.
Useful? React with 👍 / 👎.
…rrors in API responses docs
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eefd7bbb64
ℹ️ 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 again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25c092fbec
ℹ️ 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".
…ling in isEnabled function
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dd0975dfb
ℹ️ 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 again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdc90b768e
ℹ️ 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 again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cbefb3df3
ℹ️ 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".
| status: 500, | ||
| description: 'Internal server error', | ||
| schema: z.union([agentManagementErrorSchema, errorMessageResponseSchema]), |
There was a problem hiding this comment.
When both configuration reads fail in configMiddleware—which runs for every Skill operation and for Agent create, update, and upload—the middleware passes the fallback error to ErrorController, which returns a text/plain 500 body such as An unknown error occurred. This contract declares only application/json with one of two object envelopes, so generated clients can fail to deserialize the response during a configuration-store outage; include the plain-text failure representation in both shared 500 definitions or normalize this middleware path to the documented envelope.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed reachable — the double config-read failure in configMiddleware calls next(fallbackError) with a raw error that has no statusCode/body, so ErrorController's isCustomError guard is false and it falls to res.status(500).send('An unknown error occurred.') (text/plain). But this isn't specific to the config path or to these operations: it's the generic tail for any uncaught throw, and a >3 MB body lands here too (the 413 http-errors object also lacks body, so it collapses to the same plain-text 500). So it's a global unhandled-failure representation, not a per-endpoint envelope.
That plain-text 500 is one of several error-body shapes this API emits with no shared envelope, and the contract builder emits a single media type per response — documenting a text/plain variant on every operation's 500 would entrench exactly the inconsistency that belongs at the middleware layer. We're deferring this to the error-envelope normalization already raised with the maintainer (normalize the unhandled-failure path onto the documented JSON envelope) rather than annotating the plain-text shape per endpoint. Tracking it there, not fixing it in this docs PR.
…o clarify byte limit
|
@codex review again |
|
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". |
| @@ -16556,6 +16557,13 @@ | |||
| "dev": true, | |||
| "license": "MIT" | |||
| }, | |||
| "node_modules/@scarf/scarf": { | |||
There was a problem hiding this comment.
why is scarf being added?
There was a problem hiding this comment.
it's a transitive dep of swagger-ui-dist, not something we pull in directly, and it's not a runtime telemetry vector. What the search shows:
- swagger-ui-dist's entry (index.js) requires only the UI bundle, standalone preset, and path helper. it does not require scarf package. I let ai do A tree-wide grep for scarf and finds zero runtime imports in swagger-ui-dist, api/server, or packages/api/src. Scarf's only trigger is its postinstall script, so it fires at install/build time, never at server runtime. so I don't know if that is acceptable or not. We can disable the telemetry though.
The drift check added by #15928 fails on dev: #16008 added `repositoryInstructions` to the agent schema without regenerating the committed spec, so every branch merging dev inherits the failure. This is the remedy the check itself prescribes and carries no hand-written change. Drop this commit if dev regenerates first.
…ns` (#16029) Regenerates packages/api/openapi/agents.openapi.json from the code so openapi:check passes again. #16008 added repositoryInstructions to the agent schema and merged before #15928, whose committed spec was generated from a branch that predated the field, leaving dev red. Co-authored-by: Lia <lia@librechat.ai>
* fix: Serialize MCP OAuth token refresh across replicas `MCPTokenStorage.inflightRefreshes` coalesces refresh-token redemptions inside one Node process. Behind a load balancer without session affinity, one user's concurrent requests land on different replicas, each reads the same not-yet-rotated refresh token and redeems it. RFC 9700 servers treat the second redemption as replay and revoke the whole grant family, so the user is asked to authorize the MCP server again. `forceRefreshTokens` now takes a cross-replica refresh flight before redeeming, using the same `FlowStateManager.acquireLease` primitive the OAuth teardown fence uses under a distinct key. A replica that waited for the flight adopts the tokens the holder rotated instead of redeeming again; when the flight is still held after the wait window it falls back to the unfenced redemption every earlier release performed. * fix: Redeem unfenced when the refresh flight lease store fails * fix: Fence MCP refresh waiters instead of redeeming unfenced A waiter that could not take the cross-replica refresh flight fell through to an unfenced redemption after 10s, while the holder's own stale abort does not fire until 60s. A refresh taking 11 to 60 seconds therefore still let two replicas redeem one refresh token, the replay this fence exists to prevent. The waiter now polls until it either adopts the tokens the holder rotated or acquires the flight, and fails as MCPTokenRefreshUnavailableError rather than redeeming beside a live holder. `getTokens` callers defer connection recovery on that error, and the stored credential is left intact for a later attempt. The flight is keyed by the stored credential (tenant, user, server name) rather than the caller's OAuth binding digest, so a rolling config change cannot hand two replicas different locks over one stored token. The wait is an operator lever, `oauthRefreshWaitTimeout`, clamped to half the stale window. An adoption read that fails no longer gives up the held flight. * fix: Annotate derived refresh-flight constants for isolatedDeclarations * fix: Recapture the credential generation a peer published on adoption * fix: Hold the MCP refresh flight until redemption settles Four corrections to the cross-replica refresh flight. The flight lease equalled the window that aborts a stalled redemption, on the claim that an expired flight could never belong to a redemption still able to reach the token endpoint. Aborting proves no such thing: the request may have been processed with its response lost, and a stalled event loop can delay the abort past its own deadline. The lease now outlives the abort, so a peer cannot redeem a credential the provider has already rotated. A live replica still releases on settle, so the margin is paid only by one that died holding the flight. The credential snapshot was taken after the first failed acquisition, so a holder that stored and released in that gap was snapshotted post-rotation: the next attempt saw an unchanged record and redeemed the credential it should have adopted. It is taken before the first attempt now. `oauthRefreshWaitTimeout` accepted zero while the runtime mapped every non-positive value to the default, so the config validated and then behaved contrary to its value. Zero is rejected at load. Adoption announced a second credential change through `handleOAuthRefreshSuccess`, whose `onOAuthCredentialsChanged` advances authorization state after persistence this replica did not perform. That retired the generation recaptured beside it and fenced the build against its own tool publication. Adoption now updates the local token-flow cache and recaptures, without announcing. * fix: Check every held MCP refresh flight for a peer's rotation Four corrections, two of them consequences of moving the credential observation ahead of the first lease attempt. That move created a storage read inside `beginRefreshFlight`, and its failure reached a handler written for a lease-store outage, so a transient read error became an unfenced redemption beside a live peer. The flight now handles its own reads: losing the observation costs adoption and nothing else, and only the lease store failing may redeem unfenced. The observation was also never compared when the first acquisition succeeded. A peer that rotated and released before this replica contended left an acquisition that looked uncontended, and its fresh credential was redeemed a second time. Every acquisition now runs one rotation check, which also collapses two code paths into one. `getTokens` already loads the refresh record to decide a refresh is needed, so it is passed on as the observation baseline instead of read again, and the read taken under the flight is reused as the credential redeemed. Two reads on a latency-counted path where there were three. `runSilentRefresh` collapsed contention into null, sending the 401 path to interactive OAuth, so a slow peer prompted the user to authorize a server whose credential was about to be valid. The retryable outcome now travels through the silent-refresh layers and the connection defers, matched by name as well as identity because these errors cross the package boundary. * chore: Regenerate the Agents OpenAPI spec for repositoryInstructions The drift check added by #15928 fails on dev: #16008 added `repositoryInstructions` to the agent schema without regenerating the committed spec, so every branch merging dev inherits the failure. This is the remedy the check itself prescribes and carries no hand-written change. Drop this commit if dev regenerates first. * fix: preserve MCP refresh outcomes across replica boundaries * fix: keep MCP adoption fenced through publication * fix: anchor MCP refresh adoption to rejected credentials * fix: Complete OAuth Adoption Adapters and Isolate Legacy Flow Readers * fix: Preserve OAuth Request Identity Through Recovery and Discovery * fix: Fence OAuth Adoption Against All Credential Writers * fix: Gate Coordinated OAuth Rollout and Preserve Unauthenticated Identity * fix: Preserve OAuth Coalescing Across the Staged Rollout * style: Sort OAuth Timeout Constant Import * fix: Invalidate Both OAuth Token Flow Protocols on Rotation * test: Complete the Rollback Token Flow Fixture --------- Co-authored-by: Lia <lia@librechat.ai> Co-authored-by: Danny Avila <danny@librechat.ai>
2 participants
Summary
The public Agents API has no machine-readable contract. Clients cannot generate a library from it, and nothing checks that the docs match the code. This adds an OpenAPI 3.1 spec for the 14 agent and skill management endpoints, generated from the Zod schemas the server already validates against. The spec is committed, served behind a config switch, and checked in CI so it cannot drift from the code. No route is rewired and no runtime behavior changes.
This is the first pull request. Tests, lint, and the breaking-change check are follow-ups.
How it works
A small registry describes each endpoint and points at the same Zod schema the route validates against. A generator turns the registry into the spec. One file (
adapter.ts) imports the Zod-to-OpenAPI converter, so the converter is easy to replace.The serving behavior lives in
packages/api(router.ts, a router factory that takes its config reader and asset path as arguments); the/apiroute file only wires it up. The artifact is copied intodistat build time, so it ships in the multi-stage Docker image, and the docs use relative URLs so they work under a deployment base path (e.g./chat). Both routes return 404 unless the switch is on:One field cannot be expressed in OpenAPI:
edges[].promptaccepts a function at runtime. The document layer replaces only that field with a string variant. The runtime schema is untouched.Change Type
Testing
librechat.yaml, then restart the backend:http://localhost:3080/api/docsand confirm Swagger UI renders the 14 endpoints.http://localhost:3080/api/openapi.jsonand confirm the spec loads.enabled: false(or remove the block), restart, and confirm both routes return 404.npm run -w @librechat/api openapi:check.Test Configuration:
openapi:generateandopenapi:checkverified.tsc --noEmitclean forpackages/apiandpackages/data-provider.Checklist