Conversation
rupertsworld
commented
Jul 23, 2025
- Add ability to create background processes
- Add example research project to CLI
There was a problem hiding this comment.
Pull Request Overview
This pull request implements a background process management system for the Unternet Kernel. It refactors the core architecture from an Interpreter class to a Kernel class with an integrated runtime that can spawn and manage long-running processes.
- Replaces the
Interpreterwith a newKernelclass that includes process management capabilities - Introduces a
Runtimesystem for spawning and managing background processes - Updates the tool system to support returning
Processinstances for long-running operations
Reviewed Changes
Copilot reviewed 42 out of 45 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/tools.test.ts | Updates tool tests to use new createTool function and Tool interface |
| test/runtime.test.ts | Adds comprehensive test coverage for the new Runtime class |
| test/promise-process.test.ts | Tests for the PromiseProcess class for async operations |
| test/processes.test.ts | Tests for the core Process and ProcessContainer classes |
| test/messages.test.ts | Updates message type constants and adds SystemMessage tests |
| test/interpreter.test.ts | Renames to kernel tests and updates to new Kernel API |
| src/types.ts | Changes null type to Record<string, never> in JSONValue |
| src/tools.ts | Complete rewrite with new Tool interface and createTool function |
| src/stream.ts | New streaming implementation for message processing |
| src/runtime.ts | Core Runtime class for process management |
| src/processes.ts | Process and ProcessContainer classes for stateful operations |
| src/promise-process.ts | Helper class for Promise-based processes |
| src/messages.ts | Updates message types and rendering logic |
| src/kernel.ts | New Kernel class replacing Interpreter |
| src/interpreter.ts | Simplified interpreter for basic streaming |
| src/index.ts | Updates exports to new module structure |
| src/emitter.ts | Adds once() method to event emitter |
Files not reviewed (1)
- cli/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)
test/promise-process.test.ts:61
- This test is skipped and indicates missing error handling for promise rejections in PromiseProcess. The TODO comment suggests this is a known gap in functionality that should be addressed.
it.skip('handles rejected promises', async () => {
src/promise-process.ts:18
- The method name 'await' is confusing as it shadows the JavaScript await keyword. Consider renaming to 'awaitPromise' or 'executePromise' for clarity.
async await(promise: () => Promise<unknown>) {
| */ | ||
| deactivate(): void | Promise<void> {} | ||
|
|
||
| private exit() { |
There was a problem hiding this comment.
The exit() method is marked as private but needs to be called by subclasses like PromiseProcess. This should be protected instead of private to allow proper inheritance.
| private exit() { | |
| protected exit() { |
| const output = await promise(); | ||
| this.setState({ status: 'completed', output }); | ||
| this.emit('tool-result', { output }); | ||
| this.exit(); |
There was a problem hiding this comment.
The promise execution lacks error handling. If the promise rejects, it will cause an unhandled promise rejection. Add try-catch to handle errors and update state accordingly.
| const output = await promise(); | |
| this.setState({ status: 'completed', output }); | |
| this.emit('tool-result', { output }); | |
| this.exit(); | |
| try { | |
| const output = await promise(); | |
| this.setState({ status: 'completed', output }); | |
| this.emit('tool-result', { output }); | |
| } catch (error) { | |
| this.setState({ status: 'failed', output: error }); | |
| this.emit('tool-error', { error }); | |
| } finally { | |
| this.exit(); | |
| } |
| | number | ||
| | boolean | ||
| | null | ||
| | Record<string, never> |
There was a problem hiding this comment.
Changing from null to Record<string, never> in JSONValue type is a breaking change that may affect serialization/deserialization. This change should be documented or reconsidered as it deviates from standard JSON which includes null values.
| | Record<string, never> | |
| | null |