Skip to content

Commit ff10814

Browse files
committed
Implement TUI circuit breaker and safe logging for interrupt handling
- Add circuit breaker to stop terminal updates on error (e.g. SIGINT) - Refactor Logger to use file-based error logging instead of stderr - Add comprehensive tests for logger stability and plugin fault tolerance
1 parent 6d47542 commit ff10814

File tree

6 files changed

+245
-8
lines changed

6 files changed

+245
-8
lines changed

AGENTS.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ PAI is the core Personal AI Infrastructure agent implemented by this plugin. It
1010
## Capabilities
1111
- **Context Management:** Automatically loads the `core/SKILL.md` file from the user's skills directory at the start of each session, injecting it as the core system prompt.
1212
- **Event Logging:** Logs all session events, tool calls, and message updates to the `.opencode/history/raw-outputs` directory for audit and debugging purposes.
13-
- **Session Status:** Updates the terminal tab title to reflect the current activity or thought process of the agent.
13+
- **Fault-Tolerant Status Updates:** Updates the terminal tab title to reflect activity. Includes a "circuit breaker" to automatically disable updates if interrupts (e.g., ESC key) are detected, preventing TUI corruption.
1414
- **Session Summarization:** Generates a summary of the session in `.opencode/history/sessions` when a session ends (if configured).
1515

1616
## Configuration
@@ -22,8 +22,8 @@ The agent's behavior contains some configurable elements via environment variabl
2222
| `USER_NAME` | Environment variable to override the user's name (default: "Engineer"). |
2323

2424
## Codebase Structure
25-
- `src/index.ts`: The plugin entry point. It defines the `PAIPlugin` export and sets up hooks for event listening (`event`, `chat.message`, `tool.execute.after`).
25+
- `src/index.ts`: The plugin entry point. Handles hooks and implements the TUI circuit breaker logic.
2626
- `src/lib/context-loader.ts`: Responsible for reading and processing the `core/SKILL.md` file, injecting environment variables.
27-
- `src/lib/logger.ts`: Implements a buffering JSONL logger. It captures events and tool outputs, writing them to `raw-outputs/{session-id}.jsonl`. It handles file creation and flushing.
28-
- `src/lib/notifier.ts`: A utility to send POST requests to a local voice server (defaulting to `localhost:8888`). It fails gracefully (silently ignores errors) if the server is not unreachable.
27+
- `src/lib/logger.ts`: Implements a buffering JSONL logger. Captures events safely, redirecting internal errors to `system-logs` to protect the TUI.
28+
- `src/lib/notifier.ts`: A utility to send POST requests to a local voice server (defaulting to `localhost:8888`).
2929
- `src/lib/paths.ts`: Contains helper functions for resolving paths (`getHistoryDir`, `getRawOutputsDir`, `getSkillsDir`) and handling environment variable expansion in paths.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ Once installed and registered, the plugin will automatically:
3333

3434
* Load the `CORE/SKILL.md` file as core context at the start of each session.
3535
* Log all events and tool calls to the `.opencode/history/raw-outputs` directory.
36-
* Update the terminal tab title with the current session status.
36+
* Update the terminal tab title with the current session status (failsafe mode included).
3737
* Create a session summary in `.opencode/history/sessions` when a session ends.
3838

3939
---

src/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ function shouldSkipEvent(event: Event, sessionId: string | null): boolean {
3535
export const PAIPlugin: Plugin = async ({ project, directory, $, client }) => {
3636
let logger: Logger | null = null;
3737
let currentSessionId: string | null = null;
38+
let titleUpdatesDisabled: boolean = false;
3839

3940
const hooks: Hooks = {
4041
event: async ({ event }: { event: Event }) => {
@@ -63,7 +64,7 @@ export const PAIPlugin: Plugin = async ({ project, directory, $, client }) => {
6364
if (event.type === 'message.part.updated') {
6465
const part = event.properties.part;
6566

66-
if (part.type === 'text') {
67+
if (part.type === 'text' && !titleUpdatesDisabled) {
6768
const prompt = part.text;
6869
let tabTitle = 'Processing request...';
6970

@@ -86,7 +87,10 @@ export const PAIPlugin: Plugin = async ({ project, directory, $, client }) => {
8687
try {
8788
process.stderr.write(`\x1b]0;${titleWithEmoji}\x07`);
8889
} catch (e) {
89-
// ignore
90+
titleUpdatesDisabled = true;
91+
if (logger) {
92+
logger.logError('Terminal Title Update', e);
93+
}
9094
}
9195
}
9296
}

src/lib/logger.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,33 @@ export class Logger {
135135
this.writeEvent('ToolUse', payload, toolName, toolInput);
136136
}
137137

138+
public logError(context: string, error: any): void {
139+
try {
140+
const now = new Date();
141+
const pstDate = new Date(now.toLocaleString('en-US', { timeZone: process.env.TIME_ZONE || 'America/Los_Angeles' }));
142+
const year = pstDate.getFullYear();
143+
const month = String(pstDate.getMonth() + 1).padStart(2, '0');
144+
const day = String(pstDate.getDate()).padStart(2, '0');
145+
146+
const filename = `${year}-${month}-${day}_errors.log`;
147+
const filePath = getHistoryFilePath('system-logs', filename);
148+
const dir = dirname(filePath);
149+
150+
if (!existsSync(dir)) {
151+
mkdirSync(dir, { recursive: true });
152+
}
153+
154+
const timestamp = this.getPSTTimestamp();
155+
const errorMessage = error instanceof Error ? error.message : String(error);
156+
const stack = error instanceof Error ? error.stack : '';
157+
158+
const logEntry = `[${timestamp}] [${context}] ${errorMessage}\n${stack}\n-------------------\n`;
159+
appendFileSync(filePath, logEntry, 'utf-8');
160+
} catch (e) {
161+
// Intentionally silent - TUI protection
162+
}
163+
}
164+
138165
// Core write method
139166
private writeEvent(eventType: string, payload: any, toolName?: string, toolInput?: any): void {
140167
const sessionId = this.sessionId;
@@ -164,7 +191,7 @@ export class Logger {
164191
const jsonLine = JSON.stringify(hookEvent) + '\n';
165192
appendFileSync(eventsFile, jsonLine, 'utf-8');
166193
} catch (error) {
167-
console.error('Event capture error:', error);
194+
this.logError('EventCapture', error);
168195
}
169196
}
170197

tests/logger.test.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import { describe, it, expect, mock, beforeEach, afterEach } from 'bun:test';
2+
import * as fs from 'fs';
3+
import * as path from 'path';
4+
5+
const TEMP_DIR = '/tmp/pai-test-' + Date.now();
6+
7+
// Calculate absolute path to the module we want to mock
8+
const pathsModulePath = path.resolve(__dirname, '../src/lib/paths.ts');
9+
10+
mock.module(pathsModulePath, () => {
11+
return {
12+
PAI_DIR: TEMP_DIR,
13+
getHistoryFilePath: (subdir: string, filename: string) => path.join(TEMP_DIR, 'history', subdir, filename),
14+
join: path.join,
15+
dirname: path.dirname,
16+
// Add other exports if needed
17+
HOOKS_DIR: path.join(TEMP_DIR, 'hooks'),
18+
SKILLS_DIR: path.join(TEMP_DIR, 'skills'),
19+
AGENTS_DIR: path.join(TEMP_DIR, 'agents'),
20+
HISTORY_DIR: path.join(TEMP_DIR, 'history'),
21+
COMMANDS_DIR: path.join(TEMP_DIR, 'commands'),
22+
};
23+
});
24+
25+
describe('Logger Integration', () => {
26+
let Logger: any;
27+
let logger: any;
28+
29+
beforeEach(async () => {
30+
// Clean up temp dir
31+
if (fs.existsSync(TEMP_DIR)) {
32+
fs.rmSync(TEMP_DIR, { recursive: true, force: true });
33+
}
34+
fs.mkdirSync(TEMP_DIR, { recursive: true });
35+
36+
// Dynamic import to get the Logger class
37+
// We append a query string to force reload if possible? Bun doesn't support that for FS modules easily.
38+
// But since we are mocking the dependency, if Logger is re-evaluated or if it uses the mocked module, it should work.
39+
// Ideally we'd use a fresh Logger class.
40+
41+
// We might need to use the absolute path for logger too to ensure we are importing the same thing
42+
const loggerPath = path.resolve(__dirname, '../src/lib/logger.ts');
43+
const module = await import(loggerPath);
44+
Logger = module.Logger;
45+
46+
logger = new Logger('test-session', '/tmp/project');
47+
});
48+
49+
afterEach(() => {
50+
if (fs.existsSync(TEMP_DIR)) {
51+
fs.rmSync(TEMP_DIR, { recursive: true, force: true });
52+
}
53+
mock.restore();
54+
});
55+
56+
it('should log events to a file', async () => {
57+
const event = {
58+
type: 'test.event',
59+
properties: { foo: 'bar' }
60+
};
61+
62+
logger.logOpenCodeEvent(event);
63+
64+
const rawOutputsDir = path.join(TEMP_DIR, 'history', 'raw-outputs');
65+
expect(fs.existsSync(rawOutputsDir)).toBe(true);
66+
67+
const files = fs.readdirSync(rawOutputsDir);
68+
expect(files.length).toBe(1);
69+
expect(files[0].endsWith('.jsonl')).toBe(true);
70+
71+
const content = fs.readFileSync(path.join(rawOutputsDir, files[0]), 'utf-8');
72+
expect(content).toContain('test.event');
73+
expect(content).toContain('"foo":"bar"');
74+
});
75+
76+
it('should handle errors by logging to error file', async () => {
77+
// 1. Setup
78+
logger.logOpenCodeEvent({ type: 'init' });
79+
const rawOutputsDir = path.join(TEMP_DIR, 'history', 'raw-outputs');
80+
const files = fs.readdirSync(rawOutputsDir);
81+
const eventsFile = path.join(rawOutputsDir, files[0]);
82+
83+
// Delete file and replace with directory to force EISDIR
84+
fs.unlinkSync(eventsFile);
85+
fs.mkdirSync(eventsFile);
86+
87+
// 2. Trigger event logging
88+
const event = {
89+
type: 'test.event.fail',
90+
properties: { foo: 'baz' }
91+
};
92+
93+
expect(() => logger.logOpenCodeEvent(event)).not.toThrow();
94+
95+
// 3. Verify error log
96+
const systemLogsDir = path.join(TEMP_DIR, 'history', 'system-logs');
97+
expect(fs.existsSync(systemLogsDir)).toBe(true);
98+
99+
const logFiles = fs.readdirSync(systemLogsDir);
100+
expect(logFiles.length).toBeGreaterThan(0);
101+
expect(logFiles[0].endsWith('_errors.log')).toBe(true);
102+
103+
const errorContent = fs.readFileSync(path.join(systemLogsDir, logFiles[0]), 'utf-8');
104+
expect(errorContent).toContain('EventCapture');
105+
expect(errorContent).toContain('EISDIR');
106+
});
107+
});

tests/plugin.test.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import { describe, it, expect, mock, beforeEach, afterEach } from 'bun:test';
2+
import { PAIPlugin } from '../src/index';
3+
import * as fs from 'fs';
4+
import * as path from 'path';
5+
6+
// Use a temp dir to avoid polluting real history
7+
const TEMP_DIR = '/tmp/pai-test-plugin-' + Date.now();
8+
9+
// Mock paths to redirect logs
10+
const pathsModulePath = path.resolve(__dirname, '../src/lib/paths.ts');
11+
mock.module(pathsModulePath, () => {
12+
return {
13+
PAI_DIR: TEMP_DIR,
14+
getHistoryFilePath: (subdir: string, filename: string) => path.join(TEMP_DIR, 'history', subdir, filename),
15+
join: path.join,
16+
dirname: path.dirname,
17+
};
18+
});
19+
20+
describe('PAIPlugin Circuit Breaker', () => {
21+
let originalStderrWrite: any;
22+
let stderrWriteMock: any;
23+
24+
beforeEach(() => {
25+
// Setup temp dir
26+
if (fs.existsSync(TEMP_DIR)) {
27+
fs.rmSync(TEMP_DIR, { recursive: true, force: true });
28+
}
29+
fs.mkdirSync(TEMP_DIR, { recursive: true });
30+
31+
originalStderrWrite = process.stderr.write;
32+
stderrWriteMock = mock(() => {});
33+
process.stderr.write = stderrWriteMock;
34+
});
35+
36+
afterEach(() => {
37+
process.stderr.write = originalStderrWrite;
38+
if (fs.existsSync(TEMP_DIR)) {
39+
fs.rmSync(TEMP_DIR, { recursive: true, force: true });
40+
}
41+
mock.restore();
42+
});
43+
44+
it('should stop updating title after an error', async () => {
45+
const plugin = await PAIPlugin({
46+
project: { worktree: '/tmp' },
47+
directory: '/tmp',
48+
$: {},
49+
client: {}
50+
} as any);
51+
52+
const eventHook = plugin.event;
53+
if (!eventHook) throw new Error('Event hook not found');
54+
55+
// 1. First call - succeeds
56+
await eventHook({
57+
event: {
58+
type: 'message.part.updated',
59+
properties: {
60+
part: { type: 'text', text: 'hello world' }
61+
}
62+
} as any
63+
});
64+
65+
expect(stderrWriteMock).toHaveBeenCalledTimes(1);
66+
67+
// 2. Second call - make it fail
68+
stderrWriteMock.mockImplementationOnce(() => {
69+
throw new Error('EPIPE');
70+
});
71+
72+
await eventHook({
73+
event: {
74+
type: 'message.part.updated',
75+
properties: {
76+
part: { type: 'text', text: 'generating code' }
77+
}
78+
} as any
79+
});
80+
81+
// Should have been called (and failed)
82+
expect(stderrWriteMock).toHaveBeenCalledTimes(2);
83+
84+
// 3. Third call - should be skipped due to circuit breaker
85+
stderrWriteMock.mockClear();
86+
87+
await eventHook({
88+
event: {
89+
type: 'message.part.updated',
90+
properties: {
91+
part: { type: 'text', text: 'still generating' }
92+
}
93+
} as any
94+
});
95+
96+
// Should NOT have been called
97+
expect(stderrWriteMock).toHaveBeenCalledTimes(0);
98+
});
99+
});

0 commit comments

Comments
 (0)