feat: refined processes#47
Conversation
rupertsworld
commented
Aug 1, 2025
- change event names from runtime
- improve process container wrapping
- remove deprecated files in CLI
- add constants file
- improve documentation
- small refactor of message types
There was a problem hiding this comment.
Pull Request Overview
This PR refines the kernel processes and messaging system with updated APIs and improved architecture. The changes modernize the message creation pattern, enhance process management capabilities, and clean up deprecated code.
- Updated message creation API to use a type-first approach with
createMessage(type, options) - Enhanced process container architecture with better lifecycle management and serialization
- Refined event naming convention from snake_case to dot notation for better organization
Reviewed Changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/messages.ts | Updated createMessage function to use type-first parameter approach |
| src/processes.ts | Enhanced ProcessContainer with improved lifecycle and serialization capabilities |
| src/runtime.ts | Refactored with dotted event names and added process restoration functionality |
| src/tools.ts | Simplified tool creation with relaxed type constraints |
| src/stream.ts | Updated to use new message creation API and renamed MessageMetadata to BaseMessage |
| src/kernel.ts | Integrated new runtime events and simplified message creation calls |
| test/*.test.ts | Updated all tests to use new createMessage API |
| cli/src/hooks/useKernel.ts | Modified to use Interpreter instead of Kernel (appears incomplete) |
| docs/* | Removed several documentation files and updated introduction with comprehensive examples |
| cli/deprecated/* | Removed deprecated files |
Comments suppressed due to low confidence (1)
package.json:50
- Zod version 3.25.76 does not exist. The latest stable version of Zod is in the 3.23.x range. This version should be corrected to an actual released version.
"zod": "^3.25.76"
| // export function createTool<TSchema extends ZodType<any, ZodTypeDef, any>>(tool: { | ||
| // name: string; | ||
| // type?: string; | ||
| // description?: string; | ||
| // parameters: TSchema; | ||
| // execute?: ( | ||
| // args: z.infer<TSchema> | ||
| // ) => JSONValue | Promise<JSONValue> | Process | void; | ||
| // }): Tool<TSchema>; |
There was a problem hiding this comment.
This commented-out function overload should be removed if it's no longer needed, or properly implemented if it's still required. Commented code reduces maintainability.
| // export function createTool<TSchema extends ZodType<any, ZodTypeDef, any>>(tool: { | |
| // name: string; | |
| // type?: string; | |
| // description?: string; | |
| // parameters: TSchema; | |
| // execute?: ( | |
| // args: z.infer<TSchema> | |
| // ) => JSONValue | Promise<JSONValue> | Process | void; | |
| // }): Tool<TSchema>; |
| export function createTool(tool: { | ||
| name: string; | ||
| type?: string; | ||
| description?: string; | ||
| execute?: ( | ||
| args: Record<string, never> | ||
| ) => JSONValue | Promise<JSONValue> | Process; | ||
| parameters: any; | ||
| execute?: (args: any) => JSONValue | Promise<JSONValue> | Process | void; | ||
| }): Tool<undefined>; | ||
| export function createTool(tool: any): any { |
There was a problem hiding this comment.
Using 'any' type for parameters removes type safety. Consider using a more specific type or generic constraint to maintain type checking benefits.
| export function createTool(tool: { | ||
| name: string; | ||
| type?: string; | ||
| description?: string; | ||
| execute?: ( | ||
| args: Record<string, never> | ||
| ) => JSONValue | Promise<JSONValue> | Process; | ||
| parameters: any; | ||
| execute?: (args: any) => JSONValue | Promise<JSONValue> | Process | void; | ||
| }): Tool<undefined>; |
There was a problem hiding this comment.
Using 'any' type for args parameter removes type safety. The execute function should maintain proper typing based on the parameters schema.
| // TODO | ||
| suspend() {} |
There was a problem hiding this comment.
Empty TODO comment should either be removed or include specific details about what needs to be implemented in the suspend method.
| // TODO | |
| suspend() {} | |
| suspend() { | |
| this.handleSuspend(); | |
| } |
| this._process?.deactivate(); | ||
| this.emit('suspend'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Empty resume method should either have an implementation or include documentation explaining why it's empty and what the handleResume method does instead.
| /** | |
| * The `resume` method is intentionally left empty. It serves as a placeholder | |
| * for subclasses to override if needed. The actual resumption logic is handled | |
| * by the `handleResume` method, which activates the process and emits a 'resume' event. | |
| */ |
| this._process?.activate(); | ||
| this.emit('resume'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Empty exit method should either have an implementation or include documentation explaining why it's empty and what the handleExit method does instead.
| /** | |
| * The `exit` method is intentionally left empty because the actual exit logic | |
| * is handled by the `handleExit` method. This method serves as a placeholder | |
| * or entry point for external calls to trigger the exit process. | |
| */ |
| return; | ||
| // throw new Error(`Unknown or invalid tool: ${name}`); |
There was a problem hiding this comment.
Silently returning when a tool is not found or lacks execute function could mask configuration errors. Consider logging this condition or providing feedback that the tool call was ignored.
| return; | |
| // throw new Error(`Unknown or invalid tool: ${name}`); | |
| console.warn(`Tool "${name}" not found or lacks an execute function. Ignoring this tool call.`); | |
| continue; |
| Message, | ||
| ProcessContainer, | ||
| } from '@unternet/kernel'; | ||
| import { Interpreter, Message, ProcessContainer } from '@unternet/kernel'; |
There was a problem hiding this comment.
The Interpreter import appears to be incorrect since the interpreter.ts file was deleted in this PR. This will cause a compilation error.
| ProcessContainer, | ||
| } from '@unternet/kernel'; | ||
| import { Interpreter, Message, ProcessContainer } from '@unternet/kernel'; | ||
| import { Runtime } from 'inspector/promises'; |
There was a problem hiding this comment.
This import appears to be importing Node.js inspector Runtime instead of the local Runtime class. Should likely be 'import { Runtime } from '@unternet/kernel';'
| import { Runtime } from 'inspector/promises'; | |
| import { Runtime } from '@unternet/kernel'; |
| import { Runtime } from 'inspector/promises'; | ||
| import { useState, useEffect, useRef } from 'react'; | ||
|
|
||
| export function useKernel(opts: KernelOpts) { |
There was a problem hiding this comment.
KernelOpts type is referenced but not imported, which will cause a compilation error.