From 9b3e49fd5ac153c26d2bfee7cf3fc10ee6ae6e1a Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 9 Jun 2026 15:01:13 +0200 Subject: [PATCH 1/6] Open service: route load-body commands through channel for peer ops Load bodies now call the channel-routed command map so queries can invoke commands implemented only on a peer (e.g. server-side extraction). Also makes service registration idempotent and simplifies getService generics. Co-authored-by: Cursor --- code/core/src/shared/open-service/README.md | 16 ++-- code/core/src/shared/open-service/fixtures.ts | 5 +- .../src/shared/open-service/server.test-d.ts | 6 +- .../src/shared/open-service/server.test.ts | 3 +- .../service-command-transport.test.ts | 83 ++++++++++++++++++- .../open-service/service-registration.test.ts | 21 ++--- .../shared/open-service/service-registry.ts | 26 +++--- .../shared/open-service/service-runtime.ts | 60 +++++++++++++- .../shared/open-service/service-transport.ts | 22 ++++- code/core/src/shared/open-service/types.ts | 9 +- 10 files changed, 201 insertions(+), 50 deletions(-) diff --git a/code/core/src/shared/open-service/README.md b/code/core/src/shared/open-service/README.md index a03490b285dc..dda5bfe70655 100644 --- a/code/core/src/shared/open-service/README.md +++ b/code/core/src/shared/open-service/README.md @@ -158,13 +158,13 @@ Handlers resolve other registered services through `ctx.getService(serviceId)`. parameter, the return type is `RuntimeService` — query and command results are erased to `unknown`. -Pass the source service definition as a generic to recover the full typed runtime surface: +Pass the source service instance type as a generic to recover the full typed runtime surface: ```ts -import type { mutableRecordLookupServiceDef } from './mutable-record-lookup.ts'; +import type { MutableRecordLookupService } from './mutable-record-lookup.ts'; handler: (input, ctx) => { - const lookup = ctx.getService( + const lookup = ctx.getService( 'internal-fixture/mutable-record-lookup' ); @@ -230,10 +230,12 @@ That split is intentional: writing for the current server process; the registry itself lives in [service-registry.ts](./service-registry.ts), shared with the browser entrypoints -`registerService(definition)` throws `OpenServiceDuplicateRegistrationError` if a service with the -same id is already registered. The default `services` preset hook in -[common-preset.ts](../../../core-server/presets/common-preset.ts) also throws if the preset is applied -more than once in the same process, which catches duplicate registration paths early. +`registerService(definition)` is idempotent by id: registering an id that already exists returns the +existing runtime instead of throwing. This keeps core services safe to register from a `beforeAll` +annotation, which CSF4 composes twice (once in `definePreview`, once in `StoryStore`) and which also +re-runs on HMR. The default `services` preset hook in +[common-preset.ts](../../../core-server/presets/common-preset.ts) still throws if the preset is applied +more than once in the same process, which catches misconfigured preset wiring early. The internal Storybook config registers an example debug service through a dedicated preset file ([`code/.storybook/services-preset.ts`](../../../../.storybook/services-preset.ts)), gated on diff --git a/code/core/src/shared/open-service/fixtures.ts b/code/core/src/shared/open-service/fixtures.ts index 79ec7378c9fe..39b45e4b9eda 100644 --- a/code/core/src/shared/open-service/fixtures.ts +++ b/code/core/src/shared/open-service/fixtures.ts @@ -1,6 +1,7 @@ import * as v from 'valibot'; import { defineService } from './service-definition.ts'; +import type { ServiceInstanceOf } from './types.ts'; /** Shared schema used by fixtures that address one logical record by id. */ export const entryIdInputSchema = v.object({ entryId: v.string() }); @@ -53,6 +54,8 @@ export const mutableRecordLookupServiceDef = defineService({ }, }); +export type MutableRecordLookupService = ServiceInstanceOf; + export type PreloadedValueState = Record; /** Service fixture that loads state from a command before returning it. */ @@ -250,7 +253,7 @@ export function createDerivedBooleanFromChildQueryServiceDef() { input: entryIdInputSchema, output: booleanOutputSchema, handler: (input, ctx) => { - const source = ctx.getService( + const source = ctx.getService( mutableRecordLookupServiceDef.id ); const record = source.queries.getRecordFields({ diff --git a/code/core/src/shared/open-service/server.test-d.ts b/code/core/src/shared/open-service/server.test-d.ts index 5ddd53ea85e9..55ccd3a9cd9c 100644 --- a/code/core/src/shared/open-service/server.test-d.ts +++ b/code/core/src/shared/open-service/server.test-d.ts @@ -2,7 +2,7 @@ import * as v from 'valibot'; import { describe, expectTypeOf, it } from 'vitest'; import { defineService } from './index.ts'; -import { mutableRecordLookupServiceDef } from './fixtures.ts'; +import { type MutableRecordLookupService, mutableRecordLookupServiceDef } from './fixtures.ts'; import { registerService } from './server.ts'; import type { RuntimeService } from './types.ts'; @@ -130,7 +130,7 @@ describe('open-service registration types', () => { }); }); - it('types cross-service lookups when getService receives a definition generic', () => { + it('types cross-service lookups when getService receives an instance generic', () => { registerService(mutableRecordLookupServiceDef); registerService( defineService({ @@ -141,7 +141,7 @@ describe('open-service registration types', () => { input: entryIdInputSchema, output: v.nullable(v.string()), handler: (_input, ctx) => { - const lookup = ctx.getService( + const lookup = ctx.getService( 'internal-fixture/mutable-record-lookup' ); diff --git a/code/core/src/shared/open-service/server.test.ts b/code/core/src/shared/open-service/server.test.ts index 40befb30e243..6498b22f3c89 100644 --- a/code/core/src/shared/open-service/server.test.ts +++ b/code/core/src/shared/open-service/server.test.ts @@ -16,6 +16,7 @@ import { import { awaitedPreloadValueServiceDef, createSharedStaticFileServiceDef, + type MutableRecordLookupService, mutableRecordLookupServiceDef, } from './fixtures.ts'; @@ -114,7 +115,7 @@ describe('server static builds', () => { input: v.undefined(), output: v.undefined(), handler: async (_input, ctx) => { - const source = ctx.getService( + const source = ctx.getService( 'internal-fixture/mutable-record-lookup' ); const record = source.queries.getRecordFields({ diff --git a/code/core/src/shared/open-service/service-command-transport.test.ts b/code/core/src/shared/open-service/service-command-transport.test.ts index 582be998a38d..837d30c2f2e5 100644 --- a/code/core/src/shared/open-service/service-command-transport.test.ts +++ b/code/core/src/shared/open-service/service-command-transport.test.ts @@ -8,7 +8,13 @@ import * as v from 'valibot'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { mutableRecordLookupServiceDef } from './fixtures.ts'; +import { + awaitedPreloadValueServiceDef, + entryIdInputSchema, + mutableRecordLookupServiceDef, + preloadedValueOutputSchema, + voidOutputSchema, +} from './fixtures.ts'; import { defineService } from './service-definition.ts'; import { SERVICE_COMMAND_ACK, @@ -54,6 +60,29 @@ const throwingCommandServiceDef = defineService({ }, }); +/** Query `load` invokes a command that has no handler in this runtime (peer-only). */ +const loadInvokesRemoteCommandServiceDef = defineService({ + id: 'internal-fixture/load-invokes-remote-command', + description: 'Query load calls a command declared without a local handler.', + initialState: {} as Record, + queries: { + getPreloadedValue: { + description: 'Populates state via a remote-only command inside load.', + input: entryIdInputSchema, + output: preloadedValueOutputSchema, + handler: (input, ctx) => ctx.self.state[input.entryId] ?? null, + load: (input, ctx) => ctx.self.commands.preloadValue(input).then(() => undefined), + }, + }, + commands: { + preloadValue: { + description: 'Populates one entry — implemented only on a peer in this test runtime.', + input: entryIdInputSchema, + output: voidOutputSchema, + }, + }, +}); + function emittedCalls(channel: ReturnType, event: string) { return channel.emit.mock.calls.filter(([name]) => name === event); } @@ -279,3 +308,55 @@ describe('remote command responder (has local handler)', () => { expect(emittedCalls(channel, SERVICE_COMMAND_RESULT)).toHaveLength(0); }); }); + +describe('load bodies and command routing', () => { + it('calls the local command handler from a load body without emitting command-invoke', async () => { + const channel = createTestChannel(); + installTestChannel(channel); + const handlerSpy = vi.spyOn(awaitedPreloadValueServiceDef.commands.preloadValue, 'handler'); + + try { + const service = registerService(awaitedPreloadValueServiceDef); + + await service.queries.getPreloadedValue.loaded({ entryId: 'entry-a' }); + + expect(handlerSpy).toHaveBeenCalledTimes(1); + expect(handlerSpy.mock.calls[0]?.[0]).toEqual({ entryId: 'entry-a' }); + expect(emittedCalls(channel, SERVICE_COMMAND_INVOKE)).toHaveLength(0); + expect(service.queries.getPreloadedValue({ entryId: 'entry-a' })).toBe('preloaded'); + } finally { + handlerSpy.mockRestore(); + } + }); + + it('routes a load-body command through command-invoke when no local handler exists', async () => { + const channel = createTestChannel(); + installTestChannel(channel); + + const service = registerService(loadInvokesRemoteCommandServiceDef); + const promise = service.queries.getPreloadedValue.loaded({ entryId: 'entry-a' }); + + await vi.waitFor(() => expect(emittedCalls(channel, SERVICE_COMMAND_INVOKE)).toHaveLength(1)); + + expect(emittedCalls(channel, SERVICE_COMMAND_INVOKE)[0]?.[1]).toMatchObject({ + serviceId: loadInvokesRemoteCommandServiceDef.id, + commandName: 'preloadValue', + input: { entryId: 'entry-a' }, + callId: expect.any(String), + clientId: expect.any(String), + }); + + const { callId } = emittedCalls( + channel, + SERVICE_COMMAND_INVOKE + )[0]?.[1] as CommandInvokePayload; + channel.emitExternal(SERVICE_COMMAND_RESULT, { + serviceId: loadInvokesRemoteCommandServiceDef.id, + callId, + result: undefined, + clientId: 'peer', + }); + + await expect(promise).resolves.toBeNull(); + }); +}); diff --git a/code/core/src/shared/open-service/service-registration.test.ts b/code/core/src/shared/open-service/service-registration.test.ts index e6a00ebf9041..fa1143ac4b40 100644 --- a/code/core/src/shared/open-service/service-registration.test.ts +++ b/code/core/src/shared/open-service/service-registration.test.ts @@ -7,6 +7,7 @@ import { awaitedPreloadValueServiceDef, createDerivedBooleanFromChildQueryServiceDef, entryIdInputSchema, + type MutableRecordLookupService, mutableRecordLookupServiceDef, recordFieldsOutputSchema, voidOutputSchema, @@ -65,20 +66,12 @@ describe('service registration', () => { expect(descriptor.commands.assignRecordField.output).toBe(voidOutputSchema); }); - it('throws when registering the same service id twice', () => { - registerService(mutableRecordLookupServiceDef); + it('is idempotent by id: re-registering returns the existing instance instead of throwing', () => { + const first = registerService(mutableRecordLookupServiceDef); + const second = registerService(mutableRecordLookupServiceDef); - try { - registerService(mutableRecordLookupServiceDef); - expect.unreachable('Expected duplicate registration to throw'); - } catch (error) { - expect(error).toMatchObject({ - fromStorybook: true, - code: 6, - message: - 'A service with id "internal-fixture/mutable-record-lookup" is already registered.', - }); - } + expect(second).toBe(first); + expect(getRegisteredServices()).toHaveLength(1); }); it('throws a Storybook error when resolving a missing registered service id', () => { @@ -165,7 +158,7 @@ describe('service registration', () => { }, assignFromLookup: { handler: async (input, ctx) => { - const lookup = ctx.getService( + const lookup = ctx.getService( 'internal-fixture/mutable-record-lookup' ); diff --git a/code/core/src/shared/open-service/service-registry.ts b/code/core/src/shared/open-service/service-registry.ts index f92458fae470..90d44c1214ab 100644 --- a/code/core/src/shared/open-service/service-registry.ts +++ b/code/core/src/shared/open-service/service-registry.ts @@ -16,7 +16,6 @@ */ import { - OpenServiceDuplicateRegistrationError, OpenServiceMissingChannelError, OpenServiceMissingServiceError, } from '../../server-errors.ts'; @@ -34,7 +33,6 @@ import type { ServiceDescriptor, ServiceId, ServiceInstance, - ServiceInstanceOf, ServiceRegistrationOptions, ServiceRegistryApi, ServiceSummary, @@ -193,7 +191,7 @@ export interface ServiceRegisterOptions { * callers use, wraps commands to broadcast their post-mutation state, and joins the cross-peer sync * protocol as a hub or leaf (`relay`). Each runtime must install the addons channel at its entry * boundary before calling this (builders, manager boot, server `services` preset, or Node import - * bootstrap). Duplicate ids are rejected up front so lookups remain deterministic. + * bootstrap). Registration is idempotent by id: a repeated registration returns the existing runtime. */ export function registerService< TState, @@ -206,8 +204,15 @@ export function registerService< ): ServiceInstance & ServiceRegistryApi { const registry = getRegistry(); - if (registry.has(definition.id)) { - throw new OpenServiceDuplicateRegistrationError({ serviceId: definition.id }); + // Registration is idempotent by id. Re-registering an already-registered service returns the + // existing runtime instead of throwing. This deliberately swallows duplicate-id collisions, which is + // the right trade-off: core services register from a `beforeAll` annotation that CSF4 composes twice + // (once in `definePreview`, once in `StoryStore`), and `beforeAll` also re-runs on HMR. A second + // registration is a no-op rather than a crash. + const existingEntry = registry.get(definition.id); + if (existingEntry) { + return existingEntry.instance as unknown as ServiceInstance & + ServiceRegistryApi; } const ownClientId = generateClientId(); @@ -261,6 +266,7 @@ export function registerService< commands: runtime.commands as Record Promise>, implementedCommandNames, commandNames: Object.keys(resolvedDefinition.commands), + runtime, }); const instance = { @@ -311,20 +317,14 @@ export async function describeService(serviceId: ServiceId): Promise( - serviceId: ServiceId -): ServiceInstanceOf; -export function getService( - serviceId: ServiceId -): RuntimeService | ServiceInstanceOf { +export function getService(serviceId: ServiceId): TInstance { const entry = getRegistry().get(serviceId); if (!entry) { throw new OpenServiceMissingServiceError({ serviceId }); } - return entry.instance as unknown as ServiceInstanceOf; + return entry.instance as unknown as TInstance; } /** diff --git a/code/core/src/shared/open-service/service-runtime.ts b/code/core/src/shared/open-service/service-runtime.ts index 0e52b25eb509..87b4d74ada9b 100644 --- a/code/core/src/shared/open-service/service-runtime.ts +++ b/code/core/src/shared/open-service/service-runtime.ts @@ -109,6 +109,19 @@ export type ServiceRuntime< commands: ServiceInstance['commands']; queries: ServiceInstance['queries']; runLoadOnce(queryName: string, validatedInput: unknown): Promise; + /** + * Installs the channel-routed command map produced once the runtime is wired to the channel. + * + * Load bodies use this map (not the raw local one) so a command implemented only on a peer — e.g. a + * server-only `extractDocgen` invoked from the manager's `getDocgen` load — is requested remotely + * instead of throwing `OpenServiceUnimplementedOperationError` locally. Command names not in + * `implementedCommandNames` are treated as remote and routed through this map even inside the + * stale-write-gated reactive load path (remote calls carry no local `setState` to gate). + */ + attachChannelCommands( + commands: Record Promise>, + implementedCommandNames: ReadonlySet + ): void; }; /** Max number of drain iterations before `.loaded()` gives up to avoid infinite oscillation. */ @@ -390,10 +403,17 @@ type QueryRuntimeRefs = { registryApi: ServiceRegistryApi; queryDefinitions: Map>; defaultQueries: Record>; + /** + * Returns the command map load bodies should call. After the runtime is wired to the channel this + * is the channel-routed map, so a load can invoke a peer-implemented (remote) command; before that + * (and in channel-free contexts like the static build) it is the raw local map. + */ + getLoadCommands: () => CommandSelf['commands']; /** * Builds a command map whose `setState` writes are dropped once `isCurrent()` returns false. * Used by reactive subscription loads so a superseded (stale) re-run cannot overwrite the state - * produced by a newer run. + * produced by a newer run. Remote commands are routed through the channel map unchanged, since they + * carry no local `setState` to gate. */ buildGatedCommands: (isCurrent: () => boolean) => CommandSelf['commands']; }; @@ -570,7 +590,7 @@ async function runLoadBody( return refs.state; }, queries: wrappedQueries, - commands: refs.commandSelf.commands as LoadSelf['commands'], + commands: refs.getLoadCommands() as LoadSelf['commands'], }; const loadCtx: LoadCtx = { self: loadSelf, getService: refs.registryApi.getService }; @@ -1172,6 +1192,14 @@ export function createServiceRuntime< >['commands']; commandSelf.commands = commands as CommandSelf['commands']; + // The command map load bodies should call. Defaults to the raw local map (used by the static build + // and before the channel is wired); `attachChannelCommands` swaps in the channel-routed map so + // loads can invoke peer-implemented commands remotely. `remoteCommandNames` is the set of commands + // with no local handler in this runtime, which the gated reactive-load path routes through the + // channel map directly (they have no local `setState` to gate). + let loadCommands = commands as CommandSelf['commands']; + let remoteCommandNames: ReadonlySet = new Set(); + const queryDefinitions = new Map>( Object.entries(def.queries) as [string, RuntimeQueryDefinition][] ); @@ -1200,7 +1228,21 @@ export function createServiceRuntime< getService: registryApi.getService, })); gatedSelf.commands = gated as CommandSelf['commands']; - return gated as CommandSelf['commands']; + + // Route remote commands through the channel map (so a reactive load can invoke a peer command); + // keep the gated local wrapper for locally-handled commands so stale-write protection holds. + if (remoteCommandNames.size === 0) { + return gated as CommandSelf['commands']; + } + const routed = Object.fromEntries( + Object.keys(def.commands).map((name) => [ + name, + remoteCommandNames.has(name) + ? (loadCommands as Record)[name] + : (gated as Record)[name], + ]) + ); + return routed as CommandSelf['commands']; }; const refs: QueryRuntimeRefs = { @@ -1210,6 +1252,7 @@ export function createServiceRuntime< registryApi, queryDefinitions, defaultQueries, + getLoadCommands: () => loadCommands, buildGatedCommands, }; @@ -1274,6 +1317,16 @@ export function createServiceRuntime< await promise; }; + const attachChannelCommands = ( + channelCommands: Record Promise>, + implementedCommandNames: ReadonlySet + ): void => { + loadCommands = channelCommands as CommandSelf['commands']; + remoteCommandNames = new Set( + Object.keys(def.commands).filter((name) => !implementedCommandNames.has(name)) + ); + }; + return { getStateSnapshot, commandSelf, @@ -1282,6 +1335,7 @@ export function createServiceRuntime< commands, queries, runLoadOnce, + attachChannelCommands, }; } diff --git a/code/core/src/shared/open-service/service-transport.ts b/code/core/src/shared/open-service/service-transport.ts index ba8f312c5ad3..9535b5bae0f1 100644 --- a/code/core/src/shared/open-service/service-transport.ts +++ b/code/core/src/shared/open-service/service-transport.ts @@ -9,8 +9,9 @@ * adopted state. * * - {@link connectServiceToChannel} is the single entry point `registerService` uses. It wires all - * three halves below against one channel so they can never be assembled inconsistently, and returns - * the command map callers expose plus a combined teardown. + * three halves below against one channel, installs the channel-routed command map on the runtime + * (so load bodies can invoke peer-implemented commands), and returns the command map callers + * expose plus a combined teardown. * - {@link wrapCommandsForBroadcast} wraps a runtime's commands so each local call, after it resolves, * advances the last-write-wins stamp and broadcasts the full post-mutation snapshot. * - {@link connectRuntimeToChannel} attaches the sync-start initialization and patch listeners, emits @@ -398,6 +399,14 @@ export function connectCommandTransport(context: { }; } +/** Runtime surface needed to install the channel-routed command map for load bodies. */ +type ChannelConnectedRuntime = { + attachChannelCommands( + commands: Record, + implementedCommandNames: ReadonlySet + ): void; +}; + /** * Wires one service runtime to the channel end to end and returns the command map callers expose plus * a single teardown. @@ -405,6 +414,8 @@ export function connectCommandTransport(context: { * This is the one entry point `registerService` uses, so the three transport halves — command * broadcasting, the remote-command protocol, and the sync-start + patch listeners — are always * assembled together against the same `channel` and can never drift into using different channels. + * The channel-routed command map is also installed on the runtime so load bodies invoke + * peer-implemented commands remotely instead of throwing locally. */ export function connectServiceToChannel( context: RuntimeTransportContext & { @@ -416,6 +427,8 @@ export function connectServiceToChannel( implementedCommandNames: ReadonlySet; /** Every command name declared by the service definition. */ commandNames: readonly string[]; + /** Runtime to wire with the channel-routed command map for load bodies. */ + runtime: ChannelConnectedRuntime; } ): { commands: Record; disconnect: () => void } { const { @@ -428,6 +441,7 @@ export function connectServiceToChannel( commands, implementedCommandNames, commandNames, + runtime, } = context; // Wrap commands so a local mutation broadcasts its post-mutation snapshot. State adopted from peers @@ -460,6 +474,10 @@ export function connectServiceToChannel( relay, }); + // Load bodies call commands through the channel-routed map so a command implemented only on a peer + // is requested remotely instead of throwing locally. + runtime.attachChannelCommands(commandTransport.commands, implementedCommandNames); + return { commands: commandTransport.commands, disconnect: (): void => { diff --git a/code/core/src/shared/open-service/types.ts b/code/core/src/shared/open-service/types.ts index 42da04f74408..67806be8dc2c 100644 --- a/code/core/src/shared/open-service/types.ts +++ b/code/core/src/shared/open-service/types.ts @@ -30,7 +30,9 @@ export type ServiceId = string; */ export type ServiceState = TState & (TState extends readonly unknown[] - ? { __openServiceStateError: 'Service state must be a plain object, not an array.' } + ? { + __openServiceStateError: 'Service state must be a plain object, not an array.'; + } : unknown); /** Public schema shape exposed when describing a schema-backed service contract. */ @@ -372,10 +374,7 @@ export type ServiceInstanceOf = export interface ServiceRegistryApi { listServices(): Promise; describeService(serviceId: ServiceId): Promise; - getService(serviceId: ServiceId): RuntimeService; - getService( - serviceId: ServiceId - ): ServiceInstanceOf; + getService(serviceId: ServiceId): TInstance; } export type RuntimeService = ServiceInstance, Commands> & From f37fa243b5ef4eaa60bd7a934bb40336de729f52 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 9 Jun 2026 15:53:35 +0200 Subject: [PATCH 2/6] OpenService: restore getService overloads for definition and instance types getService must resolve to ServiceInstanceOf so runtime queries expose .loaded(). Keep a separate overload for explicit instance types such as DocgenService. Co-authored-by: Cursor --- code/core/src/shared/open-service/service-registry.ts | 10 ++++++++-- code/core/src/shared/open-service/types.ts | 6 +++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/code/core/src/shared/open-service/service-registry.ts b/code/core/src/shared/open-service/service-registry.ts index 81cfd7ac9afe..0dce5e59ed64 100644 --- a/code/core/src/shared/open-service/service-registry.ts +++ b/code/core/src/shared/open-service/service-registry.ts @@ -33,6 +33,7 @@ import type { ServiceDescriptor, ServiceId, ServiceInstance, + ServiceInstanceOf, ServiceRegistrationOptions, ServiceRegistryApi, ServiceSummary, @@ -323,14 +324,19 @@ export async function describeService(serviceId: ServiceId): Promise(serviceId: ServiceId): TInstance { +export function getService(serviceId: ServiceId): RuntimeService; +export function getService( + serviceId: ServiceId +): ServiceInstanceOf; +export function getService(serviceId: ServiceId): TInstance; +export function getService(serviceId: ServiceId): RuntimeService { const entry = getRegistry().get(serviceId); if (!entry) { throw new OpenServiceMissingServiceError({ serviceId }); } - return entry.instance as unknown as TInstance; + return entry.instance as unknown as RuntimeService; } /** diff --git a/code/core/src/shared/open-service/types.ts b/code/core/src/shared/open-service/types.ts index bb78ea595588..458608f50b07 100644 --- a/code/core/src/shared/open-service/types.ts +++ b/code/core/src/shared/open-service/types.ts @@ -391,7 +391,11 @@ export type ServiceInstanceOf = export interface ServiceRegistryApi { listServices(): Promise; describeService(serviceId: ServiceId): Promise; - getService(serviceId: ServiceId): TInstance; + getService(serviceId: ServiceId): RuntimeService; + getService( + serviceId: ServiceId + ): ServiceInstanceOf; + getService(serviceId: ServiceId): TInstance; } export type RuntimeService = ServiceInstance, Commands> & From 1bd10bfc1f9656d61d06cf9af1ddeb11b83ce839 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 9 Jun 2026 15:58:42 +0200 Subject: [PATCH 3/6] Revert "OpenService: restore getService overloads for definition and instance types" This reverts commit f37fa243b5ef4eaa60bd7a934bb40336de729f52. --- code/core/src/shared/open-service/service-registry.ts | 10 ++-------- code/core/src/shared/open-service/types.ts | 6 +----- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/code/core/src/shared/open-service/service-registry.ts b/code/core/src/shared/open-service/service-registry.ts index 0dce5e59ed64..81cfd7ac9afe 100644 --- a/code/core/src/shared/open-service/service-registry.ts +++ b/code/core/src/shared/open-service/service-registry.ts @@ -33,7 +33,6 @@ import type { ServiceDescriptor, ServiceId, ServiceInstance, - ServiceInstanceOf, ServiceRegistrationOptions, ServiceRegistryApi, ServiceSummary, @@ -324,19 +323,14 @@ export async function describeService(serviceId: ServiceId): Promise( - serviceId: ServiceId -): ServiceInstanceOf; -export function getService(serviceId: ServiceId): TInstance; -export function getService(serviceId: ServiceId): RuntimeService { +export function getService(serviceId: ServiceId): TInstance { const entry = getRegistry().get(serviceId); if (!entry) { throw new OpenServiceMissingServiceError({ serviceId }); } - return entry.instance as unknown as RuntimeService; + return entry.instance as unknown as TInstance; } /** diff --git a/code/core/src/shared/open-service/types.ts b/code/core/src/shared/open-service/types.ts index 458608f50b07..bb78ea595588 100644 --- a/code/core/src/shared/open-service/types.ts +++ b/code/core/src/shared/open-service/types.ts @@ -391,11 +391,7 @@ export type ServiceInstanceOf = export interface ServiceRegistryApi { listServices(): Promise; describeService(serviceId: ServiceId): Promise; - getService(serviceId: ServiceId): RuntimeService; - getService( - serviceId: ServiceId - ): ServiceInstanceOf; - getService(serviceId: ServiceId): TInstance; + getService(serviceId: ServiceId): TInstance; } export type RuntimeService = ServiceInstance, Commands> & From 753d1caed40ba08e7c90a38259e79a3aea80602d Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Tue, 9 Jun 2026 16:00:00 +0200 Subject: [PATCH 4/6] OpenService: use DocgenService instance type in manifests getService call With getService, pass runtime instance types rather than typeof the service definition. Adds DocgenService alias and updates manifests.ts. Co-authored-by: Cursor --- code/core/src/core-server/utils/manifests/manifests.ts | 4 ++-- .../src/shared/open-service/services/docgen/definition.ts | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/code/core/src/core-server/utils/manifests/manifests.ts b/code/core/src/core-server/utils/manifests/manifests.ts index c53b8298e395..d5ff1b26d222 100644 --- a/code/core/src/core-server/utils/manifests/manifests.ts +++ b/code/core/src/core-server/utils/manifests/manifests.ts @@ -9,7 +9,7 @@ import type { Polka } from 'polka'; import invariant from 'tiny-invariant'; import { getService } from '../../../shared/open-service/server.ts'; -import type { docgenServiceDef } from '../../../shared/open-service/services/docgen/definition.ts'; +import type { DocgenService } from '../../../shared/open-service/services/docgen/definition.ts'; import { Tag } from '../../../shared/constants/tags.ts'; import type { ComponentManifest, ComponentsManifest } from '../../../types/modules/core-common.ts'; import type { DocgenPayload } from '../../../shared/open-service/services/docgen/types.ts'; @@ -111,7 +111,7 @@ async function renderComponentsHtmlFromService( manifestComponentIds: string[], docsManifest?: DocsManifest ) { - const docgenService = getService('core/docgen'); + const docgenService = getService('core/docgen'); const startTime = performance.now(); const allPayloads = await docgenService.queries.getDocgenForAllComponents.loaded(); const durationMs = Math.round(performance.now() - startTime); diff --git a/code/core/src/shared/open-service/services/docgen/definition.ts b/code/core/src/shared/open-service/services/docgen/definition.ts index 1d838ad6edef..e4cef536178d 100644 --- a/code/core/src/shared/open-service/services/docgen/definition.ts +++ b/code/core/src/shared/open-service/services/docgen/definition.ts @@ -1,6 +1,7 @@ import * as v from 'valibot'; import { defineService } from 'storybook/open-service'; +import type { ServiceInstanceOf } from '../../types.ts'; import type { DocgenPayload } from './types.ts'; import { docgenQueryStaticPath } from './paths.ts'; @@ -118,3 +119,5 @@ export const docgenServiceDef = defineService({ }, }, }); + +export type DocgenService = ServiceInstanceOf; From 6c2ca410f1a5902d33e2992103d8df9b080c2c9f Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Wed, 10 Jun 2026 10:47:18 +0200 Subject: [PATCH 5/6] OpenService: address PR review on test cleanup and remoteCommandNames Use onTestFinished for handler spy restore in load-body test, and reuse a single remoteCommandNames Set cleared on attachChannelCommands. Co-authored-by: Cursor --- .../service-command-transport.test.ts | 21 +++++++++---------- .../shared/open-service/service-runtime.ts | 7 ++++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/code/core/src/shared/open-service/service-command-transport.test.ts b/code/core/src/shared/open-service/service-command-transport.test.ts index 837d30c2f2e5..1637346d36ae 100644 --- a/code/core/src/shared/open-service/service-command-transport.test.ts +++ b/code/core/src/shared/open-service/service-command-transport.test.ts @@ -6,7 +6,7 @@ * Peers are simulated with the test channel's `emitExternal`, the same approach the sync tests use. */ import * as v from 'valibot'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; import { awaitedPreloadValueServiceDef, @@ -314,19 +314,18 @@ describe('load bodies and command routing', () => { const channel = createTestChannel(); installTestChannel(channel); const handlerSpy = vi.spyOn(awaitedPreloadValueServiceDef.commands.preloadValue, 'handler'); + onTestFinished(() => { + handlerSpy.mockRestore(); + }); - try { - const service = registerService(awaitedPreloadValueServiceDef); + const service = registerService(awaitedPreloadValueServiceDef); - await service.queries.getPreloadedValue.loaded({ entryId: 'entry-a' }); + await service.queries.getPreloadedValue.loaded({ entryId: 'entry-a' }); - expect(handlerSpy).toHaveBeenCalledTimes(1); - expect(handlerSpy.mock.calls[0]?.[0]).toEqual({ entryId: 'entry-a' }); - expect(emittedCalls(channel, SERVICE_COMMAND_INVOKE)).toHaveLength(0); - expect(service.queries.getPreloadedValue({ entryId: 'entry-a' })).toBe('preloaded'); - } finally { - handlerSpy.mockRestore(); - } + expect(handlerSpy).toHaveBeenCalledTimes(1); + expect(handlerSpy.mock.calls[0]?.[0]).toEqual({ entryId: 'entry-a' }); + expect(emittedCalls(channel, SERVICE_COMMAND_INVOKE)).toHaveLength(0); + expect(service.queries.getPreloadedValue({ entryId: 'entry-a' })).toBe('preloaded'); }); it('routes a load-body command through command-invoke when no local handler exists', async () => { diff --git a/code/core/src/shared/open-service/service-runtime.ts b/code/core/src/shared/open-service/service-runtime.ts index 87b4d74ada9b..767049cccd74 100644 --- a/code/core/src/shared/open-service/service-runtime.ts +++ b/code/core/src/shared/open-service/service-runtime.ts @@ -1198,7 +1198,7 @@ export function createServiceRuntime< // with no local handler in this runtime, which the gated reactive-load path routes through the // channel map directly (they have no local `setState` to gate). let loadCommands = commands as CommandSelf['commands']; - let remoteCommandNames: ReadonlySet = new Set(); + const remoteCommandNames = new Set(); const queryDefinitions = new Map>( Object.entries(def.queries) as [string, RuntimeQueryDefinition][] @@ -1322,8 +1322,9 @@ export function createServiceRuntime< implementedCommandNames: ReadonlySet ): void => { loadCommands = channelCommands as CommandSelf['commands']; - remoteCommandNames = new Set( - Object.keys(def.commands).filter((name) => !implementedCommandNames.has(name)) + remoteCommandNames.clear(); + remoteCommandNames.add( + ...Object.keys(def.commands).filter((name) => !implementedCommandNames.has(name)) ); }; From acd0f7dbd343d0ff5b29d72c5349fc50adde3dd8 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Wed, 10 Jun 2026 11:10:18 +0200 Subject: [PATCH 6/6] fix(open-service): migrate moduleGraph getService callsites to instance type After merging next, the getService callsites for core/module-graph resolved to the definition type (no subscribe/loaded) under this PR's single getService generic. Add a ModuleGraphService instance-type alias and use it. Also fix remoteCommandNames Set.add spread misuse. Co-authored-by: Cursor --- .../change-detection/change-detection-service.ts | 4 ++-- .../change-detection/change-detection.test-helpers.ts | 4 ++-- code/core/src/shared/open-service/service-runtime.ts | 8 +++++--- .../src/shared/open-service/services/docgen/server.ts | 4 ++-- .../open-service/services/module-graph/definition.ts | 3 +++ 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/code/core/src/core-server/change-detection/change-detection-service.ts b/code/core/src/core-server/change-detection/change-detection-service.ts index 6fa6c2384240..928f7188c98e 100644 --- a/code/core/src/core-server/change-detection/change-detection-service.ts +++ b/code/core/src/core-server/change-detection/change-detection-service.ts @@ -11,7 +11,7 @@ import type { import { CHANGE_DETECTION_STATUS_TYPE_ID } from 'storybook/internal/types'; import { getService } from '../../shared/open-service/server.ts'; -import type { moduleGraphServiceDef } from '../../shared/open-service/services/module-graph/definition.ts'; +import type { ModuleGraphService } from '../../shared/open-service/services/module-graph/definition.ts'; import { getStoryIdsByAbsolutePath } from '../../shared/open-service/services/module-graph/story-files.ts'; import type { ErrorLike, @@ -151,7 +151,7 @@ export class ChangeDetectionService { } private getModuleGraph() { - return getService('core/module-graph'); + return getService('core/module-graph'); } /** True while the service is live and change-detection status publishing is enabled. */ diff --git a/code/core/src/core-server/change-detection/change-detection.test-helpers.ts b/code/core/src/core-server/change-detection/change-detection.test-helpers.ts index e6bc43b10e5f..2e2fb26ba83b 100644 --- a/code/core/src/core-server/change-detection/change-detection.test-helpers.ts +++ b/code/core/src/core-server/change-detection/change-detection.test-helpers.ts @@ -2,7 +2,7 @@ import { normalize } from 'pathe'; import { vi } from 'vitest'; import { getService } from '../../shared/open-service/server.ts'; -import type { moduleGraphServiceDef } from '../../shared/open-service/services/module-graph/definition.ts'; +import type { ModuleGraphService } from '../../shared/open-service/services/module-graph/definition.ts'; import { ModuleGraphEngine } from '../../shared/open-service/services/module-graph/engine/module-graph-engine.ts'; import type { ModuleGraphStatus } from '../../shared/open-service/services/module-graph/types.ts'; import { @@ -80,7 +80,7 @@ export function installModuleGraphQueryMock(engine: ModuleGraphEngine) { } ), }, - } as unknown as ReturnType>); + } as unknown as ModuleGraphService); return { applySnapshot: () => { diff --git a/code/core/src/shared/open-service/service-runtime.ts b/code/core/src/shared/open-service/service-runtime.ts index 767049cccd74..d772e80a0906 100644 --- a/code/core/src/shared/open-service/service-runtime.ts +++ b/code/core/src/shared/open-service/service-runtime.ts @@ -1323,9 +1323,11 @@ export function createServiceRuntime< ): void => { loadCommands = channelCommands as CommandSelf['commands']; remoteCommandNames.clear(); - remoteCommandNames.add( - ...Object.keys(def.commands).filter((name) => !implementedCommandNames.has(name)) - ); + for (const name of Object.keys(def.commands)) { + if (!implementedCommandNames.has(name)) { + remoteCommandNames.add(name); + } + } }; return { diff --git a/code/core/src/shared/open-service/services/docgen/server.ts b/code/core/src/shared/open-service/services/docgen/server.ts index 370b9879ebc1..6178fdb41a71 100644 --- a/code/core/src/shared/open-service/services/docgen/server.ts +++ b/code/core/src/shared/open-service/services/docgen/server.ts @@ -5,7 +5,7 @@ import { import { OpenServiceDocgenMissingComponentError } from '../../../../server-errors.ts'; import type { StoryIndex } from '../../../../types/modules/indexer.ts'; import { getService, registerService } from '../../server.ts'; -import type { moduleGraphServiceDef } from '../module-graph/definition.ts'; +import type { ModuleGraphService } from '../module-graph/definition.ts'; import { toStoryIndexPath } from '../module-graph/types.ts'; import { docgenServiceDef } from './definition.ts'; import type { DocgenPayload, DocgenProvider } from './types.ts'; @@ -96,7 +96,7 @@ export function registerDocgenService(options: RegisterDocgenServiceOptions) { // touched by the latest graph change; we react to that list and re-extract docgen for the affected // components — even when nobody is actively subscribed to them right now. Without this, an open // docgen consumer would keep serving stale output until it happened to re-query. - const moduleGraph = getService('core/module-graph'); + const moduleGraph = getService('core/module-graph'); moduleGraph.queries.getLatestStoryChanges.subscribe(undefined, async ({ storyFiles }) => { if (storyFiles.length === 0) { diff --git a/code/core/src/shared/open-service/services/module-graph/definition.ts b/code/core/src/shared/open-service/services/module-graph/definition.ts index 646482190d7d..856358f5a87f 100644 --- a/code/core/src/shared/open-service/services/module-graph/definition.ts +++ b/code/core/src/shared/open-service/services/module-graph/definition.ts @@ -1,6 +1,7 @@ import * as v from 'valibot'; import { defineService } from '../../service-definition.ts'; +import type { ServiceInstanceOf } from '../../types.ts'; import type { ModuleGraphServiceState } from './types.ts'; import { toStoryIndexPath } from './types.ts'; @@ -285,3 +286,5 @@ export const moduleGraphServiceDef = defineService({ }, }, }); + +export type ModuleGraphService = ServiceInstanceOf;