From 1516185c4d2a71b6689da921602fc73142d7c3ad Mon Sep 17 00:00:00 2001 From: Juergen Kunz Date: Thu, 28 Aug 2025 18:10:33 +0000 Subject: [PATCH] prepare refactor --- readme.plan.md | 271 ++++++++++++++++++++++++++----- ts/cli/commands/process/start.ts | 51 ++++-- 2 files changed, 267 insertions(+), 55 deletions(-) diff --git a/readme.plan.md b/readme.plan.md index d640471..4cba90c 100644 --- a/readme.plan.md +++ b/readme.plan.md @@ -1,56 +1,249 @@ -# TSPM SmartDaemon Service Management Refactor +# TSPM Architecture Refactoring Plan -## Problem +## Current Problems +The current architecture has several issues that make the codebase confusing: -Currently TSPM auto-spawns the daemon as a detached child process, which is improper daemon management. It should use smartdaemon for all lifecycle management and never spawn processes directly. +1. **Flat structure confusion**: All classes are mixed together in the `ts/` directory with a `classes.` prefix naming convention +2. **Unclear boundaries**: It's hard to tell what code runs in the daemon vs the client +3. **Misleading naming**: The `Tspm` class is actually the core ProcessManager, not the overall system +4. **Coupling risk**: Client code could accidentally import daemon internals, bloating bundles +5. **No architectural enforcement**: Nothing prevents cross-boundary imports -## Solution +## Goal +Refactor into a clean 3-folder architecture (daemon/client/shared) with proper separation of concerns and enforced boundaries. -Refactor to use SmartDaemon for proper systemd service integration. +## Key Insights from Architecture Review -## Implementation Tasks +### Why This Separation Makes Sense +After discussion with GPT-5, we identified that: -### Phase 1: Remove Auto-Spawn Behavior +1. **ProcessManager/Monitor/Wrapper are daemon-only**: These classes actually spawn and manage processes. Clients never need them - they only communicate via IPC. -- [x] Remove spawn import from ts/classes.ipcclient.ts -- [x] Delete startDaemon() method from IpcClient -- [x] Update connect() to throw error when daemon not running -- [x] Remove auto-reconnect logic from request() method +2. **The client is just an IPC bridge**: The client (CLI and library users) only needs to send messages to the daemon and receive responses. It should never directly manage processes. -### Phase 2: Create Service Manager +3. **Shared should be minimal**: Only the IPC protocol types and pure utilities should be shared. No Node.js APIs, no file system access. -- [x] Create new file ts/classes.servicemanager.ts -- [x] Implement TspmServiceManager class -- [x] Add getOrCreateService() method -- [x] Add enableService() method -- [x] Add disableService() method -- [x] Add getServiceStatus() method +4. **Protocol is the contract**: The IPC types are the only coupling between client and daemon. This allows independent evolution. -### Phase 3: Update CLI Commands +## Architecture Overview -- [x] Add 'enable' command to CLI -- [x] Add 'disable' command to CLI -- [x] Update 'daemon start' to work without systemd -- [x] Add 'daemon start-service' internal command for systemd -- [x] Update all commands to handle missing daemon gracefully -- [x] Add proper error messages with hints +### Folder Structure +- **ts/daemon/** - Process orchestration (runs in daemon process only) + - Contains all process management logic + - Spawns and monitors actual system processes + - Manages configuration and state + - Never imported by client code -### Phase 4: Update Documentation +- **ts/client/** - IPC communication (runs in CLI/client process) + - Only knows how to talk to the daemon via IPC + - Lightweight - no process management logic + - What library users import when they use TSPM + - Can work in any Node.js environment (or potentially browser) -- [x] Update help text in CLI -- [ ] Update command descriptions -- [x] Add service management section +- **ts/shared/** - Minimal shared contract (protocol & pure utilities) + - **protocol/** - IPC request/response types, error codes, version + - **common/** - Pure utilities with no environment dependencies + - No fs, net, child_process, or Node-specific APIs + - Keep as small as possible to minimize coupling -### Phase 5: Testing +## File Organization Rationale -- [x] Test enable command -- [x] Test disable command -- [x] Test daemon commands -- [x] Test error handling when daemon not running -- [x] Build and verify TypeScript compilation +### What Goes in Daemon +These files are daemon-only because they actually manage processes: +- `processmanager.ts` (was Tspm) - Core process orchestration logic +- `processmonitor.ts` - Monitors memory and restarts processes +- `processwrapper.ts` - Wraps child processes with logging +- `tspm.config.ts` - Persists process configurations to disk +- `tspm.daemon.ts` - Wires everything together, handles IPC requests -## Migration Notes +### What Goes in Client +These files are client-only because they just communicate: +- `tspm.ipcclient.ts` - Sends requests to daemon via Unix socket +- `tspm.servicemanager.ts` - Manages systemd service (delegates to smartdaemon) +- CLI files - Command-line interface that uses the IPC client -- Users will need to run `tspm enable` once after update -- Existing daemon instances will stop working -- Documentation needs updating to explain new behavior +### What Goes in Shared +Only the absolute minimum needed by both: +- `protocol/ipc.types.ts` - Request/response type definitions +- `protocol/error.codes.ts` - Standardized error codes +- `common/utils.errorhandler.ts` - If it's pure (no I/O) +- Parts of `paths.ts` - Constants like socket path (not OS-specific resolution) +- Plugin interfaces only (not loading logic) + +### Critical Design Decisions + +1. **Rename Tspm to ProcessManager**: The class name should reflect what it does +2. **No process management in shared**: ProcessManager, ProcessMonitor, ProcessWrapper are daemon-only +3. **Protocol versioning**: Add version to allow client/daemon compatibility +4. **Enforce boundaries**: Use TypeScript project references to prevent violations +5. **Control exports**: Package.json exports map ensures library users can't import daemon code + +## Detailed Task List + +### Phase 1: Create New Structure +- [ ] Create directory `ts/daemon/` +- [ ] Create directory `ts/client/` +- [ ] Create directory `ts/shared/` +- [ ] Create directory `ts/shared/protocol/` +- [ ] Create directory `ts/shared/common/` + +### Phase 2: Move Daemon Files +- [ ] Move `ts/daemon.ts` → `ts/daemon/index.ts` +- [ ] Move `ts/classes.daemon.ts` → `ts/daemon/tspm.daemon.ts` +- [ ] Move `ts/classes.tspm.ts` → `ts/daemon/processmanager.ts` +- [ ] Move `ts/classes.processmonitor.ts` → `ts/daemon/processmonitor.ts` +- [ ] Move `ts/classes.processwrapper.ts` → `ts/daemon/processwrapper.ts` +- [ ] Move `ts/classes.config.ts` → `ts/daemon/tspm.config.ts` + +### Phase 3: Move Client Files +- [ ] Move `ts/classes.ipcclient.ts` → `ts/client/tspm.ipcclient.ts` +- [ ] Move `ts/classes.servicemanager.ts` → `ts/client/tspm.servicemanager.ts` +- [ ] Create `ts/client/index.ts` barrel export file + +### Phase 4: Move Shared Files +- [ ] Move `ts/ipc.types.ts` → `ts/shared/protocol/ipc.types.ts` +- [ ] Create `ts/shared/protocol/protocol.version.ts` with version constant +- [ ] Create `ts/shared/protocol/error.codes.ts` with standardized error codes +- [ ] Move `ts/utils.errorhandler.ts` → `ts/shared/common/utils.errorhandler.ts` +- [ ] Analyze `ts/paths.ts` - split into constants (shared) vs resolvers (daemon) +- [ ] Move/split `ts/plugins.ts` - interfaces to shared, loaders to daemon + +### Phase 5: Rename Classes +- [ ] In `processmanager.ts`: Rename class `Tspm` → `ProcessManager` +- [ ] Update all references to `Tspm` class to use `ProcessManager` +- [ ] Update constructor in `tspm.daemon.ts` to use `ProcessManager` + +### Phase 6: Update Imports - Daemon Files +- [ ] Update imports in `ts/daemon/index.ts` +- [ ] Update imports in `ts/daemon/tspm.daemon.ts` + - [ ] Change `'./classes.tspm.js'` → `'./processmanager.js'` + - [ ] Change `'./paths.js'` → appropriate shared/daemon path + - [ ] Change `'./ipc.types.js'` → `'../shared/protocol/ipc.types.js'` +- [ ] Update imports in `ts/daemon/processmanager.ts` + - [ ] Change `'./classes.processmonitor.js'` → `'./processmonitor.js'` + - [ ] Change `'./classes.processwrapper.js'` → `'./processwrapper.js'` + - [ ] Change `'./classes.config.js'` → `'./tspm.config.js'` + - [ ] Change `'./utils.errorhandler.js'` → `'../shared/common/utils.errorhandler.js'` +- [ ] Update imports in `ts/daemon/processmonitor.ts` + - [ ] Change `'./classes.processwrapper.js'` → `'./processwrapper.js'` +- [ ] Update imports in `ts/daemon/processwrapper.ts` +- [ ] Update imports in `ts/daemon/tspm.config.ts` + +### Phase 7: Update Imports - Client Files +- [ ] Update imports in `ts/client/tspm.ipcclient.ts` + - [ ] Change `'./paths.js'` → appropriate shared/daemon path + - [ ] Change `'./ipc.types.js'` → `'../shared/protocol/ipc.types.js'` +- [ ] Update imports in `ts/client/tspm.servicemanager.ts` + - [ ] Change `'./paths.js'` → appropriate shared/daemon path +- [ ] Create exports in `ts/client/index.ts` + - [ ] Export TspmIpcClient + - [ ] Export TspmServiceManager + +### Phase 8: Update Imports - CLI Files +- [ ] Update imports in `ts/cli/index.ts` + - [ ] Change `'../classes.ipcclient.js'` → `'../client/tspm.ipcclient.js'` +- [ ] Update imports in `ts/cli/commands/service/enable.ts` + - [ ] Change `'../../../classes.servicemanager.js'` → `'../../../client/tspm.servicemanager.js'` +- [ ] Update imports in `ts/cli/commands/service/disable.ts` + - [ ] Change `'../../../classes.servicemanager.js'` → `'../../../client/tspm.servicemanager.js'` +- [ ] Update imports in `ts/cli/commands/daemon/index.ts` + - [ ] Change `'../../../classes.daemon.js'` → `'../../../daemon/tspm.daemon.js'` + - [ ] Change `'../../../classes.ipcclient.js'` → `'../../../client/tspm.ipcclient.js'` +- [ ] Update imports in `ts/cli/commands/process/*.ts` files + - [ ] Change all `'../../../classes.ipcclient.js'` → `'../../../client/tspm.ipcclient.js'` + - [ ] Change all `'../../../classes.tspm.js'` → `'../../../shared/protocol/ipc.types.js'` (for types) +- [ ] Update imports in `ts/cli/registration/index.ts` + - [ ] Change `'../../classes.ipcclient.js'` → `'../../client/tspm.ipcclient.js'` + +### Phase 9: Update Main Exports +- [ ] Update `ts/index.ts` + - [ ] Remove `export * from './classes.tspm.js'` + - [ ] Remove `export * from './classes.processmonitor.js'` + - [ ] Remove `export * from './classes.processwrapper.js'` + - [ ] Remove `export * from './classes.daemon.js'` + - [ ] Remove `export * from './classes.ipcclient.js'` + - [ ] Remove `export * from './classes.servicemanager.js'` + - [ ] Add `export * from './client/index.js'` + - [ ] Add `export * from './shared/protocol/ipc.types.js'` + - [ ] Add `export { startDaemon } from './daemon/index.js'` + +### Phase 10: Update Package.json +- [ ] Add exports map to package.json: + ```json + "exports": { + ".": "./dist_ts/client/index.js", + "./client": "./dist_ts/client/index.js", + "./daemon": "./dist_ts/daemon/index.js", + "./protocol": "./dist_ts/shared/protocol/ipc.types.js" + } + ``` + +### Phase 11: TypeScript Configuration +- [ ] Create `tsconfig.base.json` with common settings +- [ ] Create `tsconfig.shared.json` for shared code +- [ ] Create `tsconfig.client.json` with reference to shared +- [ ] Create `tsconfig.daemon.json` with reference to shared +- [ ] Update main `tsconfig.json` to use references + +### Phase 12: Testing +- [ ] Run `pnpm run build` and fix any compilation errors +- [ ] Test daemon startup: `./cli.js daemon start` +- [ ] Test process management: `./cli.js start "echo test"` +- [ ] Test client commands: `./cli.js list` +- [ ] Run existing tests: `pnpm test` +- [ ] Update test imports if needed + +### Phase 13: Documentation +- [ ] Update README.md if needed +- [ ] Document the new architecture in a comment at top of ts/index.ts +- [ ] Add comments explaining the separation in each index.ts file + +### Phase 14: Cleanup +- [ ] Delete empty directories from old structure +- [ ] Verify no broken imports remain +- [ ] Run linter and fix any issues +- [ ] Commit with message: "refactor(architecture): reorganize into daemon/client/shared structure" + +## Benefits After Completion + +### Immediate Benefits +- **Clear separation**: Instantly obvious what runs where (daemon vs client) +- **Smaller client bundles**: Client code won't accidentally include ProcessMonitor, ProcessWrapper, etc. +- **Better testing**: Can test client and daemon independently +- **Cleaner imports**: No more confusing `classes.` prefix on everything + +### Architecture Benefits +- **Enforced boundaries**: TypeScript project references prevent cross-imports +- **Protocol as contract**: Client and daemon can evolve independently +- **Version compatibility**: Protocol versioning allows client/daemon version skew +- **Security**: Internal daemon errors don't leak to clients over IPC + +### Future Benefits +- **Browser support**: Clean client could potentially work in browser +- **Embedded mode**: Could add option to run ProcessManager in-process +- **Plugin system**: Clear boundary for plugin interfaces vs implementation +- **Multi-language clients**: Other languages only need to implement IPC protocol + +## Implementation Safeguards (from GPT-5 Review) + +### Boundary Enforcement +- **TypeScript project references**: Separate tsconfig files prevent illegal imports +- **ESLint rules**: Use `import/no-restricted-paths` to catch violations +- **Package.json exports**: Control what external consumers can import + +### Keep Shared Minimal +- **No Node.js APIs**: No fs, net, child_process in shared +- **No environment access**: No process.env, no OS-specific code +- **Pure functions only**: Shared utilities must be environment-agnostic +- **Protocol-focused**: Mainly type definitions and constants + +### Prevent Environment Bleed +- **Split paths.ts**: Constants (shared) vs OS-specific resolution (daemon) +- **Plugin interfaces only**: Loading/discovery stays in daemon +- **No dynamic imports**: Keep shared statically analyzable + +### Future-Proofing +- **Protocol versioning**: Add version field for compatibility +- **Error codes**: Standardized errors instead of string messages +- **Capability negotiation**: Client can query daemon capabilities +- **Subpath exports**: Different entry points for different use cases \ No newline at end of file diff --git a/ts/cli/commands/process/start.ts b/ts/cli/commands/process/start.ts index a19dc85..dcc9045 100644 --- a/ts/cli/commands/process/start.ts +++ b/ts/cli/commands/process/start.ts @@ -10,10 +10,16 @@ export function registerStartCommand(smartcli: plugins.smartcli.Smartcli) { smartcli, 'start', async (argvArg: CliArguments) => { - const script = argvArg._[1]; - if (!script) { - console.error('Error: Please provide a script to run'); - console.log('Usage: tspm start