Skip to content

Commit bac5f29

Browse files
Copilothotlong
andcommitted
fix: defer AuthPlugin route registration to kernel:ready hook and reorder HonoServerPlugin in CLI
AuthPlugin now registers HTTP routes via a kernel:ready hook instead of directly in start(). This makes it resilient to plugin loading order — the http-server service is guaranteed to be available after all plugins have completed their init/start phases. The CLI serve command now registers HonoServerPlugin before config plugins (with duplicate detection) so the http-server service is available earlier in the lifecycle for any plugin that needs it. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 6f74cfb commit bac5f29

3 files changed

Lines changed: 88 additions & 31 deletions

File tree

packages/cli/src/commands/serve.ts

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,26 @@ export default class Serve extends Command {
200200
}
201201
}
202202

203-
203+
// Add HTTP server plugin BEFORE config plugins so that the
204+
// http-server service is available for any plugin that needs it
205+
// during init/start (e.g. AuthPlugin).
206+
// Skip if config already contains a HonoServerPlugin to avoid
207+
// duplicate registration.
208+
const configHasHonoServer = plugins.some(
209+
(p: any) => p.name === 'com.objectstack.server.hono' || p.constructor?.name === 'HonoServerPlugin'
210+
);
211+
212+
if (flags.server && !configHasHonoServer) {
213+
try {
214+
const { HonoServerPlugin } = await import('@objectstack/plugin-hono-server');
215+
const serverPlugin = new HonoServerPlugin({ port });
216+
await kernel.use(serverPlugin);
217+
trackPlugin('HonoServer');
218+
} catch (e: any) {
219+
console.warn(chalk.yellow(` ⚠ HTTP server plugin not available: ${e.message}`));
220+
}
221+
}
222+
204223
if (plugins.length > 0) {
205224
for (const plugin of plugins) {
206225
try {
@@ -236,18 +255,8 @@ export default class Serve extends Command {
236255
}
237256
}
238257

239-
// Add HTTP server plugin if not disabled
258+
// Register REST API and Dispatcher plugins (consume http.server + protocol services)
240259
if (flags.server) {
241-
try {
242-
const { HonoServerPlugin } = await import('@objectstack/plugin-hono-server');
243-
const serverPlugin = new HonoServerPlugin({ port });
244-
await kernel.use(serverPlugin);
245-
trackPlugin('HonoServer');
246-
} catch (e: any) {
247-
console.warn(chalk.yellow(` ⚠ HTTP server plugin not available: ${e.message}`));
248-
}
249-
250-
// Register REST API plugin (consumes http.server + protocol services)
251260
try {
252261
const { createRestApiPlugin } = await import('@objectstack/rest');
253262
await kernel.use(createRestApiPlugin());

packages/plugins/plugin-auth/src/auth-plugin.test.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,36 @@ describe('AuthPlugin', () => {
9898
});
9999

100100
describe('Start Phase', () => {
101+
let hookHandlers: Map<string, Array<(...args: any[]) => Promise<void>>>;
102+
101103
beforeEach(async () => {
104+
hookHandlers = new Map();
102105
authPlugin = new AuthPlugin({
103106
secret: 'test-secret-at-least-32-chars-long',
104107
baseUrl: 'http://localhost:3000',
105108
});
109+
// Capture hook registrations so we can trigger them in tests
110+
mockContext.hook = vi.fn((name: string, handler: (...args: any[]) => Promise<void>) => {
111+
if (!hookHandlers.has(name)) hookHandlers.set(name, []);
112+
hookHandlers.get(name)!.push(handler);
113+
});
106114
await authPlugin.init(mockContext);
107115
});
108116

109-
it('should register routes with HTTP server when enabled', async () => {
117+
/** Helper: invoke all handlers registered for a given hook */
118+
const triggerHook = async (name: string) => {
119+
for (const handler of hookHandlers.get(name) || []) {
120+
await handler();
121+
}
122+
};
123+
124+
it('should register a kernel:ready hook for route registration', async () => {
125+
await authPlugin.start(mockContext);
126+
127+
expect(mockContext.hook).toHaveBeenCalledWith('kernel:ready', expect.any(Function));
128+
});
129+
130+
it('should register routes with HTTP server on kernel:ready', async () => {
110131
const mockRawApp = {
111132
all: vi.fn(),
112133
};
@@ -128,6 +149,12 @@ describe('AuthPlugin', () => {
128149

129150
await authPlugin.start(mockContext);
130151

152+
// Routes should NOT be registered yet (deferred to kernel:ready)
153+
expect(mockRawApp.all).not.toHaveBeenCalled();
154+
155+
// Simulate kernel:ready
156+
await triggerHook('kernel:ready');
157+
131158
expect(mockContext.getService).toHaveBeenCalledWith('http-server');
132159
expect(mockHttpServer.getRawApp).toHaveBeenCalled();
133160
expect(mockRawApp.all).toHaveBeenCalledWith('/api/v1/auth/*', expect.any(Function));
@@ -157,6 +184,7 @@ describe('AuthPlugin', () => {
157184
});
158185

159186
await authPlugin.start(mockContext);
187+
await triggerHook('kernel:ready');
160188

161189
// Extract the registered route handler
162190
const routeHandler = mockRawApp.all.mock.calls[0][1];
@@ -201,13 +229,15 @@ describe('AuthPlugin', () => {
201229
await authPlugin.init(mockContext);
202230
await authPlugin.start(mockContext);
203231

204-
expect(mockContext.getService).not.toHaveBeenCalledWith('http-server');
232+
// Should not register kernel:ready hook for routes
233+
expect(mockContext.hook).not.toHaveBeenCalledWith('kernel:ready', expect.any(Function));
205234
});
206235

207236
it('should gracefully skip routes when http-server is not available', async () => {
208237
mockContext.getService = vi.fn(() => null);
209238

210239
await authPlugin.start(mockContext);
240+
await triggerHook('kernel:ready');
211241

212242
expect(mockContext.getService).toHaveBeenCalledWith('http-server');
213243
expect(mockContext.logger.warn).toHaveBeenCalledWith(
@@ -222,6 +252,7 @@ describe('AuthPlugin', () => {
222252
});
223253

224254
await authPlugin.start(mockContext);
255+
await triggerHook('kernel:ready');
225256

226257
expect(mockContext.logger.warn).toHaveBeenCalledWith(
227258
expect.stringContaining('No HTTP server available')
@@ -258,6 +289,12 @@ describe('AuthPlugin', () => {
258289

259290
describe('Configuration Options', () => {
260291
it('should use custom base path', async () => {
292+
const hookHandlers: Map<string, Array<(...args: any[]) => Promise<void>>> = new Map();
293+
mockContext.hook = vi.fn((name: string, handler: (...args: any[]) => Promise<void>) => {
294+
if (!hookHandlers.has(name)) hookHandlers.set(name, []);
295+
hookHandlers.get(name)!.push(handler);
296+
});
297+
261298
authPlugin = new AuthPlugin({
262299
secret: 'test-secret-at-least-32-chars-long',
263300
baseUrl: 'http://localhost:3000',
@@ -284,6 +321,11 @@ describe('AuthPlugin', () => {
284321

285322
await authPlugin.start(mockContext);
286323

324+
// Trigger kernel:ready to actually register routes
325+
for (const handler of hookHandlers.get('kernel:ready') || []) {
326+
await handler();
327+
}
328+
287329
expect(mockRawApp.all).toHaveBeenCalledWith(
288330
'/custom/auth/*',
289331
expect.any(Function)

packages/plugins/plugin-auth/src/auth-plugin.ts

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,25 +99,31 @@ export class AuthPlugin implements Plugin {
9999
throw new Error('Auth manager not initialized');
100100
}
101101

102-
// Register HTTP routes if enabled and HTTP server is available
102+
// Defer HTTP route registration to kernel:ready hook.
103+
// This ensures all plugins (including HonoServerPlugin) have completed
104+
// their init and start phases before we attempt to look up the
105+
// http-server service — making AuthPlugin resilient to plugin
106+
// loading order.
103107
if (this.options.registerRoutes) {
104-
let httpServer: IHttpServer | null = null;
105-
try {
106-
httpServer = ctx.getService<IHttpServer>('http-server');
107-
} catch {
108-
// Service not found — expected in MSW/mock mode
109-
}
108+
ctx.hook('kernel:ready', async () => {
109+
let httpServer: IHttpServer | null = null;
110+
try {
111+
httpServer = ctx.getService<IHttpServer>('http-server');
112+
} catch {
113+
// Service not found — expected in MSW/mock mode
114+
}
110115

111-
if (httpServer) {
112-
// Route registration errors should propagate (server misconfiguration)
113-
this.registerAuthRoutes(httpServer, ctx);
114-
ctx.logger.info(`Auth routes registered at ${this.options.basePath}`);
115-
} else {
116-
ctx.logger.warn(
117-
'No HTTP server available — auth routes not registered. ' +
118-
'Auth service is still available for MSW/mock environments via HttpDispatcher.'
119-
);
120-
}
116+
if (httpServer) {
117+
// Route registration errors should propagate (server misconfiguration)
118+
this.registerAuthRoutes(httpServer, ctx);
119+
ctx.logger.info(`Auth routes registered at ${this.options.basePath}`);
120+
} else {
121+
ctx.logger.warn(
122+
'No HTTP server available — auth routes not registered. ' +
123+
'Auth service is still available for MSW/mock environments via HttpDispatcher.'
124+
);
125+
}
126+
});
121127
}
122128

123129
// Register auth middleware on ObjectQL engine (if available)

0 commit comments

Comments
 (0)