Skip to content

Commit bc1c1ce

Browse files
claude[bot]claude
andauthored
fix(rest): a scoped /discovery advertises routes.auth on the unscoped base (#16957)
* test(rest): pin routes.auth on a SCOPED discovery document (#16538) No test had ever touched `routes.auth` on a scoped `/discovery` document. The three existing pins live in `packages/objectql/src/protocol-discovery.test.ts` and measure the PRODUCER, which never sees a base path — so the REST projection's scoped branch was unmeasured, and that is why the defect was green. This commit adds the failing case only; the repair follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * fix(rest): strip the environment scope from the advertised auth route (#16538) `registerDiscoveryEndpoints`' auth branch stripped only the retired `/projects/:environmentId`, while `isScoped` — the condition guarding that branch — is true only for `/environments/:environmentId`. The replace could therefore never match where it ran: it returned `basePath` unchanged, so a scoped `/discovery` advertised `routes.auth` as `/api/v1/environments/:environmentId/auth` — keeping both the scope the comment two lines above says to drop ("Auth is a control-plane concern, so use the unscoped base") and a literal, unsubstituted route parameter. The repair is the sibling MCP block's own regex, in the same handler and for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * chore(changeset): patch @objectstack/rest for the scoped auth route repair (#16538) Measured rather than assumed: `@objectstack/rest` is public (npm 17.3.0) and ships `files: ["dist", ...]`; the changed regex is present twice in both `dist/index.js` and `dist/index.cjs` — the sibling MCP strip as the positive control, and the auth strip this change moved — while the retired-only spelling has zero occurrences there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3e21cf0 commit bc1c1ce

3 files changed

Lines changed: 87 additions & 1 deletion

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
Fix `GET /discovery` advertising an unusable `routes.auth` on a scoped deployment.
6+
7+
`registerDiscoveryEndpoints` strips the environment scope off the advertised auth
8+
route, because auth is a control-plane concern. That strip named only the retired
9+
`/projects/:environmentId` spelling, while `isScoped` — the condition guarding the
10+
branch — matches only `/environments/:environmentId`, so the strip could never match
11+
where it ran. A scoped `/discovery` therefore advertised `routes.auth` as
12+
`/api/v1/environments/:environmentId/auth`: still scoped, and carrying a literal,
13+
unsubstituted route parameter. It now advertises `/api/v1/auth`, the same shape the
14+
sibling `routes.mcp` already used. Unscoped deployments are unaffected.

packages/rest/src/discovery-per-request-protocol.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,3 +330,65 @@ describe('[#9292] unscoped /discovery follows the shared resolution chain', () =
330330
expect(substance(served)).toEqual(substance(await tenantA.getDiscovery()));
331331
});
332332
});
333+
334+
// ---------------------------------------------------------------------------
335+
// [#16538] `routes.auth` is a CONTROL-PLANE concern, so a SCOPED document must
336+
// still advertise it on the unscoped base.
337+
//
338+
// The handler states that contract two lines above the computation ("Auth is a
339+
// control-plane concern, so use the unscoped base") and then strips only the
340+
// retired `/projects/:environmentId` spelling — while `isScoped`, the very
341+
// condition guarding the branch, is true only for `/environments/:environmentId`.
342+
// So the strip could never match on the branch it guarded: `replace` returned
343+
// the string unchanged and the advertised auth route kept both the scope and a
344+
// literal, unsubstituted `:environmentId`.
345+
//
346+
// These pins live HERE and not beside the three `routes.auth` pins in
347+
// `packages/objectql/src/protocol-discovery.test.ts` because those measure the
348+
// PRODUCER (`ObjectStackProtocolImplementation.getDiscovery()`), which never
349+
// sees a base path; the defect is in this file's REST projection over it, and
350+
// `@objectstack/rest` is not reachable from `packages/objectql` (it would be a
351+
// dependency cycle — `@objectstack/rest` devDepends on `@objectstack/objectql`).
352+
// That is why no test had ever touched `routes.auth` on a scoped document, and
353+
// why the defect was green.
354+
// ---------------------------------------------------------------------------
355+
356+
const AUTH_TENANT_SHAPE = { ...tenantAShape, services: [...tenantAShape.services, 'auth'] };
357+
358+
describe('[#16538] scoped /discovery advertises routes.auth on the UNSCOPED base', () => {
359+
it('strips the environment scope, leaving no :environmentId in routes.auth', async () => {
360+
const { serve } = boot({
361+
environments: { 'tenant-a': realProtocol(AUTH_TENANT_SHAPE) },
362+
host: realProtocol(hostShape),
363+
});
364+
365+
const served = await serve(SCOPED, { environmentId: 'tenant-a' });
366+
367+
// Control, first: this document really is the scoped one, and the routes
368+
// that SHOULD carry the environment do carry the resolved id. Without it a
369+
// green assertion below could just mean the unscoped branch was taken.
370+
expect(served.routes.data).toBe('/api/v1/environments/tenant-a/data');
371+
expect(served.scoping).toMatchObject({ scoped: true, environmentId: 'tenant-a' });
372+
373+
// Subject: auth is advertised on the unscoped base, as the comment above
374+
// the computation says it is.
375+
expect(served.routes.auth).toBe('/api/v1/auth');
376+
// And separately: whatever base it lands on, it never advertises an
377+
// unsubstituted route parameter. This is the half a client cannot use at
378+
// all — `GET /api/v1/environments/:environmentId/auth` is not a URL.
379+
expect(served.routes.auth).not.toContain(':environmentId');
380+
});
381+
382+
it('keeps the unscoped document\'s auth route unchanged', async () => {
383+
// The other branch of the same computation, pinned so the repair cannot be
384+
// paid for out of the unscoped answer. Green before the fix and after —
385+
// it is a regression guard, not the reproduction.
386+
const { serve } = boot({
387+
environments: { 'tenant-a': realProtocol(AUTH_TENANT_SHAPE) },
388+
host: realProtocol(AUTH_TENANT_SHAPE),
389+
withKernelManager: false,
390+
});
391+
392+
expect((await serve(UNSCOPED)).routes.auth).toBe('/api/v1/auth');
393+
});
394+
});

packages/rest/src/rest-server.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4525,9 +4525,19 @@ export class RestServer {
45254525

45264526
// Align auth route with the versioned base path if present.
45274527
// Auth is a control-plane concern, so use the unscoped base.
4528+
//
4529+
// [#16538] The strip names BOTH spellings, exactly as the MCP
4530+
// sibling above does. It used to name only the retired
4531+
// `/projects/:environmentId`, while `isScoped` — the condition
4532+
// guarding this very branch — keys on `/environments/:environmentId`
4533+
// alone. So the replace could never match where it ran: it returned
4534+
// `basePath` unchanged and a scoped `/discovery` advertised
4535+
// `/api/v1/environments/:environmentId/auth`, keeping both the scope
4536+
// this comment says to drop and a literal, unsubstituted route
4537+
// parameter. Pinned in `discovery-per-request-protocol.test.ts`.
45284538
if (discovery.routes.auth) {
45294539
const unscopedBase = isScoped
4530-
? basePath.replace(/\/projects\/:environmentId$/, '')
4540+
? basePath.replace(/\/(environments|projects)\/:environmentId$/, '')
45314541
: basePath;
45324542
discovery.routes.auth = `${unscopedBase}/auth`;
45334543
}

0 commit comments

Comments
 (0)