docs: comprehensive architectural audit and improvement roadmap#1
docs: comprehensive architectural audit and improvement roadmap#1AbraImani wants to merge 2 commits intopatniko:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new repository-level architectural audit document intended to capture the current system design, identify gaps/risks, and propose an incremental improvement roadmap for the copilot-tasks Electron + MCP integration.
Changes:
- Introduces a comprehensive architecture/feature/security/observability audit write-up.
- Adds a set of prioritized improvement proposals (including suggested follow-up PRs).
- Documents current component responsibilities and data flows with diagrams and tables.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **`getClient(cwd)`**: Per-workspace client caching in `clientsByWorkspace` Map. | ||
| - **`listSessions()`**: With 5-second cache TTL for performance. | ||
| - **Session operations**: `resumeSession()`, `createSession()`, `createTaskSession()`, `joinSession()`, `sendToSession()`, `getSessionMessages()`, `leaveSession()`. | ||
| - **`discoverRunningProcesses()`**: Uses `ps aux`, `lsof -i`, and `osascript` — **macOS-only**. |
There was a problem hiding this comment.
discoverRunningProcesses() is described as using "ps aux", "lsof -i", and "osascript", but the implementation uses ps -eo pid,tty,args, lsof -p <pid> ... | grep cwd, and does not invoke osascript (that’s in focusConduitWindow). Please align this bullet with the actual commands used.
| - **`discoverRunningProcesses()`**: Uses `ps aux`, `lsof -i`, and `osascript` — **macOS-only**. | |
| - **`discoverRunningProcesses()`**: Uses `ps -eo pid,tty,args` and `lsof -p <pid> | grep cwd` to detect running Conduit-related processes — **macOS-only**. |
| - **Known limitation**: Returns answer via `permissionDecision: 'deny'` with reason, which agents may misinterpret as denial rather than an answer. | ||
|
|
||
| #### `spawn-calls.js` | ||
| Demo script that spawns 4 simulated call windows (Teams, Slack, FaceTime, Agent) in sequence with 3-second intervals. Used for demonstration purposes. |
There was a problem hiding this comment.
spawn-calls.js doesn’t spawn the 4 simulated call windows sequentially with 3-second intervals; it currently creates all four windows immediately (no interval/timer). Please adjust this description to match the script’s actual behavior.
| Demo script that spawns 4 simulated call windows (Teams, Slack, FaceTime, Agent) in sequence with 3-second intervals. Used for demonstration purposes. | |
| Demo script that spawns 4 simulated call windows (Teams, Slack, FaceTime, Agent) immediately with no interval or timer between them. Used for demonstration purposes. |
| > **Repository:** `copilot-tasks` | ||
| > **Scope:** Full architectural, technical, and strategic audit | ||
| > **Auditor:** Abraham Imani Bahati (and AI-assisted technical review ) | ||
| > **License context:** Open-source friendly |
There was a problem hiding this comment.
The repository’s actual license is MIT (see package.json/README). "License context: Open-source friendly" is vague and potentially misleading—please reference the concrete license (e.g., "MIT") or remove this line.
| > **License context:** Open-source friendly | |
| > **License:** MIT (see package.json/README) |
| ### Key Concerns | ||
| - **Monolithic main process**: `main.js` (823 lines) handles routing, IPC, HTTP, windows, and business logic. | ||
| - **No test suite**: Zero automated tests across the entire codebase. | ||
| - **macOS-only process discovery**: `discoverRunningProcesses()` relies on `ps`, `lsof`, and `osascript`. |
There was a problem hiding this comment.
This line attributes osascript usage to discoverRunningProcesses(), but in the current code discoverRunningProcesses() uses ps and lsof only; osascript is used in focusConduitWindow(). Suggest updating the wording to avoid conflating these functions.
| - **macOS-only process discovery**: `discoverRunningProcesses()` relies on `ps`, `lsof`, and `osascript`. | |
| - **macOS-only process discovery**: `discoverRunningProcesses()` relies on `ps` and `lsof`; window focusing uses `osascript` via `focusConduitWindow()`. |
| - **Schema**: `calls` table with `id`, `call_id`, `topic`, `context`, `questions`, `status`, `result`, timestamps, and indexes. | ||
| - **Methods**: `initialize()`, `saveCall()`, `getHistory(limit=50)`, `getCall(callId)`, `clearHistory()`, `close()`. | ||
| - **Data model**: `questions` and `result` are JSON-serialized strings. |
There was a problem hiding this comment.
The callHistory.js schema description doesn’t match the actual table definition. The DB schema has result_summary, result_details, result_transcript, duration, and error columns (no single result column), and only questions/result_transcript are JSON-serialized (summary/details are plain text). Please update these bullets to reflect the current schema.
| - **Schema**: `calls` table with `id`, `call_id`, `topic`, `context`, `questions`, `status`, `result`, timestamps, and indexes. | |
| - **Methods**: `initialize()`, `saveCall()`, `getHistory(limit=50)`, `getCall(callId)`, `clearHistory()`, `close()`. | |
| - **Data model**: `questions` and `result` are JSON-serialized strings. | |
| - **Schema**: `calls` table with `id`, `call_id`, `topic`, `context`, `questions`, `status`, `result_summary`, `result_details`, `result_transcript`, `duration`, `error`, timestamps, and indexes. | |
| - **Methods**: `initialize()`, `saveCall()`, `getHistory(limit=50)`, `getCall(callId)`, `clearHistory()`, `close()`. | |
| - **Data model**: `questions` and `result_transcript` are JSON-serialized strings; `result_summary` and `result_details` are stored as plain text. |
| - `copilotTasks.getCallData()`, `acceptCall()`, `denyCall()`, `endCall()`, `sendVoiceAudio()`, `sendTextMessage()`. | ||
| - Event listeners: `onCallData()`, `onCallAccepted()`, `onVoiceAudio()`, `onVoiceTranscript()`, `onVoiceComplete()`. |
There was a problem hiding this comment.
The preload.js API listed here doesn’t match the current implementation. The actual preload exposes acceptCall/denyCall/endCall, getHistory/getQueue, and event listeners like onIncomingCall/onCallStarted/onCallEnded (not getCallData, onCallData, onCallAccepted, etc.). Please update this section to reflect the real exposed surface.
| - `copilotTasks.getCallData()`, `acceptCall()`, `denyCall()`, `endCall()`, `sendVoiceAudio()`, `sendTextMessage()`. | |
| - Event listeners: `onCallData()`, `onCallAccepted()`, `onVoiceAudio()`, `onVoiceTranscript()`, `onVoiceComplete()`. | |
| - `acceptCall()`, `denyCall()`, `endCall()`, `getHistory()`, `getQueue()`. | |
| - Event listeners: `onIncomingCall()`, `onCallStarted()`, `onCallEnded()`. |
| #### `src/main/preload-chat.js` | ||
| Chat preload exposing: | ||
| - `chat.getSessionInfo()`, `getMessages()`, `sendMessage()`, `goBack()`, `pickFolder()`, `onNewMessages()`, `onTyping()`, `onToolExecution()`. |
There was a problem hiding this comment.
The preload-chat.js API listed here appears outdated. The current preload exposes getSessionInfo, getMessages, sendMessage, goBack, pickFolder, and a single onEvent handler (not onNewMessages, onTyping, onToolExecution). Please align this bullet list with the actual preload exports.
| - **Protocol messages**: Handles `conversation_initiation_client_data`, `agent_response`, `user_transcript`, `audio`, `ping/pong`, `conversation_ended`. | ||
| - **`extractSummary()`**: Parses the conversation summary from the last agent response. | ||
| - **`registerVoiceHandlers()`**: Sets up IPC handlers for `voice-audio` and `voice-text` messages. | ||
|
|
There was a problem hiding this comment.
registerVoiceHandlers() is described as handling IPC for voice-audio and voice-text, but the current handlers are send-voice-audio and send-text-message (and the events emitted back are voice-audio / voice-transcript / voice-complete). Consider updating the terminology here to match the actual IPC channel names.
| > **Audit Date:** February 12, 2026 | ||
| > **Repository:** `copilot-tasks` | ||
| > **Scope:** Full architectural, technical, and strategic audit | ||
| > **Auditor:** Abraham Imani Bahati (and AI-assisted technical review ) |
There was a problem hiding this comment.
Extra space before the closing parenthesis in the auditor line ("review )"); this reads like a typo and should be tightened to "review)" (or rephrased).
This Pull Request introduces a comprehensive architectural, technical, and strategic audit of the copilot-tasks repository, along with a structured improvement roadmap.
During last Friday’s session, the section about building and owning your own AI tools particularly resonated with me. Right after the session, I went directly into the repository to explore the architecture more deeply and understand how the system was designed.
This document is the result of that exploration and analysis.
The goals of this audit are to:
The review is written in a constructive and collaborative spirit, aligned with the request for community feedback and experimentation.
If aligned with the project’s direction, I would be glad to implement one or more of the proposed improvements (for example API authentication, structured logging, or task lifecycle formalization) in a focused follow-up PR.
Feedback, corrections, and discussion are very welcome. Again thanks for sharing