Skip to content

Commit e227e3a

Browse files
authored
Merge pull request #884 from objectstack-ai/copilot/fix-auth-service-routing-issue
2 parents 722dcb0 + c88e82d commit e227e3a

4 files changed

Lines changed: 87 additions & 31 deletions

File tree

ROADMAP.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ business/custom objects, aligning with industry best practices (e.g., ServiceNow
330330
- v3.x: The `SystemObjectName` constants now emit `sys_`-prefixed names. Implementations using `StorageNameMapping.resolveTableName()` can set `tableName` to preserve legacy physical table names during the transition.
331331
- v3.x: The `@objectstack/plugin-auth` ObjectQL adapter now includes `AUTH_MODEL_TO_PROTOCOL` mapping to translate better-auth's hardcoded model names (`user`, `session`, `account`, `verification`) to protocol names (`sys_user`, `sys_session`, `sys_account`, `sys_verification`). Custom adapters must adopt the same mapping.
332332
- v3.x: **Bug fix**`AuthManager.createDatabaseConfig()` now wraps the ObjectQL adapter as a `DBAdapterInstance` factory function (`(options) => DBAdapter`). Previously the raw adapter object was passed, which fell through to the Kysely adapter path and failed silently. `AuthManager.handleRequest()` and `AuthPlugin.registerAuthRoutes()` now inspect `response.status >= 500` and log the error body, since better-auth catches internal errors and returns 500 Responses without throwing.
333+
- v3.x: **Bug fix**`AuthPlugin` now defers HTTP route registration to a `kernel:ready` hook instead of doing it synchronously in `start()`. This makes the plugin resilient to plugin loading order — the `http-server` service is guaranteed to be available after all plugins complete their init/start phases. The CLI `serve` command also registers `HonoServerPlugin` before config plugins (with duplicate detection) for the same reason.
333334
- v4.0: Legacy un-prefixed aliases will be fully removed.
334335

335336
---

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: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,19 @@ describe('AuthPlugin', () => {
88
let mockContext: PluginContext;
99
let authPlugin: AuthPlugin;
1010

11+
/** Shared hook capture utilities for tests that need kernel:ready simulation */
12+
const createHookCapture = () => {
13+
const handlers = new Map<string, Array<(...args: any[]) => Promise<void>>>();
14+
const hookFn = vi.fn((name: string, handler: (...args: any[]) => Promise<void>) => {
15+
if (!handlers.has(name)) handlers.set(name, []);
16+
handlers.get(name)!.push(handler);
17+
});
18+
const trigger = async (name: string) => {
19+
for (const h of handlers.get(name) || []) await h();
20+
};
21+
return { handlers, hookFn, trigger };
22+
};
23+
1124
beforeEach(() => {
1225
mockContext = {
1326
registerService: vi.fn(),
@@ -98,15 +111,26 @@ describe('AuthPlugin', () => {
98111
});
99112

100113
describe('Start Phase', () => {
114+
let hookCapture: ReturnType<typeof createHookCapture>;
115+
101116
beforeEach(async () => {
117+
hookCapture = createHookCapture();
102118
authPlugin = new AuthPlugin({
103119
secret: 'test-secret-at-least-32-chars-long',
104120
baseUrl: 'http://localhost:3000',
105121
});
122+
// Capture hook registrations so we can trigger them in tests
123+
mockContext.hook = hookCapture.hookFn;
106124
await authPlugin.init(mockContext);
107125
});
108126

109-
it('should register routes with HTTP server when enabled', async () => {
127+
it('should register a kernel:ready hook for route registration', async () => {
128+
await authPlugin.start(mockContext);
129+
130+
expect(mockContext.hook).toHaveBeenCalledWith('kernel:ready', expect.any(Function));
131+
});
132+
133+
it('should register routes with HTTP server on kernel:ready', async () => {
110134
const mockRawApp = {
111135
all: vi.fn(),
112136
};
@@ -128,6 +152,12 @@ describe('AuthPlugin', () => {
128152

129153
await authPlugin.start(mockContext);
130154

155+
// Routes should NOT be registered yet (deferred to kernel:ready)
156+
expect(mockRawApp.all).not.toHaveBeenCalled();
157+
158+
// Simulate kernel:ready
159+
await hookCapture.trigger('kernel:ready');
160+
131161
expect(mockContext.getService).toHaveBeenCalledWith('http-server');
132162
expect(mockHttpServer.getRawApp).toHaveBeenCalled();
133163
expect(mockRawApp.all).toHaveBeenCalledWith('/api/v1/auth/*', expect.any(Function));
@@ -157,6 +187,7 @@ describe('AuthPlugin', () => {
157187
});
158188

159189
await authPlugin.start(mockContext);
190+
await hookCapture.trigger('kernel:ready');
160191

161192
// Extract the registered route handler
162193
const routeHandler = mockRawApp.all.mock.calls[0][1];
@@ -201,13 +232,15 @@ describe('AuthPlugin', () => {
201232
await authPlugin.init(mockContext);
202233
await authPlugin.start(mockContext);
203234

204-
expect(mockContext.getService).not.toHaveBeenCalledWith('http-server');
235+
// Should not register kernel:ready hook for routes
236+
expect(mockContext.hook).not.toHaveBeenCalledWith('kernel:ready', expect.any(Function));
205237
});
206238

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

210242
await authPlugin.start(mockContext);
243+
await hookCapture.trigger('kernel:ready');
211244

212245
expect(mockContext.getService).toHaveBeenCalledWith('http-server');
213246
expect(mockContext.logger.warn).toHaveBeenCalledWith(
@@ -222,6 +255,7 @@ describe('AuthPlugin', () => {
222255
});
223256

224257
await authPlugin.start(mockContext);
258+
await hookCapture.trigger('kernel:ready');
225259

226260
expect(mockContext.logger.warn).toHaveBeenCalledWith(
227261
expect.stringContaining('No HTTP server available')
@@ -258,6 +292,9 @@ describe('AuthPlugin', () => {
258292

259293
describe('Configuration Options', () => {
260294
it('should use custom base path', async () => {
295+
const { hookFn, trigger } = createHookCapture();
296+
mockContext.hook = hookFn;
297+
261298
authPlugin = new AuthPlugin({
262299
secret: 'test-secret-at-least-32-chars-long',
263300
baseUrl: 'http://localhost:3000',
@@ -284,6 +321,9 @@ describe('AuthPlugin', () => {
284321

285322
await authPlugin.start(mockContext);
286323

324+
// Trigger kernel:ready to actually register routes
325+
await trigger('kernel:ready');
326+
287327
expect(mockRawApp.all).toHaveBeenCalledWith(
288328
'/custom/auth/*',
289329
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)