Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ sandbox:
- src/
- tests/
- /tmp
- ./
timeout:
seconds: 600
gracePeriod: 5
Expand Down
2 changes: 1 addition & 1 deletion coverage.txt
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
✖ failing tests:

test at tests/unit/cron_sync.test.js:80:2
✖ adds a job that exists on disk but not in crontab (26.172986ms)
✖ adds a job that exists on disk but not in crontab (23.302345ms)
AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

3 !== 0
Expand Down
127 changes: 127 additions & 0 deletions openspec/changes/optimistic-gc/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
# Design: Optimistic Garbage Collection

## Architecture

```
┌──────────────────────────────────────────────────────┐
│ index.js (entry) │
│ │
│ Load GC manager ──► gcManager.start(config.memory) │
│ Export gcManager.isIdle() for TUI integration │
└──────────────────┬───────────────────────────────────┘
┌──────────┴──────────┐
▼ ▼
┌───────────────┐ ┌──────────────────┐
│ GC Manager │ │ TUI App │
│ (src/memory/ │ │ (src/tui/app.js)│
│ gc.js) │ │ │
│ │ │ Streaming on │
│ Timer: 30s │ │ ──► setIsBusy │
│ check: │ │ Streaming off │
│ - heap? │ │ ──► setIsIdle │
│ - idle? │ │ │
│ │ │ exports: │
│ global.gc() │ │ setIsBusy, │
└───────────────┘ │ setIsIdle │
└──────────────────┘
```

## Components

### 1. GC Manager (`src/memory/gc.js`)

A class that owns the GC lifecycle:

```javascript
class GcManager {
constructor(config)
start() // starts the timer
stop() // clears the timer, stops collection
_check() // periodic check: heap + idle
_collect() // calls global.gc() if conditions met
}
```

**Check logic (runs every `intervalMs`):**

1. If gc is disabled → skip
2. If `gcAvailable === false` (no `--expose-gc`) → skip silently
3. If not idle (`isIdle === false`) → skip (TUI is streaming/processing)
4. If `now - lastActivityTime < idleTimeoutMs` → skip (not idle long enough; debounce brief pauses)
5. If heapUsed / heapTotal < heapThreshold → skip
6. If heap threshold met → call `global.gc()`
7. Log: "GC triggered: heap at X%"

**Idle detection:**
- `setIdle(false)` called by TUI when streaming starts
- `setIdle(true)` called by TUI when streaming ends
- GC only runs when `isIdle()` returns `true`
- Monotonic idle timeout: TUI must have been idle for at least `idleTimeoutMs` before GC fires. Prevents GC from firing during brief pauses between user input and assistant response.

### 2. TUI Integration (`src/tui/app.js`)

The App component receives callbacks via props:

```javascript
<App
setIsBusy={() => gcManager.setIdle(false)}
setIsIdle={() => gcManager.setIdle(true)}
...
/>
```

Called at streaming boundaries:
- `setIsBusy()` — when `setStatusMessage("Streaming...")` in `handleChat()`
- `setIsIdle()` — when streaming completes or errors out

### 3. Config Schema (`src/config/schemas.js`)

Add to `MemorySchema`:

```javascript
gc: z.object({
enabled: z.boolean().default(true),
intervalMs: z.number().int().positive().default(30000),
idleTimeoutMs: z.number().int().positive().default(10000),
heapThreshold: z.number().min(0.01).max(0.99).default(0.8),
}).default({})
```

### 4. Dockerfile

Update the npm start command to include `--expose-gc`:

```dockerfile
cd /app && exec node --expose-gc index.js --mode interactive
```

## Design Decisions

### Why not `setInterval` during streaming?

GC is synchronous and pauses the event loop. Even a brief pause during streaming would manifest as stutter in the TUI. We only check conditions on the interval; the actual `global.gc()` call is gated behind the idle check.

### Why `heapUsed / heapTotal`?

`heapTotal` is the total heap size currently allocated by V8. `heapUsed` is what's actually in use. The ratio tells us heap efficiency. When high, V8 has allocated more than it needs — a prime candidate for GC.

### Why warn once and skip silently?

If `--expose-gc` isn't available, `global.gc` is `undefined`. We warn on first detection so the user knows the feature is inactive, then suppress further warnings.

### Why 30-second interval and 80% threshold?

Sensible defaults. Frequent enough to catch memory growth, infrequent enough to avoid overhead. Users can tune via config.

### Why not GC in CLI mode?

CLI is short-lived. GC benefits long-running processes. TUI is the primary long-running mode.

### Why only gate on streaming, not all TUI activity?

The idle check currently gates only on streaming state. Tool execution, skill invocation, and other TUI work are not explicitly guarded. This means GC could pause the event loop during a tool call. The tradeoff is simplicity: tracking all TUI activity would require a more complex busy flag. If tool calls become a concern, the idle check can be expanded to include a general `isBusy` flag.

### Timer lifecycle and cleanup

The GC manager's interval timer must be cleaned up on shutdown to avoid dangling timers calling `setIdle()` on unmounted components. The `stop()` method clears the interval. Callers should invoke `stop()` during shutdown (via `registerShutdownHandler`) or in a React `useEffect` cleanup.
49 changes: 49 additions & 0 deletions openspec/changes/optimistic-gc/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Proposal: Optimistic Garbage Collection

## Why

The V8 engine's garbage collector runs automatically, but under sustained memory pressure — especially with the memory system writing files, loading context, and maintaining session state — V8 may not collect aggressively enough before heap usage becomes problematic. An **optimistic GC** gives us proactive control: we trigger V8's collector during idle periods and when heap usage crosses a threshold, keeping memory footprint lean without manual intervention.

## What

Add an optional, timer-driven garbage collection mechanism that:

- Calls `global.gc()` (V8's internal GC) when conditions are met
- Triggers during TUI idle periods (no active streaming/conversation)
- Triggers when heap usage reaches 80% of total heap size
- Runs on a configurable interval (default: 30 seconds)
- Degrades gracefully when `--expose-gc` is not available

## Scope

| In Scope | Out of Scope |
|-----------------------------------------------|------------------------------------------|
| `src/memory/gc.js` — GC manager module | Modifying V8 internals or GC algorithms |
| Config schema for `memory.gc` settings | GC for sandboxed child processes |
| TUI idle detection integration | Memory profiling or leak detection |
| Dockerfile `--expose-gc` flag | CLI mode GC (interactive only) |
| Unit tests | |

## Config

New config section under `memory`:

```yaml
memory:
gc:
enabled: true
intervalMs: 30000
idleTimeoutMs: 10000
heapThreshold: 0.8
```

- `enabled` — Toggle GC on/off (default: `true`)
- `intervalMs` — How often to check conditions (default: 30s)
- `idleTimeoutMs` — Minimum idle time before GC fires (default: 10s)
- `heapThreshold` — Heap usage ratio that triggers GC (default: 0.8 = 80%)

## Impact

- **TUI responsiveness**: GC only fires when idle + streaming is off. No visible impact.
- **Memory footprint**: Proactive collection keeps heap usage lower over time.
- **Docker**: Requires `--expose-gc` Node.js flag in the runtime command.
41 changes: 41 additions & 0 deletions openspec/changes/optimistic-gc/specs/memory-gc/spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
## ADDED Requirements

### Requirement: Optimistic Garbage Collection Trigger
When enabled, the system SHALL periodically check heap usage and trigger V8 garbage collection via `global.gc()` when the heap usage ratio exceeds a configurable threshold and the TUI is idle.

#### Scenario: GC triggers when heap threshold is exceeded during idle
- **WHEN** the TUI is idle (no active streaming) for at least `memory.gc.idleTimeoutMs`
- **AND** `heapUsed / heapTotal >= memory.gc.heapThreshold`
- **THEN** the system calls `global.gc()` and logs the heap usage percentage

#### Scenario: GC does not trigger during active streaming
- **WHEN** the TUI is actively streaming a response
- **THEN** the system skips GC even if heap threshold is exceeded

#### Scenario: GC does not trigger when below threshold
- **WHEN** `heapUsed / heapTotal < memory.gc.heapThreshold`
- **THEN** the system skips GC regardless of idle state

#### Scenario: GC triggers after sustained idle period
- **WHEN** the TUI has been idle (no active streaming) for at least `memory.gc.idleTimeoutMs`
- **AND** `heapUsed / heapTotal >= memory.gc.heapThreshold`
- **THEN** the system calls `global.gc()` and logs the heap usage percentage

### Requirement: GC Graceful Degradation
When `--expose-gc` is not available (i.e., `global.gc` is undefined), the system SHALL log a single warning message on first detection and skip all subsequent GC checks silently.

#### Scenario: GC warns when --expose-gc is not enabled
- **WHEN** the GC manager is constructed and `global.gc` is undefined
- **THEN** the system logs a warning: `[gc] V8 garbage collection not available (node --expose-gc required)`
- **AND** the system does not attempt further GC calls

### Requirement: GC Configuration
The system SHALL support configurable GC parameters via `config.yaml` under `memory.gc`.

#### Scenario: GC can be disabled via config
- **WHEN** `memory.gc.enabled` is set to `false` in config
- **THEN** the GC manager does not start and no GC checks are performed

#### Scenario: GC uses custom interval and thresholds
- **WHEN** `memory.gc.intervalMs`, `memory.gc.idleTimeoutMs`, and `memory.gc.heapThreshold` are configured
- **THEN** the GC manager uses these values instead of defaults (30000ms, 10000ms, 0.8)
91 changes: 91 additions & 0 deletions openspec/changes/optimistic-gc/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
## Implementation Tasks

### 1. Config Schema

- [x] 1.1 Add `gc` sub-object to `MemorySchema` in `src/config/schemas.js` ✅ **Already done**
- [x] `enabled: z.boolean().default(true)`
- [x] `intervalMs: z.number().int().positive().default(30000)`
- [x] `idleTimeoutMs: z.number().int().positive().default(10000)`
- [x] `heapThreshold: z.number().min(0.01).max(0.99).default(0.8)`
- [x] `gc: z.object({...}).default({})`
- [x] 1.2 Add `gc` defaults to `DEFAULT_CONFIG.memory` in `src/config/schemas.js` ✅ **Already done**
- [ ] 1.3 Add test: `tests/unit/config/gc.test.js` — validates gc config parsing
- [ ] Test default gc config values
- [ ] Test custom gc config values
- [ ] Test gc disabled via config
- [ ] Test heapThreshold bounds (rejects 0 and 1)

### 2. GC Manager Module

- [ ] 2.1 Create `src/memory/gc.js` — `GcManager` class
- [ ] `constructor(config)` — stores config, initializes state, checks `global.gc` availability immediately
- [ ] `start()` — starts the interval timer
- [ ] `stop()` — clears interval timer (for cleanup on unmount)
- [ ] `_check()` — periodic check: idle timeout → heap → gc
- [ ] `_collect()` — calls `global.gc()` if conditions met
- [ ] `isIdle()` — getter for idle state
- [ ] `setIdle(value)` — setter for idle state, updates `lastActivityTime`
- [ ] 2.2 Idle detection logic
- [ ] Track `isIdle` boolean (default: true)
- [ ] Track `lastActivityTime` monotonic timestamp (updated on every `setIdle()` call)
- [ ] `_check()` only triggers GC when `isIdle && (now - lastActivityTime >= idleTimeoutMs)`
- [ ] 2.3 Heap check logic
- [ ] Read `process.memoryUsage()`
- [ ] Compute `heapUsed / heapTotal`
- [ ] Compare against `heapThreshold`
- [ ] 2.4 Graceful degradation
- [ ] Check `typeof global.gc === 'function'` in constructor (not on first `_check()`)
- [ ] If unavailable: log warning once, set `gcAvailable = false`
- [ ] If `gcAvailable === false`: skip all subsequent checks silently
- [ ] 2.5 Logging
- [ ] Log GC trigger: `[gc] triggered: heap at X%`
- [ ] Log warning (once): `[gc] V8 GC not available (node --expose-gc required)`
- [ ] 2.6 Export `GcManager` as default export

### 3. TUI Integration

- [ ] 3.1 In `index.js`, instantiate `GcManager` and wire to TUI
- [ ] Import `GcManager` from `./src/memory/gc.js`
- [ ] Create `gcManager = new GcManager(config.memory)`
- [ ] Call `gcManager.start()` if `config.memory.gc?.enabled !== false`
- [ ] Pass `gcManager` callbacks as props to `App` component (see 3.3)
- [ ] 3.2 In `src/tui/app.js`, accept idle callbacks via props
- [ ] Add `setIsBusy` and `setIsIdle` to App component props
- [ ] Use callbacks in `handleChat` closure scope:
- [ ] Call `setIsBusy()` when streaming starts (`setStatusMessage("Streaming...")`)
- [ ] Call `setIsIdle()` when streaming completes (after `setStatusMessage("Received response")`)
- [ ] Call `setIsIdle()` when streaming errors out (in catch block)
- [ ] 3.3 In `index.js`, pass callbacks to App component
- [ ] `setIsBusy: () => gcManager.setIdle(false)`
- [ ] `setIsIdle: () => gcManager.setIdle(true)`
- [ ] 3.4 In `index.js` or `app.js`, ensure timer cleanup
- [ ] Register `gcManager.stop()` on shutdown (via `registerShutdownHandler` or `onExit`)
- [ ] Alternatively: add `useEffect` cleanup in App that calls `gcManager.stop()`

### 4. Dockerfile

- [ ] 4.1 Add `--expose-gc` to the Node.js start command
- [ ] Update package.json `"start"` script to `"node --expose-gc index.js --mode interactive"`
- [ ] (Alternative) Update Dockerfile profile line to use `node --expose-gc index.js --mode interactive` directly
- [ ] Choose one approach and apply consistently

### 5. Tests

- [ ] 5.1 `tests/unit/gc.test.js` — GC Manager unit tests
- [ ] Test: GC triggers when heap threshold exceeded and idle for full timeout period
- [ ] Test: GC does not trigger when not idle
- [ ] Test: GC does not trigger when below threshold
- [ ] Test: GC does not trigger when disabled
- [ ] Test: GC warns once in constructor when --expose-gc unavailable, then skips
- [ ] Test: GC uses custom interval and thresholds from config
- [ ] Test: idle timeout prevents GC during brief idle periods (< idleTimeoutMs)
- [ ] Test: stop() clears the interval timer
- [ ] 5.2 `tests/unit/config/gc.test.js` — Config schema tests (see 1.3)

### 6. Integration / Verification

- [ ] 6.1 Run full test suite: `npm test`
- [ ] 6.2 Verify lint passes: `npm run lint`
- [ ] 6.3 Manual test: Run `node --expose-gc index.js --mode interactive`, verify GC logs appear
- [ ] 6.4 Manual test: Run without `--expose-gc`, verify single warning appears at startup
- [ ] 6.5 Update `README.md` — document `memory.gc` config section
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"main": "index.js",
"bin": "index.js",
"scripts": {
"start": "node index.js --mode interactive",
"start": "node --expose-gc index.js --mode interactive",
"test": "node --test tests/**/*.test.js",
"coverage": "node --test --experimental-test-coverage --test-coverage-exclude=dist/** --test-coverage-exclude=tests/** --test-reporter=spec tests/**/*.test.js 2>&1 | grep -A 1000 \"start of coverage report\" > coverage.txt",
"fix": "oxlint --fix *.js src tests/unit && oxfmt *.js src tests/unit --write",
Expand Down
9 changes: 9 additions & 0 deletions src/config/schemas.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ export const MemorySchema = z.object({
maxEntries: z.number().int().positive().default(10),
})
.default({ ttlDays: 7, maxEntries: 10 }),
gc: z
.object({
enabled: z.boolean().default(true),
intervalMs: z.number().int().positive().default(30000),
idleTimeoutMs: z.number().int().positive().default(10000),
heapThreshold: z.number().min(0.01).max(0.99).default(0.8),
})
.default({}),
});

// --- Telemetry schemas ---
Expand Down Expand Up @@ -242,6 +250,7 @@ export const DEFAULT_CONFIG = {
errorsDir: "memory/errors/",
schedulesDir: "memory/schedules/",
ephemeral: { ttlDays: 7, maxEntries: 10 },
gc: { enabled: true, intervalMs: 30000, idleTimeoutMs: 10000, heapThreshold: 0.8 },
},
telemetry: {
enabled: false,
Expand Down