Prioritize /api/v1/discovery over .well-known/objectstack per protocol#1151
Prioritize /api/v1/discovery over .well-known/objectstack per protocol#1151
Conversation
- Updated client to try /api/v1/discovery first - Fall back to .well-known/objectstack as secondary option - Updated integration test description to reflect new priority - Updated log messages to reflect primary vs fallback status Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/55becaf7-61f2-4a6a-bee3-0a27d1c16e03 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Updates the @objectstack/client discovery flow to match the ObjectStack protocol by attempting /api/v1/discovery first and only falling back to /.well-known/objectstack if needed.
Changes:
- Reordered
ObjectStackClient.connect()discovery probing to prioritize/api/v1/discovery. - Updated integration test case labeling to reflect
/api/v1/discoveryas the protocol-standard discovery method.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/client/src/index.ts | Reorders discovery probes and adjusts logging/error messages for primary vs fallback discovery. |
| packages/client/tests/integration/01-discovery.test.ts | Renames TC-DISC-001 description/title to indicate /api/v1/discovery as the primary discovery endpoint. |
| const discoveryUrl = `${this.baseUrl}/api/v1/discovery`; | ||
| this.logger.debug('Probing protocol-standard discovery endpoint', { url: discoveryUrl }); | ||
| const res = await this.fetchImpl(discoveryUrl); |
There was a problem hiding this comment.
discoveryUrl is built via string concatenation with baseUrl (${this.baseUrl}/api/v1/discovery). If baseUrl contains a path segment (e.g. https://host/some/prefix), this will probe .../some/prefix/api/v1/discovery, which is inconsistent with the .well-known logic (which correctly uses new URL(this.baseUrl).origin). Consider constructing the protocol-standard discovery URL using the parsed origin as well (and falling back to a root-relative /api/v1/discovery when baseUrl isn’t absolute).
| if (res.ok) { | ||
| const body = await res.json(); | ||
| data = body.data || body; | ||
| this.logger.debug('Discovered via /api/v1/discovery'); |
There was a problem hiding this comment.
In the primary /api/v1/discovery probe, non-2xx responses are silently ignored (only exceptions are logged). This makes it hard to diagnose cases like 401/404/500 where the request succeeds but returns an error status. Consider adding a debug log when res.ok is false (include status/statusText) so users can see why the primary discovery path was skipped.
| this.logger.debug('Discovered via /api/v1/discovery'); | |
| this.logger.debug('Discovered via /api/v1/discovery'); | |
| } else { | |
| this.logger.debug('Protocol-standard discovery endpoint returned non-success status', { | |
| url: discoveryUrl, | |
| status: res.status, | |
| statusText: res.statusText | |
| }); |
| describe('TC-DISC-001: Protocol-standard Discovery via /api/v1/discovery', () => { | ||
| test('should discover API from /api/v1/discovery', async () => { | ||
| const client = new ObjectStackClient({ | ||
| baseUrl: TEST_SERVER_URL, | ||
| debug: true | ||
| }); | ||
|
|
||
| const discovery = await client.connect(); |
There was a problem hiding this comment.
This test was renamed to claim protocol-standard discovery via /api/v1/discovery, but it doesn’t assert that connect() actually used that endpoint (it would still pass if the server only supports the .well-known fallback). Consider wrapping fetch with a spy in the test to assert the first attempted URL is /api/v1/discovery, and add a companion test that forces /api/v1/discovery to fail and verifies fallback to /.well-known/objectstack.
The client was attempting discovery in the wrong order, trying
.well-known/objectstackfirst and falling back to/api/v1/discovery. The protocol specifies that/api/v1/discoveryshould be the primary endpoint.Changes
Client discovery logic (
packages/client/src/index.ts)/api/v1/discoveryfirst (primary, protocol-standard)/.well-known/objectstackif primary failsIntegration tests (
packages/client/tests/integration/01-discovery.test.ts)/api/v1/discoveryas primary methodBehavior
Before:
After:
Both endpoints remain functional; only the priority order changed to match protocol requirements.