docs: revise MCP standalone plan per architecture review
This commit is contained in:
@@ -5,7 +5,6 @@
|
||||
Ship a bundled CLI tool (`bds-mcp`) alongside the Electron app that:
|
||||
|
||||
- Runs as a self-contained MCP server over **stdio** (for Claude Desktop, VS Code, etc.)
|
||||
- Spins up **HTTP :4125** alongside stdio for App View callbacks (same shared state)
|
||||
- Works entirely against the **SQLite database** — no Electron process required
|
||||
- Signals the running app of changes via a **`db_notifications`** table
|
||||
- Is installed into agent configs by the app's existing agent-config UI (with add **and remove**)
|
||||
@@ -19,15 +18,15 @@ Ship a bundled CLI tool (`bds-mcp`) alongside the Electron app that:
|
||||
│ bds-mcp (bundled CLI binary) │ │ bDS Electron app │
|
||||
│ │ │ │
|
||||
│ StdioServerTransport ──────┐ │ │ HTTP MCPServer :4124 │
|
||||
│ StreamableHTTP :4125 ──────┤ │ │ ├── same engines │
|
||||
│ │ │ │ ├── same engines │
|
||||
│ │ │ │ └── NotificationWatcher │
|
||||
│ PostEngine ├───┼─────┼──► SQLite (WAL mode, shared) │
|
||||
│ MediaEngine │ │ │ posts / media / scripts … │
|
||||
│ ScriptEngine │ │ │ db_notifications ← new │
|
||||
│ TemplateEngine │ │ └────────────────────────────────────┘
|
||||
│ MCPServer (tools/res) │ │ ▲
|
||||
│ ProposalStore (in-memory) │ │ setInterval polls
|
||||
└─────────────────────────────┘ │ refresh caches + IPC to renderer
|
||||
│ ProposalStore (in-memory) │ │ fs.watch(-wal) + 100 ms debounce
|
||||
└─────────────────────────────┘ │ invalidate engines + IPC to renderer
|
||||
│
|
||||
agent (Claude Desktop, VS Code…) ─┘
|
||||
(stdio)
|
||||
@@ -38,8 +37,7 @@ agent (Claude Desktop, VS Code…) ─┘
|
||||
| Context | Port | Transport |
|
||||
|---|---|---|
|
||||
| bDS app running | :4124 | HTTP (existing, unchanged) |
|
||||
| CLI stdio mode — agent | — | stdio |
|
||||
| CLI stdio mode — App View callbacks | :4125 | HTTP |
|
||||
| `bds-mcp` CLI | — | stdio |
|
||||
|
||||
---
|
||||
|
||||
@@ -92,6 +90,31 @@ Behaviour:
|
||||
|
||||
---
|
||||
|
||||
### C0 — Decouple `DatabaseConnection` from Electron
|
||||
|
||||
`src/main/database/connection.ts` currently imports `{ app } from 'electron'` for three
|
||||
things: `app.getPath('userData')`, `app.isPackaged`, and `process.resourcesPath`. The CLI
|
||||
cannot load this file at all without Electron present.
|
||||
|
||||
Refactor `DatabaseConnection` to accept explicit constructor arguments:
|
||||
|
||||
```typescript
|
||||
interface DatabaseConnectionConfig {
|
||||
dbPath: string; // absolute path to bds.db
|
||||
migrationsFolder: string; // absolute path to the drizzle/ migrations dir
|
||||
createDirs?: string[]; // extra directories to mkdir at startup
|
||||
}
|
||||
```
|
||||
|
||||
The Electron `main.ts` constructs it as today (computing paths via `app.getPath` before
|
||||
passing them in). The CLI computes the same paths via `platformConfigPath()` and
|
||||
`__dirname`-relative resolution from the bundle. All Electron imports are removed from
|
||||
`connection.ts` itself.
|
||||
|
||||
This is a prerequisite for every CLI section that follows.
|
||||
|
||||
---
|
||||
|
||||
### C — `platformConfigPath()` — userData without Electron (`src/cli/platform.ts`)
|
||||
|
||||
Pure-Node helper to resolve the same path as Electron's `app.getPath('userData')`:
|
||||
@@ -123,84 +146,211 @@ Injected as an optional constructor argument on `PostEngine`, `ScriptEngine`,
|
||||
|
||||
---
|
||||
|
||||
### E — `NotificationWatcher` (new engine, app-side only)
|
||||
### E — Engine `invalidate()` API
|
||||
|
||||
No engine currently exposes a way to signal that its data is stale. Before
|
||||
`NotificationWatcher` can be implemented, each engine that holds any in-memory state needs
|
||||
an `invalidate(entityId?: string): void` method that clears the relevant cached entries
|
||||
(or the whole cache when `entityId` is omitted), forcing a fresh DB read on the next
|
||||
access. Engines that are already fully stateless (query DB on every call) get a no-op stub
|
||||
so `NotificationWatcher` can call a uniform API.
|
||||
|
||||
Engines to update: `PostEngine`, `MediaEngine`, `ScriptEngine`, `TemplateEngine`.
|
||||
|
||||
---
|
||||
|
||||
### F — `NotificationWatcher` (new engine, app-side only)
|
||||
|
||||
`src/main/engine/NotificationWatcher.ts`
|
||||
|
||||
- `setInterval` polling every 2 s
|
||||
- Reads `db_notifications WHERE seenAt IS NULL AND origin = 'cli'`
|
||||
- For each row: invalidates the relevant engine's cache, calls
|
||||
`mainWindow.webContents.send('entity:changed', { entity, entityId, action })`
|
||||
- Stamps `seenAt = Date.now()` on processed rows
|
||||
- Started in `main.ts` alongside `PreviewServer` and `MCPServer`
|
||||
|
||||
---
|
||||
|
||||
### F — `MCPServer.startCli()` (extend existing `MCPServer.ts`)
|
||||
Uses `fs.watch()` on the SQLite WAL file (`bds.db-wal`) for near-instant change
|
||||
detection instead of polling. When a write lands in WAL mode, the `-wal` file is modified
|
||||
before a checkpoint; watching it gives sub-100 ms latency.
|
||||
|
||||
```typescript
|
||||
async startCli(appViewsPort = 4125): Promise<void> {
|
||||
// 1. Stdio transport — primary agent connection
|
||||
const stdioTransport = new StdioServerTransport();
|
||||
const stdioMcp = this.createServer({ mode: 'cli', appViewsPort });
|
||||
await stdioMcp.connect(stdioTransport);
|
||||
export class NotificationWatcher {
|
||||
private dbWatcher: FSWatcher | null = null;
|
||||
private walWatcher: FSWatcher | null = null;
|
||||
private debounceTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
|
||||
// 2. HTTP :4125 — App View callbacks, same ProposalStore
|
||||
await this.startHttp(appViewsPort, { mode: 'cli' });
|
||||
constructor(
|
||||
private readonly dbPath: string, // absolute path to bds.db
|
||||
private readonly db: DrizzleDB,
|
||||
private readonly engines: WatchableEngines,
|
||||
private readonly mainWindow: BrowserWindow,
|
||||
private readonly debounceMs = 100,
|
||||
) {}
|
||||
|
||||
start(): void {
|
||||
// Watch the main db file (catches checkpoints)
|
||||
this.dbWatcher = watch(this.dbPath, { persistent: false },
|
||||
() => this.schedule());
|
||||
|
||||
// Watch the WAL file (catches every write in WAL mode)
|
||||
// The WAL file may not exist yet if no writes have occurred; we create
|
||||
// a short-lived retry watcher that tries once per second until the file
|
||||
// appears, at which point walWatcher is assigned and the retry cancels.
|
||||
this.attachWalWatcher();
|
||||
}
|
||||
|
||||
private attachWalWatcher(): void {
|
||||
const walPath = this.dbPath + '-wal';
|
||||
try {
|
||||
this.walWatcher = watch(walPath, { persistent: false },
|
||||
() => this.schedule());
|
||||
} catch {
|
||||
// -wal does not exist yet; retry in 1 s
|
||||
setTimeout(() => this.attachWalWatcher(), 1000);
|
||||
}
|
||||
}
|
||||
|
||||
private schedule(): void {
|
||||
if (this.debounceTimer) clearTimeout(this.debounceTimer);
|
||||
this.debounceTimer = setTimeout(() => this.process(), this.debounceMs);
|
||||
}
|
||||
|
||||
private async process(): Promise<void> {
|
||||
const rows = await this.db
|
||||
.select()
|
||||
.from(dbNotifications)
|
||||
.where(and(
|
||||
isNull(dbNotifications.seenAt),
|
||||
eq(dbNotifications.origin, 'cli'),
|
||||
));
|
||||
|
||||
for (const row of rows) {
|
||||
this.engines[row.entity]?.invalidate(row.entityId);
|
||||
this.mainWindow.webContents.send(
|
||||
'entity:changed',
|
||||
{ entity: row.entity, entityId: row.entityId, action: row.action },
|
||||
);
|
||||
await this.db
|
||||
.update(dbNotifications)
|
||||
.set({ seenAt: Date.now() })
|
||||
.where(eq(dbNotifications.id, row.id));
|
||||
}
|
||||
}
|
||||
|
||||
stop(): void {
|
||||
if (this.debounceTimer) clearTimeout(this.debounceTimer);
|
||||
this.dbWatcher?.close();
|
||||
this.walWatcher?.close();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Both `McpServer` instances are created via the same `createServer()` factory sharing the
|
||||
same `ProposalStore` instance. App View HTML receives the `:4125` base URL injected as a
|
||||
`<meta name="bds-mcp-base-url">` tag at render time so `callServerTool()` points to the
|
||||
right endpoint regardless of mode.
|
||||
`WatchableEngines` is a plain record keyed by entity name (`post`, `media`, `script`,
|
||||
`template`) mapping to objects that expose `invalidate()`. Started in `main.ts` alongside
|
||||
`PreviewServer` and `MCPServer`; stopped in the `app.on('before-quit')` handler.
|
||||
|
||||
---
|
||||
|
||||
### G — `src/cli/bds-mcp.ts` — CLI entrypoint (no Electron imports)
|
||||
### G — `MCPServer.startCli()` (extend existing `MCPServer.ts`)
|
||||
|
||||
The App View's `app.callServerTool()` routes through the MCP host (Claude Desktop, VS
|
||||
Code, etc.) over the **same** connection the server is on — it does not make a direct HTTP
|
||||
call to a secondary endpoint. The CLI therefore needs exactly one server and one transport:
|
||||
|
||||
```typescript
|
||||
async startCli(): Promise<void> {
|
||||
const server = this.createMcpServer();
|
||||
const transport = new StdioServerTransport();
|
||||
await server.connect(transport);
|
||||
|
||||
// Keep alive until the host closes stdin or the process receives a signal
|
||||
await new Promise<void>((resolve) => {
|
||||
process.stdin.on('close', resolve);
|
||||
process.once('SIGTERM', resolve);
|
||||
process.once('SIGINT', resolve);
|
||||
});
|
||||
|
||||
await server.close();
|
||||
}
|
||||
```
|
||||
|
||||
No secondary HTTP port. No `bds-mcp-base-url` meta tag. The `ProposalStore` is owned by
|
||||
the single `MCPServer` instance as before.
|
||||
|
||||
---
|
||||
|
||||
### H — `src/cli/bds-mcp.ts` — CLI entrypoint (no Electron imports)
|
||||
|
||||
```
|
||||
1. platformConfigPath() → userData dir
|
||||
2. getDatabase(path.join(userData, 'bds.db'))
|
||||
3. SELECT active project (isActive = 1); exit with message if none
|
||||
4. instantiate engines with CliNotifier DB writer
|
||||
5. new MCPServer({ postEngine, mediaEngine, … })
|
||||
6. mcpServer.startCli()
|
||||
2. migrationsFolder = path.join(__dirname, '../drizzle') // bundled alongside .cjs
|
||||
3. new DatabaseConnection({ dbPath, migrationsFolder })
|
||||
4. await db.initializeLocal()
|
||||
5. SELECT active project (isActive = 1); exit with message if none
|
||||
6. instantiate engines with CliNotifier DB writer
|
||||
7. new MCPServer({ postEngine, mediaEngine, … })
|
||||
8. await mcpServer.startCli()
|
||||
```
|
||||
|
||||
No `electron` imports. Fully standalone Node process.
|
||||
|
||||
**Graceful shutdown** — register handlers *before* `startCli()` so cleanup runs on both
|
||||
normal exit and forced signals:
|
||||
|
||||
```typescript
|
||||
async function shutdown(db: DatabaseConnection, store: ProposalStore): Promise<void> {
|
||||
store.destroy();
|
||||
await db.close(); // flushes any in-flight @libsql writes
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
process.once('SIGTERM', () => shutdown(db, mcpServer.proposalStore));
|
||||
process.once('SIGINT', () => shutdown(db, mcpServer.proposalStore));
|
||||
process.once('beforeExit', () => shutdown(db, mcpServer.proposalStore));
|
||||
```
|
||||
|
||||
(`startCli()` awaits `process.stdin` close / signal and then falls through to the same
|
||||
cleanup, so shutdown runs exactly once with the `once` guards.)
|
||||
|
||||
---
|
||||
|
||||
### H — esbuild/Vite build target for CLI bundle
|
||||
### I — esbuild/Vite build target for CLI bundle
|
||||
|
||||
New build target in `vite.config.ts` (or `package.json` scripts):
|
||||
|
||||
```
|
||||
src/cli/bds-mcp.ts → dist/cli/bds-mcp.cjs
|
||||
target: node, format: cjs, bundle: true
|
||||
externals: native modules only (better-sqlite3)
|
||||
```
|
||||
|
||||
The project uses `@libsql/client` with platform-specific native binaries (e.g.
|
||||
`@libsql/darwin-arm64`). These **cannot** be bundled by esbuild. Externalize all
|
||||
`@libsql/*` packages and ensure the platform binary directories are included alongside
|
||||
the `.cjs` file in `extraResources`:
|
||||
|
||||
```
|
||||
externals: ['@libsql/client', '@libsql/darwin-arm64', '@libsql/linux-x64-gnu', …]
|
||||
```
|
||||
|
||||
Also bundle the `drizzle/` migrations folder next to `bds-mcp.cjs` so the path resolution
|
||||
in section H (`path.join(__dirname, '../drizzle')`) works at runtime inside the app bundle.
|
||||
|
||||
Runs as part of `npm run build` before the Electron build.
|
||||
|
||||
---
|
||||
|
||||
### I — electron-builder `extraResources`
|
||||
### J — electron-builder `extraResources`
|
||||
|
||||
```json
|
||||
"build": {
|
||||
"extraResources": [
|
||||
{ "from": "dist/cli/bds-mcp.cjs", "to": "bds-mcp.cjs" }
|
||||
{ "from": "dist/cli/bds-mcp.cjs", "to": "bds-mcp.cjs" },
|
||||
{ "from": "drizzle", "to": "drizzle" }
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
Placed at `Contents/Resources/bds-mcp.cjs` in the macOS app bundle.
|
||||
`bds-mcp.cjs` is placed at `Contents/Resources/bds-mcp.cjs`; the `drizzle/` migrations
|
||||
folder is placed at `Contents/Resources/drizzle/` so the CLI migration resolver
|
||||
(`path.join(__dirname, '../drizzle')`) resolves correctly from `Contents/Resources/`.
|
||||
|
||||
---
|
||||
|
||||
### J — `MCPAgentConfigEngine` — add Claude Desktop + stdio support + remove
|
||||
### K — `MCPAgentConfigEngine` — add Claude Desktop + stdio support + remove
|
||||
|
||||
#### New agent type
|
||||
|
||||
@@ -259,7 +409,7 @@ buttons, so users never have to hunt down config files manually.
|
||||
|
||||
---
|
||||
|
||||
### K — `ScriptEngine` / `TemplateEngine` — draft lifecycle methods
|
||||
### L — `ScriptEngine` / `TemplateEngine` — draft lifecycle methods
|
||||
|
||||
New methods on each engine:
|
||||
|
||||
@@ -277,7 +427,7 @@ deleteDraftTemplate(id: string): Promise<boolean>
|
||||
|
||||
---
|
||||
|
||||
### L — `ProposalStore` — map proposals to DB row IDs
|
||||
### M — `ProposalStore` — map proposals to DB row IDs
|
||||
|
||||
For `draftPost` proposals the store already maps `proposalId → postId` (the DB row). Extend
|
||||
this consistently:
|
||||
@@ -290,35 +440,45 @@ this consistently:
|
||||
`accept_proposal` dispatches to `publishScript` / `publishTemplate` / `publishPost` etc.
|
||||
`discard_proposal` dispatches to `deleteDraftScript` / `deleteDraftTemplate` / `deletePost`.
|
||||
|
||||
**Known failure mode for in-memory metadata proposals:** if the CLI process is killed after
|
||||
a `propose_media_metadata` or `propose_post_metadata` call but before accept/discard, the
|
||||
`proposalId` no longer exists in any `ProposalStore`. The App View buttons will call
|
||||
`accept_proposal` and receive `{ success: false, message: "Proposal … not found" }` —
|
||||
this is already user-visible in the review UI. No silent corruption occurs; the post or
|
||||
media item is simply unchanged. This is accepted as the trade-off for keeping metadata
|
||||
proposals lightweight (no DB rows, no filesystem side effects).
|
||||
|
||||
---
|
||||
|
||||
## Unresolved Questions
|
||||
|
||||
None — all design decisions resolved:
|
||||
## Design Decisions
|
||||
|
||||
| Question | Decision |
|
||||
|---|---|
|
||||
| Active project in CLI | Use `isActive = 1` from DB; fail fast if none |
|
||||
| Proposal durability | Scripts/templates use DB draft rows; metadata proposals stay in-memory |
|
||||
| App View URL in CLI mode | Injected as `<meta name="bds-mcp-base-url">` at render time |
|
||||
| Proposal durability | Scripts/templates use DB draft rows; metadata proposals stay in-memory (failure mode documented in M) |
|
||||
| App View tool calls in CLI mode | `app.callServerTool()` routes through the MCP host, not a direct HTTP call — no secondary port needed |
|
||||
| Claude Desktop support | stdio via bundled `bds-mcp.cjs` + `ELECTRON_RUN_AS_NODE=1` |
|
||||
| Remove from config | New `removeFromConfig()` method + Remove buttons in UI |
|
||||
| DB change detection | `fs.watch()` on `-wal` file + 100 ms debounce; falls back to watching main db file on checkpoint |
|
||||
| DB coupling to Electron | `DatabaseConnection` accepts explicit paths; Electron-specific path resolution stays in `main.ts` |
|
||||
|
||||
---
|
||||
|
||||
## Implementation Order (TDD per AGENTS.md)
|
||||
|
||||
1. **Migrations** (A + B) — schema changes first, all other work builds on them
|
||||
2. **`platformConfigPath()`** (C) — pure function, easy to test
|
||||
3. **`CliNotifier` + DB writer** (D) — unit tests with mock DB
|
||||
4. **`ScriptEngine`/`TemplateEngine` draft lifecycle** (K) — tests first
|
||||
5. **`ProposalStore` DB mapping** (L) — extend existing tests
|
||||
6. **`NotificationWatcher`** (E) — test with mock DB rows
|
||||
7. **`MCPAgentConfigEngine` — `claude-desktop` + `removeFromConfig`** (J) — extend existing tests
|
||||
8. **`MCPServer.startCli()`** (F) — test with in-process transports
|
||||
9. **`src/cli/bds-mcp.ts`** (G) — integration test: spawn, send `initialize`, assert response
|
||||
10. **Build target + `extraResources`** (H + I) — verify `npm run build` produces bundle
|
||||
11. **UI: Remove buttons** — renderer component changes, follows J
|
||||
12. **`NotificationWatcher` wired in `main.ts`** (E, app side)
|
||||
2. **Decouple `DatabaseConnection` from Electron** (C0) — prerequisite for all CLI code; update existing DB tests to pass explicit paths
|
||||
3. **`platformConfigPath()`** (C) — pure function, easy to test
|
||||
4. **`CliNotifier` + DB writer** (D) — unit tests with mock DB
|
||||
5. **Engine `invalidate()` API** (E) — add and test on each engine before NotificationWatcher
|
||||
6. **`ScriptEngine`/`TemplateEngine` draft lifecycle** (L) — tests first
|
||||
7. **`ProposalStore` DB mapping** (M) — extend existing tests
|
||||
8. **`NotificationWatcher`** (F) — test with mock DB rows + mock `fs.watch` emitter
|
||||
9. **`MCPAgentConfigEngine` — `claude-desktop` + `removeFromConfig`** (K) — extend existing tests
|
||||
10. **`MCPServer.startCli()`** (G) — test with in-process `StdioServerTransport`
|
||||
11. **`src/cli/bds-mcp.ts`** (H) — integration test: spawn process, send `initialize`, assert response, assert clean shutdown on SIGTERM
|
||||
12. **Build target + `extraResources`** (I + J) — verify `npm run build` produces bundle and `drizzle/` is co-located
|
||||
13. **UI: Remove buttons** — renderer component changes, follows K
|
||||
14. **`NotificationWatcher` wired in `main.ts`** (F, app side) + stopped in `before-quit`
|
||||
|
||||
Each step: write failing test → implement → green → next.
|
||||
|
||||
Reference in New Issue
Block a user