Add real-time session event monitoring to execution screen#12
Add real-time session event monitoring to execution screen#12
Conversation
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
…ts, document event subscription Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds real-time session event monitoring to provide users with granular visibility into Copilot SDK agent behavior during task execution. The implementation subscribes to SDK session events (tool executions, progress updates, token usage, errors) and displays them in a toggleable event log panel accessible via the e keyboard shortcut.
Changes:
- Added session event subscription infrastructure in
copilot.tswithSessionEventDatainterface and callback plumbing throughsendPromptandsendPromptSync - Extended executor callbacks to forward events with task context (
onSessionEventreceives taskId + event data) - Implemented event log UI in execute screen with bounded 100-event memory, showing last 8 events with timestamps and color-coded error highlighting
- Added documentation for the new feature in README with usage instructions
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/copilot.ts | Added SessionEventData interface, onSessionEvent callback to StreamCallbacks, and session.on() subscription to capture all SDK events |
| src/services/copilot.test.ts | Added type structure tests for SessionEventData covering tool execution, usage, and error events |
| src/services/executor.ts | Extended ExecutionCallbacks with optional onSessionEvent, forwarding events from copilot.ts with taskId context for both regular and init tasks |
| src/services/executor.test.ts | Added tests verifying onSessionEvent callback signature and multi-task event handling |
| src/screens/execute.tsx | Implemented event log UI with DisplayEvent interface, formatSessionEvent helper, toggle functionality (e key), bounded 100-event buffer, and last-8 display panel |
| README.md | Added keyboard shortcut table entries and "Real-Time Session Event Monitoring" feature section with usage instructions |
src/screens/execute.tsx
Outdated
| {visible.map((evt, i) => ( | ||
| <Box key={i}> | ||
| <Text color="gray">[{evt.timestamp}] </Text> | ||
| <Text color="cyan">{evt.taskId}: </Text> | ||
| <Text color={evt.isError ? 'red' : 'white'}>{evt.message}</Text> | ||
| </Box> | ||
| ))} |
There was a problem hiding this comment.
Using array index as React key can cause rendering issues when the event log is updated. Since events are bounded to the last 100 and displayed items are sliced from the end, the same index could refer to different events after updates. Consider adding a unique identifier to DisplayEvent (like a counter or timestamp+taskId combination) and use that as the key instead of the array index.
There was a problem hiding this comment.
Fixed in d9709b9. Added a unique id field to DisplayEvent using a counter-based approach (taskId-timestamp-counter), and updated the map to use evt.id as the React key instead of the array index.
| import { describe, it, expect, vi } from 'vitest'; | ||
| import type { SessionEventData } from './copilot.js'; | ||
|
|
||
| describe('SessionEventData', () => { | ||
| it('should have correct type structure', () => { | ||
| const mockEvent: SessionEventData = { | ||
| type: 'tool.execution_start', | ||
| timestamp: new Date().toISOString(), | ||
| data: { toolName: 'test-tool' }, | ||
| }; | ||
|
|
||
| expect(mockEvent.type).toBe('tool.execution_start'); | ||
| expect(mockEvent.timestamp).toBeDefined(); | ||
| expect(mockEvent.data).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should handle tool.execution_complete events', () => { | ||
| const mockEvent: SessionEventData = { | ||
| type: 'tool.execution_complete', | ||
| timestamp: new Date().toISOString(), | ||
| data: { | ||
| toolCallId: 'test-123', | ||
| success: true, | ||
| result: { content: 'Task completed' }, | ||
| }, | ||
| }; | ||
|
|
||
| expect(mockEvent.type).toBe('tool.execution_complete'); | ||
| expect(mockEvent.data).toHaveProperty('success'); | ||
| }); | ||
|
|
||
| it('should handle session.error events', () => { | ||
| const mockEvent: SessionEventData = { | ||
| type: 'session.error', | ||
| timestamp: new Date().toISOString(), | ||
| data: { | ||
| errorType: 'timeout', | ||
| message: 'Request timed out', | ||
| }, | ||
| }; | ||
|
|
||
| expect(mockEvent.type).toBe('session.error'); | ||
| expect((mockEvent.data as { message: string }).message).toBe('Request timed out'); | ||
| }); | ||
|
|
||
| it('should handle assistant.usage events', () => { | ||
| const mockEvent: SessionEventData = { | ||
| type: 'assistant.usage', | ||
| timestamp: new Date().toISOString(), | ||
| data: { | ||
| model: 'claude-sonnet-4', | ||
| inputTokens: 100, | ||
| outputTokens: 50, | ||
| }, | ||
| }; | ||
|
|
||
| expect(mockEvent.type).toBe('assistant.usage'); | ||
| expect((mockEvent.data as { inputTokens: number }).inputTokens).toBe(100); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The tests in this file only verify the interface type structure and don't test actual SDK integration or event callback behavior. Consider adding integration tests that mock the SDK's session.on() method to verify that events are properly captured and forwarded through the callback chain.
There was a problem hiding this comment.
Enhanced in 81acab6. Added comprehensive tests for StreamCallbacks including:
- Event callback structure validation
- Multiple sequential event handling
- Event data preservation when forwarding
- Tests verify callbacks properly handle tool.execution_start, tool.execution_progress, tool.execution_complete, session.error, and assistant.usage events
| import { describe, it, expect, vi } from 'vitest'; | ||
| import type { ExecutionCallbacks } from './executor.js'; | ||
| import type { SessionEventData } from './copilot.js'; | ||
|
|
||
| describe('ExecutionCallbacks with session events', () => { | ||
| it('should define onSessionEvent callback', () => { | ||
| const mockCallback: ExecutionCallbacks = { | ||
| onTaskStart: vi.fn(), | ||
| onTaskDelta: vi.fn(), | ||
| onTaskDone: vi.fn(), | ||
| onTaskFailed: vi.fn(), | ||
| onBatchComplete: vi.fn(), | ||
| onAllDone: vi.fn(), | ||
| onSessionEvent: vi.fn(), | ||
| }; | ||
|
|
||
| expect(mockCallback.onSessionEvent).toBeDefined(); | ||
| expect(typeof mockCallback.onSessionEvent).toBe('function'); | ||
| }); | ||
|
|
||
| it('should call onSessionEvent with taskId and event data', () => { | ||
| const onSessionEvent = vi.fn(); | ||
|
|
||
| const mockEvent: SessionEventData = { | ||
| type: 'tool.execution_start', | ||
| timestamp: new Date().toISOString(), | ||
| data: { toolName: 'bash' }, | ||
| }; | ||
|
|
||
| onSessionEvent('task-1', mockEvent); | ||
|
|
||
| expect(onSessionEvent).toHaveBeenCalledWith('task-1', mockEvent); | ||
| expect(onSessionEvent).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('should handle multiple session events for different tasks', () => { | ||
| const onSessionEvent = vi.fn(); | ||
|
|
||
| const event1: SessionEventData = { | ||
| type: 'tool.execution_start', | ||
| timestamp: new Date().toISOString(), | ||
| data: { toolName: 'bash' }, | ||
| }; | ||
|
|
||
| const event2: SessionEventData = { | ||
| type: 'tool.execution_complete', | ||
| timestamp: new Date().toISOString(), | ||
| data: { success: true }, | ||
| }; | ||
|
|
||
| onSessionEvent('task-1', event1); | ||
| onSessionEvent('task-2', event2); | ||
|
|
||
| expect(onSessionEvent).toHaveBeenCalledTimes(2); | ||
| expect(onSessionEvent).toHaveBeenNthCalledWith(1, 'task-1', event1); | ||
| expect(onSessionEvent).toHaveBeenNthCalledWith(2, 'task-2', event2); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The tests only verify that callbacks can be called with the correct signature, but don't test the actual event forwarding behavior in the executor. Consider adding tests that verify the executor correctly forwards events from sendPromptSync through the onSessionEvent callback with the appropriate taskId context.
There was a problem hiding this comment.
Enhanced in 81acab6. Added tests that verify:
- Event forwarding with taskId context enrichment
- Complete task lifecycle with event flow
- Multiple tasks with correct taskId assignment
- Error event handling with taskId context
- Event data structure preservation through the forwarding chain
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
…g validation Co-authored-by: colindembovsky <1932561+colindembovsky@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Implementation Plan: Real-Time Session Event Monitoring
✅ All tasks completed!
idfield to DisplayEventRecent Changes
copilot.test.tswith comprehensive StreamCallbacks testingexecutor.test.tswith event forwarding and lifecycle validationOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.