From efa461f2ef92a7a1fd4fb47cdda6f98be3dbf60a Mon Sep 17 00:00:00 2001 From: Chad Miller Date: Mon, 10 Aug 2026 10:45:48 -0700 Subject: [PATCH] feat(core): serve community.lexicon.service.describe One unauthenticated query answering with this service's roles and every XRPC method it routes. Optional endpoints cannot be discovered any other way. atproto publishes no method list, so a client asking whether a server does permissioned data has to call a space method and read the failure, where an absent feature and a broken one look alike. bulleted.app asks this question and reported it could not read the answer, because the endpoint was not here. The list is derived from the route table when the request runs, not kept beside it. It cannot drift, and it names the space methods exactly when spaces are enabled, which is the question being asked. Proxied app.bsky.* methods are absent because they are not routed here; claiming them would describe the AppView's capabilities as this server's. com.atproto.space.getSpace answers alongside the simplespace name. The proposal moved the method between namespaces while implementations were already shipping: the atproto reference PR serves the second, ngerakines.me/atproto-crates the first, and a client that guesses wrong reads the feature as absent. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/spaces-reference-parity.md | 18 ++++- docs/permissioned-data.md | 12 +++ packages/core/src/pds.js | 35 ++++++++ packages/spaces/src/handlers/manage.js | 38 +++++---- packages/spaces/test/manage.test.js | 9 +++ test/service-describe.test.js | 108 +++++++++++++++++++++++++ 6 files changed, 204 insertions(+), 16 deletions(-) create mode 100644 test/service-describe.test.js diff --git a/.changeset/spaces-reference-parity.md b/.changeset/spaces-reference-parity.md index bc40f9e..a2f8bdf 100644 --- a/.changeset/spaces-reference-parity.md +++ b/.changeset/spaces-reference-parity.md @@ -3,8 +3,22 @@ '@pdsjs/spaces': minor --- -Bring permissioned data back in line with the reference implementation, and -answer unregistered XRPC methods the way the protocol specifies. +Describe what this server serves, bring permissioned data back in line with the +reference implementation, and answer unregistered XRPC methods the way the +protocol specifies. + +`community.lexicon.service.describe` is served: one unauthenticated query +answering with this service's roles and every XRPC method it routes. Optional +endpoints cannot be discovered any other way, since atproto publishes no method +list, and a client that has to call a space method and read the failure cannot +tell an absent feature from a broken one. The answer is derived from the route +table rather than kept beside it, so it cannot drift and it names the space +methods exactly when spaces are enabled. + +`com.atproto.space.getSpace` answers alongside +`com.atproto.simplespace.getSpace`. The proposal moved the method between +namespaces while implementations were already shipping, and both names are in +use. An unrouted `/xrpc/` returned a `text/plain` 404, which a client cannot tell apart from a transport failure. It now returns 501 with a diff --git a/docs/permissioned-data.md b/docs/permissioned-data.md index 577bfd8..a540330 100644 --- a/docs/permissioned-data.md +++ b/docs/permissioned-data.md @@ -14,6 +14,18 @@ application. scopes, and the `com.atproto.simplespace` management endpoints. Node, Deno, and Cloudflare Workers all support it. +## Telling a client spaces are here + +`community.lexicon.service.describe` answers with this server's roles and every +XRPC method it routes, so the space methods appear in that list exactly when +`PDS_ENABLE_SPACES` is set. A client has no other way to find out: atproto +publishes no method list, and calling a space method to see what happens cannot +separate a server without the feature from one that is broken. + +`com.atproto.space.getSpace` and `com.atproto.simplespace.getSpace` both answer. +The proposal moved the method between namespaces after implementations had +shipped, and a client that guesses the wrong one reads the feature as absent. + ## Blobs A space record's blobs are ordinary account blobs. They are uploaded through diff --git a/packages/core/src/pds.js b/packages/core/src/pds.js index 451acc3..b55fe75 100644 --- a/packages/core/src/pds.js +++ b/packages/core/src/pds.js @@ -456,6 +456,7 @@ export class PersonalDataServer { // a route table and passes it in. Core never imports @pdsjs/spaces; this is // the only seam. With it absent, space endpoints do not exist and fall // through to the normal 404. + /** @type {Routes} */ this.routes = { ...routes, ...this._sync.routes, @@ -469,6 +470,11 @@ export class PersonalDataServer { ...this._oauth.routes, ...this._backup.routes, ...(spaceRoutes ?? {}), + // Declared last so it can see every route above it. The handler reads + // this.routes when it runs, not now. + '/xrpc/community.lexicon.service.describe': { + handler: async () => this.describeService(), + }, }; this.spacesEnabled = Boolean(spaceRoutes); @@ -639,6 +645,35 @@ export class PersonalDataServer { return publicKey; } + /** + * community.lexicon.service.describe - what this service is and serves. + * + * Optional endpoints cannot be discovered any other way. atproto publishes no + * method list, so a client asking "does this server do permissioned data" + * otherwise has to call a space method and read the failure, where an absent + * feature and a broken one look alike. A client two hops away sees neither. + * + * The answer is derived from the route table rather than kept beside it, so + * it cannot drift and it names the space methods exactly when spaces are on. + * Proxied `app.bsky.*` methods are absent because they are not routed here; + * claiming them would describe the AppView's capabilities as this server's. + * + * @returns {Response} + */ + describeService() { + const methods = Object.keys(this.routes) + .filter((path) => path.startsWith('/xrpc/')) + .map((path) => path.slice('/xrpc/'.length)) + // `_health` is an endpoint, not a lexicon method. + .filter((nsid) => nsid.includes('.')) + .sort() + .map((nsid) => ({ + $type: 'community.lexicon.service.describe#nsid', + value: nsid, + })); + return Response.json({ roles: ['pds'], methods }); + } + /** * Handle incoming request - main entry point * @param {Request} request diff --git a/packages/spaces/src/handlers/manage.js b/packages/spaces/src/handlers/manage.js index b1ac2f1..8a022d4 100644 --- a/packages/spaces/src/handlers/manage.js +++ b/packages/spaces/src/handlers/manage.js @@ -252,6 +252,22 @@ export function createManageRoutes(ctx) { return row; } + /** @type {import('@pdsjs/core/pds').Route} */ + const getSpaceRoute = { + auth: 'optional', + handler: async (request, url, auth) => { + const space = url.searchParams.get('space'); + if (!space) return errorResponse('InvalidRequest', 'space is required'); + const denied = await authorizeRead(request, space, auth); + if (denied) return denied; + const row = await spaceStorage.getSpace(space); + if (!row?.isOwner) { + return errorResponse('SpaceNotFound', 'Space not found', 404); + } + return Response.json({ uri: row.uri, ...writeConfig(row) }); + }, + }; + return { '/xrpc/com.atproto.simplespace.createSpace': { method: 'POST', @@ -369,20 +385,14 @@ export function createManageRoutes(ctx) { // Authority role: describe a space. The config names the member policy and // the clients allowed to reach it, so a caller outside the space perimeter // does not get to read it. - '/xrpc/com.atproto.simplespace.getSpace': { - auth: 'optional', - handler: async (request, url, auth) => { - const space = url.searchParams.get('space'); - if (!space) return errorResponse('InvalidRequest', 'space is required'); - const denied = await authorizeRead(request, space, auth); - if (denied) return denied; - const row = await spaceStorage.getSpace(space); - if (!row?.isOwner) { - return errorResponse('SpaceNotFound', 'Space not found', 404); - } - return Response.json({ uri: row.uri, ...writeConfig(row) }); - }, - }, + '/xrpc/com.atproto.simplespace.getSpace': getSpaceRoute, + + // The proposal moved this method from com.atproto.space to + // com.atproto.simplespace while implementations were already shipping, and + // both names are in use: the atproto reference PR serves the second, + // ngerakines.me/atproto-crates the first. Both answer here until the draft + // settles, since a client that guesses wrong reads the feature as absent. + '/xrpc/com.atproto.space.getSpace': getSpaceRoute, '/xrpc/com.atproto.simplespace.addMember': { method: 'POST', diff --git a/packages/spaces/test/manage.test.js b/packages/spaces/test/manage.test.js index 47e200c..289af0a 100644 --- a/packages/spaces/test/manage.test.js +++ b/packages/spaces/test/manage.test.js @@ -373,6 +373,15 @@ describe('getSpace', () => { expect(res.status).toBe(404); }); + it('answers under the com.atproto.space name too', async () => { + await store.putSpace(makeSpaceRow(SPACE, { isOwner: true })); + const res = await get('/xrpc/com.atproto.space.getSpace', { + space: SPACE, + }); + expect(res.status).toBe(200); + expect((await res.json()).uri).toBe(SPACE); + }); + it('refuses a caller with neither a session nor a credential', async () => { await store.putSpace(makeSpaceRow(SPACE, { isOwner: true })); const res = await get( diff --git a/test/service-describe.test.js b/test/service-describe.test.js new file mode 100644 index 0000000..7974bbd --- /dev/null +++ b/test/service-describe.test.js @@ -0,0 +1,108 @@ +// community.lexicon.service.describe is the only way a client learns whether an +// optional feature is served here. atproto publishes no method list, so without +// this a caller has to invoke a space method and read the failure, where an +// absent feature and a broken one look the same. +import { describe, expect, it } from 'vitest'; +import { PersonalDataServer } from '../packages/core/src/pds.js'; + +const PATH = '/xrpc/community.lexicon.service.describe'; + +/** @param {import('../packages/core/src/pds.js').Routes} [spaceRoutes] */ +function createPds(spaceRoutes) { + // Nothing here reaches storage, so the ports can be bare stubs. + return new PersonalDataServer({ + actorStorage: /** @type {any} */ ({}), + sharedStorage: /** @type {any} */ ({}), + blobs: /** @type {any} */ ({}), + jwtSecret: 'test-secret', + spaceRoutes, + }); +} + +/** @param {PersonalDataServer} pds */ +async function describeBody(pds) { + const response = await pds.fetch( + new Request(`https://pds.example.com${PATH}`), + ); + expect(response.status).toBe(200); + return response.json(); +} + +describe('community.lexicon.service.describe', () => { + it('answers unauthenticated, naming the pds role', async () => { + const body = await describeBody(createPds()); + expect(body.roles).toEqual(['pds']); + }); + + it('names each method with its union member', async () => { + const body = await describeBody(createPds()); + for (const entry of body.methods) { + expect(entry.$type).toBe('community.lexicon.service.describe#nsid'); + expect(entry.value).toMatch(/^[a-z][a-z0-9.]*\.[a-zA-Z]+$/); + } + }); + + it('describes exactly the routed methods, sorted', async () => { + const pds = createPds(); + const body = await describeBody(pds); + + const routed = Object.keys(pds.routes) + .filter((p) => p.startsWith('/xrpc/')) + .map((p) => p.slice('/xrpc/'.length)) + .filter((n) => n.includes('.')) + .sort(); + + expect(body.methods.map((/** @type {any} */ m) => m.value)).toEqual(routed); + }); + + it('describes itself', async () => { + const body = await describeBody(createPds()); + expect(body.methods.map((/** @type {any} */ m) => m.value)).toContain( + 'community.lexicon.service.describe', + ); + }); + + it('omits _health, which is an endpoint rather than a method', async () => { + const body = await describeBody(createPds()); + const values = body.methods.map((/** @type {any} */ m) => m.value); + expect(values).not.toContain('_health'); + expect(values.some((/** @type {string} */ v) => v.startsWith('_'))).toBe( + false, + ); + }); + + it('omits the proxied app.bsky methods it does not route', async () => { + const body = await describeBody(createPds()); + const values = body.methods.map((/** @type {any} */ m) => m.value); + // Served locally, so declared. + expect(values).toContain('app.bsky.actor.getPreferences'); + // Forwarded to the AppView; claiming it would describe the AppView. + expect(values).not.toContain('app.bsky.feed.getTimeline'); + }); + + it('names no space method while spaces are off', async () => { + const body = await describeBody(createPds()); + const values = body.methods.map((/** @type {any} */ m) => m.value); + expect(values.some((/** @type {string} */ v) => v.includes('space'))).toBe( + false, + ); + }); + + it('names the space methods once spaces are on', async () => { + const body = await describeBody( + createPds( + /** @type {any} */ ({ + '/xrpc/com.atproto.space.listSpaces': { + handler: async () => new Response(), + }, + '/xrpc/com.atproto.simplespace.createSpace': { + handler: async () => new Response(), + }, + }), + ), + ); + const values = body.methods.map((/** @type {any} */ m) => m.value); + expect(values).toContain('com.atproto.space.listSpaces'); + expect(values).toContain('com.atproto.simplespace.createSpace'); + }); +}); -- 2.51.2