Add event log service, diff panel, conflict broker tests and IPC updates#54
Merged
Conversation
- Add Phase 7 domain types: ToolRiskLevel, ToolName, ToolClassification, ToolCallRequest, ToolCallResponse, ApprovalDecision, PatchSet, PatchResult, Conflict, ConflictResolution, FileHashResult, SensitivePathCheckResult - Add type guards for all new Phase 7 types - Implement PermissionBroker: deny-first policy, allow rules, sensitive path escalation, approval queue with timeout - Implement ConflictBroker: conflict detection via hash comparison, patch application with atomic operations, risk classification - Implement ToolRouter: 8 file tools (readFile, searchFiles, listDirectory, proposePatch, applyPatch, writeFile, deleteFile, renameFile) with permission checking and sensitive path protection - Add IPC channels and preload API for tool calls, approvals, patches, conflicts, sensitive path checks, file hashes - Add main process IPC handlers for Phase 7 operations - Add standalone file operations (deleteFileStandalone, renameFileStandalone, listDirectoryStandalone, searchFilesStandalone) - Add getWorkspaceRoots() to WorkspaceService - 101 new unit tests (1020 total), all quality gates pass - Architecture: 0 dependency violations (56 modules, 142 dependencies)
- tests/unit/permission-broker.test.ts: Replace Math.random() (S2245 PRNG) with simple incrementing counter for test callId generation - packages/services/src/workspace-service.ts: Remove duplicate standalone isPathExcluded and matchesIncludeFile functions, inline the one-liner logic directly in searchInDirRecursive to eliminate code duplication
…nitive complexity, await w try
- Add EventLogService with append/query/clear and subscriber notifications - Add MonacoDiffPanel for visualizing app patches - Add EventLogPanel with filtering and auto-scroll - Extend IPC channel definitions for event log and diff - Update conflict-broker, permission-broker, tool-router, model-gateway, workspace-service - Update App.tsx, Explorer.tsx, editor/index.ts, styles.css - Add unit tests: event-log-service, event-log-panel, app-patch-conflict - Add integration tests: sensitive-file-approval-flow, tool-router-event-log - Add E2E tests: diff-panel, event-log - Update shared exports and mock-preload-api
There was a problem hiding this comment.
Pull request overview
This PR introduces a Phase 7 tool-execution/approval pipeline (PermissionBroker + ToolRouter + ConflictBroker), adds an Event Log service and UI (including a Monaco-based diff viewer), and wires new IPC endpoints across the Electron main/preload/workbench layers with accompanying unit/integration/E2E tests.
Changes:
- Add EventLogService + IPC endpoints, and an EventLogPanel UI with MonacoDiffPanel for viewing diffs attached to log entries.
- Add PermissionBroker + ToolRouter + ConflictBroker plumbing (including patch application + conflict handling + approval prompts).
- Add broad test coverage across unit/integration/E2E for the new approval/log/diff/conflict flows.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/phase7-type-guards.test.ts | Adds unit tests for new Phase 7 shared type guards. |
| tests/unit/permission-broker.test.ts | Adds unit tests for PermissionBroker + sensitive path + risk helpers. |
| tests/unit/event-log-service.test.ts | Adds unit tests for EventLogService (append/query/clear + sanitization). |
| tests/unit/event-log-panel.test.tsx | Adds React unit tests for EventLogPanel rendering and controls. |
| tests/unit/conflict-broker.test.ts | Adds unit tests for conflict detection, patch hashing/application, and auto-merge paths. |
| tests/unit/app-patch-conflict.test.tsx | Adds UI tests ensuring PatchConflictDialog renders/responds to conflict events. |
| tests/shared/mock-preload-api.ts | Extends shared preload mock with Event Log API stubs. |
| tests/integration/tool-router-event-log.test.ts | Adds integration test ensuring ToolRouter logs patch events when EventLogService is provided. |
| tests/integration/sensitive-file-approval-flow.test.ts | Adds integration tests for sensitive-path approval + defense-in-depth tool blocking. |
| tests/e2e/event-log.spec.ts | Adds E2E coverage for Event Log panel rendering, filtering, clearing, and accessibility attributes. |
| tests/e2e/diff-panel.spec.ts | Adds E2E coverage for diff generation via IPC and Event Log integration. |
| packages/workbench/src/styles.css | Adds styling for approval dialogs, patch conflict dialog, Monaco diff panel, and Event Log panel. |
| packages/workbench/src/Explorer.tsx | Routes rename/delete through toolCall approval flow when available. |
| packages/workbench/src/EventLogPanel.tsx | New EventLogPanel component with filtering and Monaco diff display for entries with diffs. |
| packages/workbench/src/editor/MonacoDiffPanel.tsx | New Monaco diff viewer wrapper component. |
| packages/workbench/src/editor/index.ts | Exports MonacoDiffPanel from editor module index. |
| packages/workbench/src/App.tsx | Adds Event Log bottom panel + tool approval dialog + patch conflict dialog wiring. |
| packages/shared/src/ipc.ts | Extends IPC channels and adds Phase 7 types + event log types + type guards. |
| packages/shared/src/index.ts | Re-exports new IPC types and guard helpers from shared entrypoint. |
| packages/services/src/workspace-service.ts | Exposes binary extension helpers and factors file ops into standalone helpers; tracks workspace roots. |
| packages/services/src/tool-router.ts | New ToolRouter coordinating permission checks and tool execution; logs patch events. |
| packages/services/src/permission-broker.ts | New PermissionBroker with tool classifications, allow rules, sensitive path checks, and approval queue. |
| packages/services/src/model-gateway.ts | Wires tool execution through ToolRouter (instead of returning placeholders). |
| packages/services/src/index.ts | Exposes new Phase 7 services and helpers from services package. |
| packages/services/src/event-log-service.ts | New EventLogService with filtering and diff sanitization. |
| packages/services/src/conflict-broker.ts | New ConflictBroker and patch conflict detection/application + auto-merge routines. |
| apps/desktop/src/preload/index.ts | Extends preload API with tool router, approval, conflict, hashing, and event log IPC bridges. |
| apps/desktop/src/main/index.ts | Registers IPC handlers for Phase 7 tools/approvals/conflicts plus Event Log; adds sensitive-path guards. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+36
to
+45
| it('should trim entries when exceeding max', () => { | ||
| const smallService = new EventLogService(3); | ||
| smallService.append({ level: 'info', source: 'test', message: '1' }); | ||
| smallService.append({ level: 'info', source: 'test', message: '2' }); | ||
| smallService.append({ level: 'info', source: 'test', message: '3' }); | ||
| smallService.append({ level: 'info', source: 'test', message: '4' }); | ||
|
|
||
| const result = service.query(); | ||
| expect(result.status === 'ok').toBe(true); | ||
| }); |
Comment on lines
+280
to
+301
| async waitForApproval(callId: string): Promise<{ decision: ApprovalDecision; request: ToolCallRequest } | null> { | ||
| const pending = this.pendingApprovals.get(callId); | ||
| if (!pending) return null; | ||
|
|
||
| let timedOut = false; | ||
| const timeout = setTimeout(() => { | ||
| timedOut = true; | ||
| this.pendingApprovals.delete(callId); | ||
| }, this.approvalTimeoutMs); | ||
|
|
||
| const decision = await pending.promise; | ||
| clearTimeout(timeout); | ||
|
|
||
| // If the timeout fired before the promise resolved, the entry was already | ||
| // deleted. Return null to signal timeout. | ||
| if (timedOut) return null; | ||
|
|
||
| // If decision is undefined/null (shouldn't happen with proper resolve, but guard anyway) | ||
| if (!decision) return null; | ||
|
|
||
| return { decision, request: pending.request }; | ||
| } |
Comment on lines
+539
to
+547
| // Check both paths for sensitivity | ||
| if (sensitiveCheck?.isSensitive) { | ||
| return { | ||
| status: 'error', | ||
| callId: request.callId, | ||
| code: 'ACCESS_DENIED', | ||
| message: `Zmiana nazwy wrażliwego pliku zabroniona: ${oldPath}` | ||
| }; | ||
| } |
Comment on lines
+599
to
+609
| if (response.status === 'error') { | ||
| // If conflict detected, push conflict event to renderer for UI handling | ||
| const conflictData = (response as Record<string, unknown>).conflict; | ||
| if (response.code === 'WRITE_CONFLICT' && conflictData) { | ||
| if (!mainWindow.isDestroyed()) { | ||
| mainWindow.webContents.send(IPC_CHANNELS.conflictDetected, conflictData); | ||
| } | ||
| return { status: 'error', code: 'WRITE_CONFLICT', message: response.message }; | ||
| } | ||
| return { status: 'error', code: response.code, message: response.message }; | ||
| } |
Comment on lines
+381
to
+388
| <pre className="patch-conflict-diff" aria-label="Diff preview"> | ||
| {diffLines.map((line, i) => ( | ||
| <div key={i} className={`diff-line diff-line--${line.prefix === '---' ? 'removed' : line.prefix === '+++' ? 'added' : 'header'}`}> | ||
| <span className="diff-prefix">{line.prefix}</span> | ||
| <span className="diff-text">{line.text}</span> | ||
| </div> | ||
| ))} | ||
| </pre> |
| border-width: 0; | ||
| } | ||
|
|
||
| /* ?? Phase 7: Tool Approval Dialog ?????????????????????????????????????? */ |
Comment on lines
+41
to
+52
| // ?? Helper: open workspace so event log IPC is fully wired ???????????????= | ||
|
|
||
| async function openWorkspace() { | ||
| await stubDialog(app, 'showOpenDialog', { | ||
| filePaths: [tempDir], | ||
| canceled: false, | ||
| }); | ||
| await page.getByRole('button', { name: /open folder/i }).click(); | ||
| await expect(page.locator('output[aria-label="Workspace status"]')).toContainText('opened', { timeout: 15000 }); | ||
| } | ||
|
|
||
| // ?? Tests ????????????????????????????????????????????????????????????????? |
Comment on lines
+199
to
+202
| const uniqueFiles = new Set(edit.operations.map(op => op.filePath)); | ||
| if (uniqueFiles.size > 1) { | ||
| return routeMultiFileEdit(edit, uniqueFiles, mainWindow); | ||
| } |
Comment on lines
+267
to
+271
| // Context-anchor validation: verify that the lines immediately before | ||
| // and after each operation's range still match the expected content. | ||
| // This detects when external edits have touched the same region, preventing | ||
| // silent "last writer wins" overwrites of concurrent user changes. | ||
| for (const op of rangedOps) { |
…ay, permission-broker, tool-router - 216 nowych testów zwiększających pokrycie kodu >80% dla 6 plików - matchMedia polyfill w setup.ts (wymagany przez monaco-editor) - Poprawka sortowania źródeł w EventLogService (localeCompare)
- event-log-panel-coverage: as any dla undefined pól z exactOptionalPropertyTypes - explorer-deep-coverage: mockAgent przyjmuje any zamiast Partial<AgentDeckPreloadApi> - tool-router-coverage: mock conflict-broker module, poprawny ConflictKind i createdAt
- app-coverage-deep: usunięcie nieużywanego user, dodanie eslint-disable dla any - event-log-panel-coverage: @ts-expect-error zamiast as any dla undefined pól - explorer-deep-coverage: mockAgent z typem any, usunięcie nieużywanego importu act - model-gateway-coverage: usunięcie nieużywanych importów i callCount - permission-broker-coverage: usunięcie nieużywanych importów escalateRisk/isHighRisk - tool-router-coverage: usunięcie nieużywanych importów Conflict/PatchSet
…and app coverage; update conflict broker and vitest config
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary