Skip to content

Commit 605f8d9

Browse files
rekmarksclaude
andcommitted
fix: Fix launch-subcluster RPC result type for JSON compatibility
Use nullable() instead of optional() for bootstrapResult field, and define a JSON-compatible LaunchSubclusterRpcResult type that uses null instead of undefined for JSON serialization. Also update tests to match the new behavior. Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent e69fbb1 commit 605f8d9

File tree

4 files changed

+173
-164
lines changed

4 files changed

+173
-164
lines changed
Lines changed: 47 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,37 @@
1-
import type { ClusterConfig, Kernel, KRef, VatId } from '@metamask/ocap-kernel';
1+
import type {
2+
ClusterConfig,
3+
Kernel,
4+
KernelStatus,
5+
KRef,
6+
VatId,
7+
} from '@metamask/ocap-kernel';
28
import { describe, it, expect, vi, beforeEach } from 'vitest';
39

410
import { makeKernelFacade } from './kernel-facade.ts';
511
import type { KernelFacade } from './kernel-facade.ts';
612

13+
const makeClusterConfig = (): ClusterConfig => ({
14+
bootstrap: 'v1',
15+
vats: {
16+
v1: {
17+
bundleSpec: 'test-source',
18+
},
19+
},
20+
});
21+
722
describe('makeKernelFacade', () => {
823
let mockKernel: Kernel;
924
let facade: KernelFacade;
1025

1126
beforeEach(() => {
1227
mockKernel = {
1328
launchSubcluster: vi.fn().mockResolvedValue({
14-
body: '#{"status":"ok"}',
15-
slots: [],
29+
subclusterId: 'sc1',
30+
bootstrapRootKref: 'ko1',
31+
bootstrapResult: {
32+
body: '#{"result":"ok"}',
33+
slots: [],
34+
},
1635
}),
1736
terminateSubcluster: vi.fn().mockResolvedValue(undefined),
1837
queueMessage: vi.fn().mockResolvedValue({
@@ -22,12 +41,9 @@ describe('makeKernelFacade', () => {
2241
getStatus: vi.fn().mockResolvedValue({
2342
vats: [],
2443
subclusters: [],
25-
remoteComms: false,
26-
}),
27-
pingVat: vi.fn().mockResolvedValue({
28-
pingVatResult: 'pong',
29-
roundTripMs: 10,
44+
remoteComms: { isInitialized: false },
3045
}),
46+
pingVat: vi.fn().mockResolvedValue('pong'),
3147
} as unknown as Kernel;
3248

3349
facade = makeKernelFacade(mockKernel);
@@ -42,94 +58,41 @@ describe('makeKernelFacade', () => {
4258

4359
describe('launchSubcluster', () => {
4460
it('delegates to kernel with correct arguments', async () => {
45-
const config: ClusterConfig = {
46-
name: 'test-cluster',
47-
vats: [
48-
{
49-
name: 'test-vat',
50-
bundleSpec: { type: 'literal', source: 'test' },
51-
},
52-
],
53-
};
61+
const config = makeClusterConfig();
5462

5563
await facade.launchSubcluster(config);
5664

5765
expect(mockKernel.launchSubcluster).toHaveBeenCalledWith(config);
5866
expect(mockKernel.launchSubcluster).toHaveBeenCalledTimes(1);
5967
});
6068

61-
it('returns result from kernel with parsed subclusterId and wrapped kref', async () => {
69+
it('returns result with subclusterId and rootKref from kernel', async () => {
6270
const kernelResult = {
63-
body: '#{"subclusterId":"s1"}',
64-
slots: ['ko1'],
71+
subclusterId: 's1',
72+
bootstrapRootKref: 'ko1',
73+
bootstrapResult: { body: '#null', slots: [] },
6574
};
6675
vi.mocked(mockKernel.launchSubcluster).mockResolvedValueOnce(
6776
kernelResult,
6877
);
6978

70-
const config: ClusterConfig = {
71-
name: 'test-cluster',
72-
vats: [],
73-
};
79+
const config = makeClusterConfig();
7480

7581
const result = await facade.launchSubcluster(config);
7682

77-
// The facade should parse the CapData and return a LaunchResult
7883
expect(result).toStrictEqual({
7984
subclusterId: 's1',
80-
rootKref: { kref: 'ko1' },
81-
rootKrefString: 'ko1',
85+
rootKref: 'ko1',
8286
});
8387
});
8488

8589
it('propagates errors from kernel', async () => {
8690
const error = new Error('Launch failed');
8791
vi.mocked(mockKernel.launchSubcluster).mockRejectedValueOnce(error);
8892

89-
const config: ClusterConfig = {
90-
name: 'test-cluster',
91-
vats: [],
92-
};
93-
94-
await expect(facade.launchSubcluster(config)).rejects.toThrow(error);
95-
});
96-
97-
it('throws when kernel returns no capData', async () => {
98-
vi.mocked(mockKernel.launchSubcluster).mockResolvedValueOnce(
99-
undefined as unknown as ReturnType<Kernel['launchSubcluster']>,
100-
);
101-
102-
const config = makeClusterConfig();
103-
104-
await expect(facade.launchSubcluster(config)).rejects.toThrow(
105-
'launchSubcluster: expected capData with root kref',
106-
);
107-
});
108-
109-
it('throws when capData body has no subclusterId', async () => {
110-
vi.mocked(mockKernel.launchSubcluster).mockResolvedValueOnce({
111-
body: '#{}',
112-
slots: ['ko1'],
113-
});
114-
115-
const config = makeClusterConfig();
116-
117-
await expect(facade.launchSubcluster(config)).rejects.toThrow(
118-
'launchSubcluster: expected subclusterId in body',
119-
);
120-
});
121-
122-
it('throws when capData slots is empty', async () => {
123-
vi.mocked(mockKernel.launchSubcluster).mockResolvedValueOnce({
124-
body: '#{"subclusterId":"sc1"}',
125-
slots: [],
126-
});
127-
12893
const config = makeClusterConfig();
12994

130-
await expect(facade.launchSubcluster(config)).rejects.toThrow(
131-
'launchSubcluster: expected root kref in slots',
132-
);
95+
await expect(facade.launchSubcluster(config)).rejects.toThrow(error);
13396
});
13497
});
13598

@@ -194,10 +157,10 @@ describe('makeKernelFacade', () => {
194157
});
195158

196159
it('returns status from kernel', async () => {
197-
const expectedStatus = {
198-
vats: [{ id: 'v1', name: 'test-vat' }],
160+
const expectedStatus: KernelStatus = {
161+
vats: [{ id: 'v1' as VatId, name: 'test-vat' }],
199162
subclusters: [],
200-
remoteComms: true,
163+
remoteComms: { isInitialized: true },
201164
};
202165
vi.mocked(mockKernel.getStatus).mockResolvedValueOnce(expectedStatus);
203166

@@ -215,7 +178,7 @@ describe('makeKernelFacade', () => {
215178

216179
describe('pingVat', () => {
217180
it('delegates to kernel with correct vatId', async () => {
218-
const vatId: VatId = 'v1';
181+
const vatId: VatId = 'v1' as VatId;
219182

220183
await facade.pingVat(vatId);
221184

@@ -224,18 +187,25 @@ describe('makeKernelFacade', () => {
224187
});
225188

226189
it('returns result from kernel', async () => {
227-
const expectedResult = { pingVatResult: 'pong', roundTripMs: 5 };
190+
const expectedResult = 'pong';
228191
vi.mocked(mockKernel.pingVat).mockResolvedValueOnce(expectedResult);
229192

230-
const result = await facade.pingVat('v1');
231-
expect(result).toStrictEqual(expectedResult);
193+
const result = await facade.pingVat('v1' as VatId);
194+
expect(result).toBe(expectedResult);
232195
});
233196

234197
it('propagates errors from kernel', async () => {
235198
const error = new Error('Ping vat failed');
236199
vi.mocked(mockKernel.pingVat).mockRejectedValueOnce(error);
237200

238-
await expect(facade.pingVat('v1')).rejects.toThrow(error);
201+
await expect(facade.pingVat('v1' as VatId)).rejects.toThrow(error);
202+
});
203+
});
204+
205+
describe('getVatRoot', () => {
206+
it('returns wrapped kref', async () => {
207+
const result = await facade.getVatRoot('ko123');
208+
expect(result).toStrictEqual({ kref: 'ko123' });
239209
});
240210
});
241211
});

packages/kernel-browser-runtime/src/rpc-handlers/launch-subcluster.test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,32 @@ describe('launchSubclusterHandler', () => {
4444
{ kernel: mockKernel },
4545
params,
4646
);
47-
expect(result).toBe(mockResult);
47+
expect(result).toStrictEqual(mockResult);
48+
});
49+
50+
it('converts undefined bootstrapResult to null for JSON compatibility', async () => {
51+
const mockResult = {
52+
subclusterId: 's1',
53+
bootstrapRootKref: 'ko1',
54+
bootstrapResult: undefined,
55+
};
56+
const mockKernel = {
57+
launchSubcluster: vi.fn().mockResolvedValue(mockResult),
58+
};
59+
const params = {
60+
config: {
61+
bootstrap: 'test-bootstrap',
62+
vats: {},
63+
},
64+
};
65+
const result = await launchSubclusterHandler.implementation(
66+
{ kernel: mockKernel },
67+
params,
68+
);
69+
expect(result).toStrictEqual({
70+
subclusterId: 's1',
71+
bootstrapRootKref: 'ko1',
72+
bootstrapResult: null,
73+
});
4874
});
4975
});
Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,38 @@
1+
import type { CapData } from '@endo/marshal';
12
import type { MethodSpec, Handler } from '@metamask/kernel-rpc-methods';
2-
import type {
3-
Kernel,
4-
ClusterConfig,
5-
SubclusterLaunchResult,
6-
} from '@metamask/ocap-kernel';
3+
import type { Kernel, ClusterConfig, KRef } from '@metamask/ocap-kernel';
74
import { ClusterConfigStruct, CapDataStruct } from '@metamask/ocap-kernel';
85
import {
96
object,
107
string,
11-
optional,
8+
nullable,
129
type as structType,
1310
} from '@metamask/superstruct';
1411

15-
const SubclusterLaunchResultStruct = structType({
12+
/**
13+
* JSON-compatible version of SubclusterLaunchResult for RPC.
14+
* Uses null instead of undefined for JSON serialization.
15+
*/
16+
type LaunchSubclusterRpcResult = {
17+
subclusterId: string;
18+
bootstrapRootKref: string;
19+
bootstrapResult: CapData<KRef> | null;
20+
};
21+
22+
const LaunchSubclusterRpcResultStruct = structType({
1623
subclusterId: string(),
1724
bootstrapRootKref: string(),
18-
bootstrapResult: optional(CapDataStruct),
25+
bootstrapResult: nullable(CapDataStruct),
1926
});
2027

2128
export const launchSubclusterSpec: MethodSpec<
2229
'launchSubcluster',
2330
{ config: ClusterConfig },
24-
Promise<SubclusterLaunchResult>
31+
Promise<LaunchSubclusterRpcResult>
2532
> = {
2633
method: 'launchSubcluster',
2734
params: object({ config: ClusterConfigStruct }),
28-
result: SubclusterLaunchResultStruct,
35+
result: LaunchSubclusterRpcResultStruct,
2936
};
3037

3138
export type LaunchSubclusterHooks = {
@@ -35,15 +42,21 @@ export type LaunchSubclusterHooks = {
3542
export const launchSubclusterHandler: Handler<
3643
'launchSubcluster',
3744
{ config: ClusterConfig },
38-
Promise<SubclusterLaunchResult>,
45+
Promise<LaunchSubclusterRpcResult>,
3946
LaunchSubclusterHooks
4047
> = {
4148
...launchSubclusterSpec,
4249
hooks: { kernel: true },
4350
implementation: async (
4451
{ kernel }: LaunchSubclusterHooks,
4552
params: { config: ClusterConfig },
46-
): Promise<SubclusterLaunchResult> => {
47-
return kernel.launchSubcluster(params.config);
53+
): Promise<LaunchSubclusterRpcResult> => {
54+
const result = await kernel.launchSubcluster(params.config);
55+
// Convert undefined to null for JSON compatibility
56+
return {
57+
subclusterId: result.subclusterId,
58+
bootstrapRootKref: result.bootstrapRootKref,
59+
bootstrapResult: result.bootstrapResult ?? null,
60+
};
4861
},
4962
};

0 commit comments

Comments
 (0)