feat: improved processes#50
Conversation
rupertsworld
commented
Aug 8, 2025
rupertsworld
commented
- fix serializing issue
- clearer process snapshot handling & tests
- fix errors with async
There was a problem hiding this comment.
Pull Request Overview
This PR improves process management with better serialization, clearer process snapshot handling, and async functionality fixes. The changes restructure the process system to separate concerns between the process runtime and individual process containers.
- Restructured process architecture into separate modules for better organization
- Fixed serialization issues by improving type definitions and snapshot handling
- Enhanced async process support with new promise-based process creation
Reviewed Changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Added undefined to JSONValue type definition |
| src/tools.ts | Updated imports and return types for better flexibility |
| src/runtime.ts | Refactored runtime to use constructor-based process spawning |
| src/resources.ts | Extended ResourceIcon interface with JSONValue support |
| src/promise-process.ts | Converted to factory function for creating promise-based processes |
| src/processes/ | Split monolithic processes.ts into modular structure |
| src/messages.ts | Updated import path for ProcessContainer |
| src/kernel.ts | Updated event handlers and process spawning logic |
| src/index.ts | Reordered exports for better organization |
| docs/introduction.md | Updated example to return constructor instead of instance |
| cli/src/tools/ | Added async test tool and updated deep research implementation |
| cli/src/components/MessageBlock.tsx | Added console.log for debugging |
| cli/package.json | Removed watch flag from dev script |
| README.md | Added build step to installation instructions |
| test/ | Removed existing test files and added new runtime tests |
| parameters: z.object({ query: z.string() }), | ||
| execute: ({ query }) => { | ||
| return new PromiseProcess('deep_research', () => deepResearch(query)); | ||
| createPromiseProcess('deep_research', () => deepResearch(query)); |
There was a problem hiding this comment.
The createPromiseProcess function is called but its return value is not returned from the execute function. This will cause the tool to return undefined instead of the expected process constructor.
| createPromiseProcess('deep_research', () => deepResearch(query)); | |
| return createPromiseProcess('deep_research', () => deepResearch(query)); |
| } | ||
|
|
||
| if (msg.type === 'tool-results') { | ||
| console.log(msg); |
There was a problem hiding this comment.
Debug console.log statement should be removed from production code. Consider using a proper logging library or removing this debugging statement.
| console.log(msg); |
| // TODO: Logic to decide whether/when to suspend | ||
| container.handleSuspend(); | ||
| }; | ||
|
|
||
| const ctor = this.constructors.get(snapshot.type)!; | ||
| const process = new ctor(snapshot.state); | ||
| process.name = snapshot.name; | ||
| process.icons = snapshot.icons; | ||
| container.resume = () => { | ||
| // TODO: Logic to decide whether/when to resume | ||
| container.handleResume(); | ||
| }; | ||
|
|
||
| const container = this.registerProcess(process, snapshot.id); | ||
| this.emit('process.restored', { pid: container.id, process: container }); | ||
| container.exit = () => { | ||
| // TODO: Logic to decide whether/when to exit |
There was a problem hiding this comment.
The suspend, resume, and exit methods are being overridden with inline functions that contain TODO comments. This creates unclear responsibilities and incomplete functionality that could lead to runtime issues.
| this.emit('exit'); | ||
| } | ||
|
|
||
| serialize(): ProcessSnapshot { |
There was a problem hiding this comment.
The serialize method may return an incomplete ProcessSnapshot when type is undefined, as it's marked optional in the interface but ProcessSnapshot expects it to be present for proper deserialization.
| serialize(): ProcessSnapshot { | |
| serialize(): ProcessSnapshot { | |
| if (this.type === undefined) { | |
| throw new Error('Cannot serialize ProcessSnapshot: type is undefined.'); | |
| } |
