Uh oh!
There was an error while loading. Please reload this page.
fix: register discovery endpoint at /api/v1/discovery - #621
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…/api/v1 Both RestServer and DispatcherPlugin now register discovery at the /discovery sub-path so that GET /api/v1/discovery returns the API discovery payload alongside the existing /api/v1 and /.well-known/objectstack endpoints. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Fixes a missing route registration so GET /api/v1/discovery no longer returns 404, aligning runtime behavior with Studio UI and expected discovery URL conventions.
Changes:
- Register the RestServer discovery handler on both
/api/v1and/api/v1/discovery. - Add a
/api/v1/discoverydiscovery route inDispatcherPluginalongside/.well-known/objectstack. - Update REST route assertion tests to expect the new discovery path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/runtime/src/dispatcher-plugin.ts | Adds GET ${prefix}/discovery route returning dispatcher discovery info. |
| packages/rest/src/rest-server.ts | Reuses a shared discovery handler and registers it at both basePath and ${basePath}/discovery. |
| packages/rest/src/rest.test.ts | Updates route assertions to include /api/v1/discovery and adjusts discovery-disable expectations. |
| server.get(`${prefix}/discovery`, async (_req: any, res: any) => { | ||
| res.json({ data: dispatcher.getDiscoveryInfo(prefix) }); | ||
| }); |
There was a problem hiding this comment.
The newly added ${prefix}/discovery route in createDispatcherPlugin isn't covered by tests. Since packages/runtime already has Vitest coverage for runtime/dispatcher behavior, please add a unit test that starts the plugin against a mocked IHttpServer and asserts the discovery route is registered (and, ideally, that it doesn’t conflict when @objectstack/rest is also present).
| server.get(`${prefix}/discovery`, async (_req: any, res: any) => { | ||
| res.json({ data: dispatcher.getDiscoveryInfo(prefix) }); | ||
| }); |
There was a problem hiding this comment.
createDispatcherPlugin now registers GET ${prefix}/discovery, but @objectstack/rest (RestServer) also registers GET /api/v1/discovery. In the default stacks (e.g. CLI serve and DevPlugin) both plugins are enabled, so this will double-register the same method/path on the same IHttpServer, which can cause route conflicts (some servers throw on duplicates, others have precedence rules) and can return a different payload shape than RestServer (Dispatcher returns HttpDispatcher.getDiscoveryInfo(...), RestServer returns protocol.getDiscovery()). Consider avoiding duplicate registration when com.objectstack.rest.api is present (e.g. via ctx.getKernel().getPlugins()), or make this handler delegate to the protocol discovery response to keep the payload consistent.
GET /api/v1/discoveryreturned 404 because no route was registered at that path. Discovery was only reachable at/api/v1(RestServer) and/.well-known/objectstack(DispatcherPlugin), but Studio UI and user expectations reference/api/v1/discovery.Changes
packages/rest/src/rest-server.ts—registerDiscoveryEndpoints()now registers the shared handler at bothbasePathand${basePath}/discoverypackages/runtime/src/dispatcher-plugin.ts— AddedGET ${prefix}/discoveryroute alongside/.well-known/objectstackpackages/rest/src/rest.test.ts— Updated route assertions for the new/discoverypathDiscovery endpoints after this change
/.well-known/objectstack/api/v1/api/v1/discovery💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.