Skip to content

Commit 0fb5d78

Browse files
Copilothotlong
andcommitted
fix(core): Address code review feedback - improve error handling and documentation
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 47c6255 commit 0fb5d78

4 files changed

Lines changed: 31 additions & 20 deletions

File tree

packages/core/examples/enhanced-kernel-example.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ async function setupServiceFactories(kernel: EnhancedObjectKernel) {
174174
kernel.registerServiceFactory(
175175
'request-id',
176176
() => {
177-
return `req-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
177+
return `req-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`;
178178
},
179179
ServiceLifecycle.TRANSIENT
180180
);

packages/core/src/enhanced-kernel.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ describe('EnhancedObjectKernel', () => {
145145
await expect(async () => {
146146
await kernel.bootstrap();
147147
}).rejects.toThrow('timeout');
148-
}, 10000);
148+
}, 1000); // Test should complete in 1 second
149149

150150
it('should timeout plugin start if it takes too long', async () => {
151151
const plugin: PluginMetadata = {
@@ -163,7 +163,7 @@ describe('EnhancedObjectKernel', () => {
163163
await expect(async () => {
164164
await kernel.bootstrap();
165165
}).rejects.toThrow();
166-
}, 10000);
166+
}, 1000); // Test should complete in 1 second
167167

168168
it('should complete plugin startup within timeout', async () => {
169169
const plugin: PluginMetadata = {

packages/core/src/enhanced-kernel.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -462,18 +462,28 @@ export class EnhancedObjectKernel {
462462

463463
private registerShutdownSignals(): void {
464464
const signals: NodeJS.Signals[] = ['SIGINT', 'SIGTERM', 'SIGQUIT'];
465+
let shutdownInProgress = false;
466+
467+
const handleShutdown = async (signal: string) => {
468+
if (shutdownInProgress) {
469+
this.logger.warn(`Shutdown already in progress, ignoring ${signal}`);
470+
return;
471+
}
472+
473+
shutdownInProgress = true;
474+
this.logger.info(`Received ${signal} - initiating graceful shutdown`);
475+
476+
try {
477+
await this.shutdown();
478+
process.exit(0);
479+
} catch (error) {
480+
this.logger.error('Shutdown failed', error as Error);
481+
process.exit(1);
482+
}
483+
};
465484

466485
for (const signal of signals) {
467-
process.on(signal, async () => {
468-
this.logger.info(`Received ${signal} - initiating graceful shutdown`);
469-
try {
470-
await this.shutdown();
471-
process.exit(0);
472-
} catch (error) {
473-
this.logger.error('Shutdown failed', error as Error);
474-
process.exit(1);
475-
}
476-
});
486+
process.on(signal, () => handleShutdown(signal));
477487
}
478488
}
479489

packages/core/src/plugin-loader.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@ export class PluginLoader {
228228

229229
/**
230230
* Detect circular dependencies in service factories
231+
* Note: This only detects cycles in service dependencies, not plugin dependencies.
232+
* Plugin dependency cycles are detected in the kernel's resolveDependencies method.
231233
*/
232234
detectCircularDependencies(): string[] {
233235
const cycles: string[] = [];
@@ -369,24 +371,23 @@ export class PluginLoader {
369371
return;
370372
}
371373

372-
// Configuration validation would happen here
373-
// For now, just log that schema is present
374-
this.logger.debug(`Plugin ${plugin.name} has configuration schema`);
374+
// TODO: Configuration validation implementation
375+
// This requires plugin config to be passed during loading
376+
// For now, just validate that the schema exists
377+
this.logger.debug(`Plugin ${plugin.name} has configuration schema (validation not yet implemented)`);
375378
}
376379

377380
private async verifyPluginSignature(plugin: PluginMetadata): Promise<void> {
378381
if (!plugin.signature) {
379382
return;
380383
}
381384

382-
// Signature verification would happen here
383-
// This is a placeholder for security implementation
384-
this.logger.debug(`Plugin ${plugin.name} signature verification (placeholder)`);
385-
385+
// TODO: Plugin signature verification implementation
386386
// In a real implementation:
387387
// 1. Extract public key from trusted source
388388
// 2. Verify signature against plugin code hash
389389
// 3. Throw error if verification fails
390+
this.logger.debug(`Plugin ${plugin.name} signature verification (not yet implemented)`);
390391
}
391392

392393
private async getSingletonService<T>(registration: ServiceRegistration): Promise<T> {

0 commit comments

Comments
 (0)