From e7c1d5138012514de8e2c3e4d90694bcee6c89f6 Mon Sep 17 00:00:00 2001 From: suntianc Date: Sun, 26 Jul 2026 10:07:14 -0700 Subject: [PATCH 1/8] =?UTF-8?q?refactor(deepagent):=20Conversation=20Worki?= =?UTF-8?q?ng=20State=20=E6=94=B6=E6=8B=A2=E4=B8=BA=20lifecycle/compaction?= =?UTF-8?q?/reconciliation=20=E4=B8=89=E6=A8=A1=E5=9D=97=20(#197)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - reconciliation 吸收 worker promise 包装并导出默认 runner 工厂 - compaction 吸收 compaction runner 与 storage 物理检查 - 删除 maintenance facade，业务库读取绑定移至 IPC 注册处 - 两个 worker 入口因 rollup 独立 input 约束保留，仅改类型导入来源 - runner/storage 行为测试随代码迁入对应模块测试文件，行为零变更 Co-Authored-By: Claude Fable 5 --- ...on-working-state-compaction-runner.test.ts | 62 --------- ...rsation-working-state-compaction-runner.ts | 83 ------------ ...rsation-working-state-compaction-worker.ts | 2 +- ...versation-working-state-compaction.test.ts | 128 +++++++++++++++++- .../conversation-working-state-compaction.ts | 128 +++++++++++++++++- .../conversation-working-state-maintenance.ts | 37 ----- ...ion-working-state-reconciliation-worker.ts | 2 +- ...ation-working-state-reconciliation.test.ts | 71 +++++++++- ...nversation-working-state-reconciliation.ts | 72 ++++++++++ ...conversation-working-state-storage.test.ts | 72 ---------- .../conversation-working-state-storage.ts | 45 ------ ...sation-working-state-worker-runner.test.ts | 70 ---------- ...onversation-working-state-worker-runner.ts | 68 ---------- .../conversation-working-state.test.ts | 2 +- .../deepagent/conversation-working-state.ts | 8 +- src/main/index.ts | 6 +- src/main/ipc-handlers.test.ts | 33 ++++- src/main/ipc-handlers.ts | 38 ++++-- 18 files changed, 461 insertions(+), 466 deletions(-) delete mode 100644 src/main/deepagent/conversation-working-state-compaction-runner.test.ts delete mode 100644 src/main/deepagent/conversation-working-state-compaction-runner.ts delete mode 100644 src/main/deepagent/conversation-working-state-maintenance.ts delete mode 100644 src/main/deepagent/conversation-working-state-storage.test.ts delete mode 100644 src/main/deepagent/conversation-working-state-storage.ts delete mode 100644 src/main/deepagent/conversation-working-state-worker-runner.test.ts delete mode 100644 src/main/deepagent/conversation-working-state-worker-runner.ts diff --git a/src/main/deepagent/conversation-working-state-compaction-runner.test.ts b/src/main/deepagent/conversation-working-state-compaction-runner.test.ts deleted file mode 100644 index a7a34689..00000000 --- a/src/main/deepagent/conversation-working-state-compaction-runner.test.ts +++ /dev/null @@ -1,62 +0,0 @@ -import { EventEmitter } from 'events'; -import { describe, expect, it, vi } from 'vitest'; -import { - ConversationWorkingStateCompactionRunner, - type ConversationWorkingStateCompactionWorker, -} from './conversation-working-state-compaction-runner'; - -class FakeWorker extends EventEmitter implements ConversationWorkingStateCompactionWorker { - unref(): void {} -} - -const request = { - checkpointDatabasePath: '/tmp/deepagents-checkpoints.db', - liveThreadIds: ['conversation-1'], -}; - -describe('ConversationWorkingStateCompactionRunner', () => { - it('runs compaction in a Worker and forwards real maintenance phases', async () => { - const worker = new FakeWorker(); - const onPhase = vi.fn(); - const runner = new ConversationWorkingStateCompactionRunner( - () => '/app/compaction-worker.js', - (_workerPath, workerRequest) => { - expect(workerRequest).toEqual(request); - queueMicrotask(() => { - worker.emit('message', { type: 'phase', phase: 'rebuilding' }); - worker.emit('message', { - type: 'result', - result: { physicalBytesBefore: 4096, physicalBytesAfter: 2048 }, - }); - }); - return worker; - } - ); - - await expect(runner.run(request, onPhase)).resolves.toEqual({ - physicalBytesBefore: 4096, - physicalBytesAfter: 2048, - }); - expect(onPhase).toHaveBeenCalledWith('rebuilding'); - }); - - it('preserves a stable Worker failure code', async () => { - const worker = new FakeWorker(); - const runner = new ConversationWorkingStateCompactionRunner( - () => '/app/compaction-worker.js', - () => { - queueMicrotask(() => worker.emit('message', { - type: 'error', - code: 'INSUFFICIENT_DISK_SPACE', - error: 'not enough room', - })); - return worker; - } - ); - - await expect(runner.run(request)).rejects.toMatchObject({ - code: 'INSUFFICIENT_DISK_SPACE', - message: 'not enough room', - }); - }); -}); diff --git a/src/main/deepagent/conversation-working-state-compaction-runner.ts b/src/main/deepagent/conversation-working-state-compaction-runner.ts deleted file mode 100644 index b2737b08..00000000 --- a/src/main/deepagent/conversation-working-state-compaction-runner.ts +++ /dev/null @@ -1,83 +0,0 @@ -import { Worker } from 'worker_threads'; -import type { - ConversationWorkingStateFailureReason, - ConversationWorkingStateMaintenancePhase, -} from '../../shared/conversation-working-state'; -import type { - ConversationWorkingStateCompactionRequest, - ConversationWorkingStateCompactionResult, -} from './conversation-working-state-compaction'; -import { ConversationWorkingStateCompactionError } from './conversation-working-state-compaction'; - -export type ConversationWorkingStateCompactionWorkerResponse = - | { type: 'phase'; phase: ConversationWorkingStateMaintenancePhase } - | { type: 'result'; result: ConversationWorkingStateCompactionResult } - | { type: 'error'; code: ConversationWorkingStateFailureReason; error: string }; - -export interface ConversationWorkingStateCompactionWorker { - unref(): void; - on(event: 'message', listener: (message: ConversationWorkingStateCompactionWorkerResponse) => void): this; - once(event: 'error', listener: (error: Error) => void): this; - once(event: 'exit', listener: (code: number) => void): this; -} - -type WorkerFactory = ( - workerPath: string, - request: ConversationWorkingStateCompactionRequest -) => ConversationWorkingStateCompactionWorker; - -const createNodeWorker: WorkerFactory = (workerPath, request) => - new Worker(workerPath, { workerData: request }); - -export interface ConversationWorkingStateCompactionRunnerContract { - run( - request: ConversationWorkingStateCompactionRequest, - onPhase?: (phase: ConversationWorkingStateMaintenancePhase) => void - ): Promise; -} - -export class ConversationWorkingStateCompactionRunner -implements ConversationWorkingStateCompactionRunnerContract { - constructor( - private readonly resolveWorkerPath: () => string, - private readonly createWorker: WorkerFactory = createNodeWorker - ) {} - - run( - request: ConversationWorkingStateCompactionRequest, - onPhase?: (phase: ConversationWorkingStateMaintenancePhase) => void - ): Promise { - return new Promise((resolve, reject) => { - const worker = this.createWorker(this.resolveWorkerPath(), request); - worker.unref(); - let settled = false; - const settle = (callback: () => void) => { - if (settled) return; - settled = true; - callback(); - }; - - worker.on('message', (message) => { - if (message.type === 'phase') { - if (!settled) onPhase?.(message.phase); - return; - } - settle(() => { - if (message.type === 'result') { - resolve(message.result); - } else { - reject(new ConversationWorkingStateCompactionError(message.code, message.error)); - } - }); - }); - worker.once('error', (error) => settle(() => reject(error))); - worker.once('exit', (code) => { - settle(() => reject(new Error( - code === 0 - ? 'Conversation Working State compaction Worker exited without a result.' - : `Conversation Working State compaction Worker exited with code ${code}.` - ))); - }); - }); - } -} diff --git a/src/main/deepagent/conversation-working-state-compaction-worker.ts b/src/main/deepagent/conversation-working-state-compaction-worker.ts index e9026cc1..d4cf6f10 100644 --- a/src/main/deepagent/conversation-working-state-compaction-worker.ts +++ b/src/main/deepagent/conversation-working-state-compaction-worker.ts @@ -3,9 +3,9 @@ import { compactConversationWorkingStateStorage, ConversationWorkingStateCompactionError, type ConversationWorkingStateCompactionRequest, + type ConversationWorkingStateCompactionWorkerResponse, } from './conversation-working-state-compaction'; import { CONVERSATION_WORKING_STATE_FAILURE_REASONS } from '../../shared/conversation-working-state'; -import type { ConversationWorkingStateCompactionWorkerResponse } from './conversation-working-state-compaction-runner'; if (!parentPort) { throw new Error('Conversation Working State compaction requires a Worker parent port.'); diff --git a/src/main/deepagent/conversation-working-state-compaction.test.ts b/src/main/deepagent/conversation-working-state-compaction.test.ts index 483a2052..ba25f5f0 100644 --- a/src/main/deepagent/conversation-working-state-compaction.test.ts +++ b/src/main/deepagent/conversation-working-state-compaction.test.ts @@ -5,11 +5,15 @@ import { spawn } from 'child_process'; import Database from 'better-sqlite3'; import { SqliteSaver } from '@langchain/langgraph-checkpoint-sqlite'; import type { Checkpoint, CheckpointMetadata } from '@langchain/langgraph-checkpoint'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { EventEmitter } from 'events'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { compactConversationWorkingStateStorage, + ConversationWorkingStateCompactionRunner, findConversationWorkingStateMaintenanceBlocker, + inspectConversationWorkingStateStorage, recoverInterruptedConversationWorkingStateCompaction, + type ConversationWorkingStateCompactionWorker, } from './conversation-working-state-compaction'; describe('Conversation Working State compaction engine', () => { @@ -534,3 +538,125 @@ describe('findConversationWorkingStateMaintenanceBlocker', () => { } }); }); + +describe('ConversationWorkingStateCompactionRunner', () => { + class FakeWorker extends EventEmitter implements ConversationWorkingStateCompactionWorker { + unref(): void {} + } + + const request = { + checkpointDatabasePath: '/tmp/deepagents-checkpoints.db', + liveThreadIds: ['conversation-1'], + }; + + it('runs compaction in a Worker and forwards real maintenance phases', async () => { + const worker = new FakeWorker(); + const onPhase = vi.fn(); + const runner = new ConversationWorkingStateCompactionRunner( + () => '/app/compaction-worker.js', + (_workerPath, workerRequest) => { + expect(workerRequest).toEqual(request); + queueMicrotask(() => { + worker.emit('message', { type: 'phase', phase: 'rebuilding' }); + worker.emit('message', { + type: 'result', + result: { physicalBytesBefore: 4096, physicalBytesAfter: 2048 }, + }); + }); + return worker; + } + ); + + await expect(runner.run(request, onPhase)).resolves.toEqual({ + physicalBytesBefore: 4096, + physicalBytesAfter: 2048, + }); + expect(onPhase).toHaveBeenCalledWith('rebuilding'); + }); + + it('preserves a stable Worker failure code', async () => { + const worker = new FakeWorker(); + const runner = new ConversationWorkingStateCompactionRunner( + () => '/app/compaction-worker.js', + () => { + queueMicrotask(() => worker.emit('message', { + type: 'error', + code: 'INSUFFICIENT_DISK_SPACE', + error: 'not enough room', + })); + return worker; + } + ); + + await expect(runner.run(request)).rejects.toMatchObject({ + code: 'INSUFFICIENT_DISK_SPACE', + message: 'not enough room', + }); + }); +}); + +describe('inspectConversationWorkingStateStorage', () => { + let tempDir: string; + let databasePath: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cdf-working-state-storage-')); + databasePath = path.join(tempDir, 'deepagents-checkpoints.db'); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it('reports zero usage for a missing database without creating it', () => { + expect(inspectConversationWorkingStateStorage(databasePath)).toEqual({ + physicalBytes: 0, + estimatedReclaimableBytes: 0, + }); + expect(fs.existsSync(databasePath)).toBe(false); + }); + + it('reports valid bounded usage for an empty database', () => { + new Database(databasePath).close(); + + const status = inspectConversationWorkingStateStorage(databasePath); + + expect(status.physicalBytes).toBe(fs.statSync(databasePath).size); + expect(status.estimatedReclaimableBytes).toBeGreaterThanOrEqual(0); + expect(status.estimatedReclaimableBytes).toBeLessThanOrEqual(status.physicalBytes); + }); + + it('accounts for live SQLite sidecars in physical usage', () => { + const db = new Database(databasePath); + db.pragma('journal_mode = WAL'); + db.exec('CREATE TABLE payloads (value BLOB)'); + db.prepare('INSERT INTO payloads VALUES (?)').run(Buffer.alloc(256 * 1024, 1)); + + const expectedPhysicalBytes = [databasePath, `${databasePath}-wal`, `${databasePath}-shm`] + .reduce((total, filePath) => total + (fs.existsSync(filePath) ? fs.statSync(filePath).size : 0), 0); + const status = inspectConversationWorkingStateStorage(databasePath); + + expect(status.physicalBytes).toBe(expectedPhysicalBytes); + expect(status.estimatedReclaimableBytes).toBeLessThanOrEqual(expectedPhysicalBytes); + db.close(); + }); + + it('estimates reclaimable space for a freelist-heavy database without reading payloads', () => { + const db = new Database(databasePath); + db.exec('CREATE TABLE payloads (id INTEGER PRIMARY KEY, value BLOB)'); + const insert = db.prepare('INSERT INTO payloads (value) VALUES (?)'); + const insertMany = db.transaction(() => { + for (let index = 0; index < 24; index += 1) { + insert.run(Buffer.alloc(128 * 1024, index)); + } + }); + insertMany(); + db.exec('DELETE FROM payloads WHERE id <= 20'); + db.close(); + + const status = inspectConversationWorkingStateStorage(databasePath); + + expect(status.estimatedReclaimableBytes).toBeGreaterThan(0); + expect(status.estimatedReclaimableBytes).toBeLessThanOrEqual(status.physicalBytes); + }); +}); diff --git a/src/main/deepagent/conversation-working-state-compaction.ts b/src/main/deepagent/conversation-working-state-compaction.ts index 7cc1a353..b0fb9695 100644 --- a/src/main/deepagent/conversation-working-state-compaction.ts +++ b/src/main/deepagent/conversation-working-state-compaction.ts @@ -12,14 +12,54 @@ import { CONVERSATION_WORKING_STATE_BLOCK_REASONS, CONVERSATION_WORKING_STATE_FAILURE_REASONS, } from '../../shared/conversation-working-state'; +import { Worker } from 'worker_threads'; import { conversationWorkingStateTableExists, reconcileOrphanConversationWorkingState, } from './conversation-working-state-reconciliation'; -import { - getConversationWorkingStatePhysicalBytes, - inspectConversationWorkingStateStorage, -} from './conversation-working-state-storage'; + +export interface ConversationWorkingStateStorageInspection { + physicalBytes: number; + estimatedReclaimableBytes: number; +} + +const SQLITE_STORAGE_SUFFIXES = ['', '-wal', '-shm'] as const; + +export function getConversationWorkingStatePhysicalBytes(databasePath: string): number { + return SQLITE_STORAGE_SUFFIXES.reduce((total, suffix) => { + const filePath = `${databasePath}${suffix}`; + try { + return total + fs.statSync(filePath).size; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return total; + throw error; + } + }, 0); +} + +export function inspectConversationWorkingStateStorage( + databasePath: string +): ConversationWorkingStateStorageInspection { + const physicalBytes = getConversationWorkingStatePhysicalBytes(databasePath); + if (!fs.existsSync(databasePath) || physicalBytes === 0) { + return { physicalBytes, estimatedReclaimableBytes: 0 }; + } + + const db = new Database(databasePath, { readonly: true, fileMustExist: true }); + try { + const pageSize = db.pragma('page_size', { simple: true }) as number; + const pageCount = db.pragma('page_count', { simple: true }) as number; + const freelistCount = db.pragma('freelist_count', { simple: true }) as number; + const estimatedCompactedBytes = Math.max(0, pageCount - freelistCount) * pageSize; + const estimatedReclaimableBytes = Math.max( + 0, + Math.min(physicalBytes, physicalBytes - estimatedCompactedBytes) + ); + return { physicalBytes, estimatedReclaimableBytes }; + } finally { + db.close(); + } +} export interface ConversationWorkingStateCompactionRequest { checkpointDatabasePath: string; @@ -422,3 +462,83 @@ export function compactConversationWorkingStateStorage( if (!rollbackCreated) removeFileFamilyBestEffort(sqliteFileFamily(rollbackPath)); } } + +export type ConversationWorkingStateCompactionWorkerResponse = + | { type: 'phase'; phase: ConversationWorkingStateMaintenancePhase } + | { type: 'result'; result: ConversationWorkingStateCompactionResult } + | { type: 'error'; code: ConversationWorkingStateFailureReason; error: string }; + +export interface ConversationWorkingStateCompactionWorker { + unref(): void; + on(event: 'message', listener: (message: ConversationWorkingStateCompactionWorkerResponse) => void): this; + once(event: 'error', listener: (error: Error) => void): this; + once(event: 'exit', listener: (code: number) => void): this; +} + +type CompactionWorkerFactory = ( + workerPath: string, + request: ConversationWorkingStateCompactionRequest +) => ConversationWorkingStateCompactionWorker; + +const createNodeCompactionWorker: CompactionWorkerFactory = (workerPath, request) => + new Worker(workerPath, { workerData: request }); + +export interface ConversationWorkingStateCompactionRunnerContract { + run( + request: ConversationWorkingStateCompactionRequest, + onPhase?: (phase: ConversationWorkingStateMaintenancePhase) => void + ): Promise; +} + +export class ConversationWorkingStateCompactionRunner +implements ConversationWorkingStateCompactionRunnerContract { + constructor( + private readonly resolveWorkerPath: () => string, + private readonly createWorker: CompactionWorkerFactory = createNodeCompactionWorker + ) {} + + run( + request: ConversationWorkingStateCompactionRequest, + onPhase?: (phase: ConversationWorkingStateMaintenancePhase) => void + ): Promise { + return new Promise((resolve, reject) => { + const worker = this.createWorker(this.resolveWorkerPath(), request); + worker.unref(); + let settled = false; + const settle = (callback: () => void) => { + if (settled) return; + settled = true; + callback(); + }; + + worker.on('message', (message) => { + if (message.type === 'phase') { + if (!settled) onPhase?.(message.phase); + return; + } + settle(() => { + if (message.type === 'result') { + resolve(message.result); + } else { + reject(new ConversationWorkingStateCompactionError(message.code, message.error)); + } + }); + }); + worker.once('error', (error) => settle(() => reject(error))); + worker.once('exit', (code) => { + settle(() => reject(new Error( + code === 0 + ? 'Conversation Working State compaction Worker exited without a result.' + : `Conversation Working State compaction Worker exited with code ${code}.` + ))); + }); + }); + } +} + +/** Default worker-backed runner: the compaction worker bundle sits beside the main bundle. */ +export function createConversationWorkingStateCompactionRunner(): ConversationWorkingStateCompactionRunnerContract { + return new ConversationWorkingStateCompactionRunner( + () => path.join(__dirname, 'conversation-working-state-compaction-worker.js') + ); +} diff --git a/src/main/deepagent/conversation-working-state-maintenance.ts b/src/main/deepagent/conversation-working-state-maintenance.ts deleted file mode 100644 index bf89a973..00000000 --- a/src/main/deepagent/conversation-working-state-maintenance.ts +++ /dev/null @@ -1,37 +0,0 @@ -import path from 'path'; -import db from '../database'; -import { conversationWorkingStateLifecycle } from './conversation-working-state'; -import { findConversationWorkingStateMaintenanceBlocker } from './conversation-working-state-compaction'; -import { ConversationWorkingStateCompactionRunner } from './conversation-working-state-compaction-runner'; - -const compactionRunner = new ConversationWorkingStateCompactionRunner( - () => path.join(__dirname, 'conversation-working-state-compaction-worker.js') -); - -function readMaintenanceBlocker() { - return findConversationWorkingStateMaintenanceBlocker(db); -} - -function readLiveConversationIds() { - return (db.prepare('SELECT id FROM sessions').all() as Array<{ id: string }>) - .map((session) => session.id); -} - -export function getConversationWorkingStateStorageStatus() { - const status = conversationWorkingStateLifecycle.getStorageStatus(); - if (status.phase === 'analyzing' || status.phase === 'optimizing') { - return status; - } - return { - ...status, - blockedReason: conversationWorkingStateLifecycle.getMaintenanceBlocker(readMaintenanceBlocker), - }; -} - -export function compactConversationWorkingState() { - return conversationWorkingStateLifecycle.compact( - readMaintenanceBlocker, - readLiveConversationIds, - compactionRunner - ); -} diff --git a/src/main/deepagent/conversation-working-state-reconciliation-worker.ts b/src/main/deepagent/conversation-working-state-reconciliation-worker.ts index d6d3bbc1..2d8f97cd 100644 --- a/src/main/deepagent/conversation-working-state-reconciliation-worker.ts +++ b/src/main/deepagent/conversation-working-state-reconciliation-worker.ts @@ -2,8 +2,8 @@ import { parentPort, workerData } from 'worker_threads'; import { reconcileOrphanConversationWorkingState, type ConversationWorkingStateReconciliationRequest, + type ConversationWorkingStateWorkerResponse, } from './conversation-working-state-reconciliation'; -import type { ConversationWorkingStateWorkerResponse } from './conversation-working-state-worker-runner'; if (!parentPort) { throw new Error('Conversation Working State reconciliation requires a Worker parent port.'); diff --git a/src/main/deepagent/conversation-working-state-reconciliation.test.ts b/src/main/deepagent/conversation-working-state-reconciliation.test.ts index db6b3c28..3124a0dd 100644 --- a/src/main/deepagent/conversation-working-state-reconciliation.test.ts +++ b/src/main/deepagent/conversation-working-state-reconciliation.test.ts @@ -1,9 +1,14 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; +import { EventEmitter } from 'events'; import Database from 'better-sqlite3'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { reconcileOrphanConversationWorkingState } from './conversation-working-state-reconciliation'; +import { + ConversationWorkingStateWorkerRunner, + reconcileOrphanConversationWorkingState, + type ConversationWorkingStateWorker, +} from './conversation-working-state-reconciliation'; describe('reconcileOrphanConversationWorkingState', () => { let tempDir: string; @@ -120,3 +125,67 @@ describe('reconcileOrphanConversationWorkingState', () => { reopened.close(); }); }); + +describe('ConversationWorkingStateWorkerRunner', () => { + class FakeWorker extends EventEmitter implements ConversationWorkingStateWorker { + unrefCalled = false; + + unref(): void { + this.unrefCalled = true; + } + } + + const request = { + checkpointDatabasePath: '/tmp/deepagents-checkpoints.db', + liveThreadIds: ['conversation-1'], + }; + + it('passes the reconciliation request to the Worker and returns its result', async () => { + const worker = new FakeWorker(); + let receivedPath = ''; + let receivedRequest: unknown; + const runner = new ConversationWorkingStateWorkerRunner( + () => '/app/reconciliation-worker.js', + (workerPath, workerRequest) => { + receivedPath = workerPath; + receivedRequest = workerRequest; + queueMicrotask(() => worker.emit('message', { + ok: true, + result: { deletedThreadCount: 2 }, + })); + return worker; + } + ); + + await expect(runner.run(request)).resolves.toEqual({ deletedThreadCount: 2 }); + expect(receivedPath).toBe('/app/reconciliation-worker.js'); + expect(receivedRequest).toEqual(request); + expect(worker.unrefCalled).toBe(true); + }); + + it('rejects a structured Worker failure', async () => { + const worker = new FakeWorker(); + const runner = new ConversationWorkingStateWorkerRunner( + () => '/app/reconciliation-worker.js', + () => { + queueMicrotask(() => worker.emit('message', { ok: false, error: 'database busy' })); + return worker; + } + ); + + await expect(runner.run(request)).rejects.toThrow('database busy'); + }); + + it('rejects when the Worker exits before reporting a result', async () => { + const worker = new FakeWorker(); + const runner = new ConversationWorkingStateWorkerRunner( + () => '/app/reconciliation-worker.js', + () => { + queueMicrotask(() => worker.emit('exit', 1)); + return worker; + } + ); + + await expect(runner.run(request)).rejects.toThrow('exited with code 1'); + }); +}); diff --git a/src/main/deepagent/conversation-working-state-reconciliation.ts b/src/main/deepagent/conversation-working-state-reconciliation.ts index 53ce4b03..e2955835 100644 --- a/src/main/deepagent/conversation-working-state-reconciliation.ts +++ b/src/main/deepagent/conversation-working-state-reconciliation.ts @@ -1,4 +1,6 @@ import fs from 'fs'; +import path from 'path'; +import { Worker } from 'worker_threads'; import Database from 'better-sqlite3'; export interface ConversationWorkingStateReconciliationRequest { @@ -63,3 +65,73 @@ export function reconcileOrphanConversationWorkingState( db.close(); } } + +export type ConversationWorkingStateWorkerResponse = + | { ok: true; result: ConversationWorkingStateReconciliationResult } + | { ok: false; error: string }; + +export interface ConversationWorkingStateWorker { + unref(): void; + once(event: 'message', listener: (message: ConversationWorkingStateWorkerResponse) => void): this; + once(event: 'error', listener: (error: Error) => void): this; + once(event: 'exit', listener: (code: number) => void): this; +} + +type WorkerFactory = ( + workerPath: string, + request: ConversationWorkingStateReconciliationRequest +) => ConversationWorkingStateWorker; + +const createNodeWorker: WorkerFactory = (workerPath, request) => + new Worker(workerPath, { workerData: request }); + +export interface ConversationWorkingStateReconciliationRunner { + run( + request: ConversationWorkingStateReconciliationRequest + ): Promise; +} + +export class ConversationWorkingStateWorkerRunner +implements ConversationWorkingStateReconciliationRunner { + constructor( + private readonly resolveWorkerPath: () => string, + private readonly createWorker: WorkerFactory = createNodeWorker + ) {} + + run( + request: ConversationWorkingStateReconciliationRequest + ): Promise { + return new Promise((resolve, reject) => { + const worker = this.createWorker(this.resolveWorkerPath(), request); + worker.unref(); + let settled = false; + const settle = (callback: () => void) => { + if (settled) return; + settled = true; + callback(); + }; + + worker.once('message', (message) => { + settle(() => { + if (message.ok) resolve(message.result); + else reject(new Error(message.error)); + }); + }); + worker.once('error', (error) => settle(() => reject(error))); + worker.once('exit', (code) => { + settle(() => reject(new Error( + code === 0 + ? 'Conversation Working State Worker exited without a result.' + : `Conversation Working State Worker exited with code ${code}.` + ))); + }); + }); + } +} + +/** Default worker-backed runner: the reconciliation worker bundle sits beside the main bundle. */ +export function createConversationWorkingStateReconciliationRunner(): ConversationWorkingStateReconciliationRunner { + return new ConversationWorkingStateWorkerRunner( + () => path.join(__dirname, 'conversation-working-state-reconciliation-worker.js') + ); +} diff --git a/src/main/deepagent/conversation-working-state-storage.test.ts b/src/main/deepagent/conversation-working-state-storage.test.ts deleted file mode 100644 index e0ed2b13..00000000 --- a/src/main/deepagent/conversation-working-state-storage.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -import fs from 'fs'; -import os from 'os'; -import path from 'path'; -import Database from 'better-sqlite3'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { inspectConversationWorkingStateStorage } from './conversation-working-state-storage'; - -describe('inspectConversationWorkingStateStorage', () => { - let tempDir: string; - let databasePath: string; - - beforeEach(() => { - tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cdf-working-state-storage-')); - databasePath = path.join(tempDir, 'deepagents-checkpoints.db'); - }); - - afterEach(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); - }); - - it('reports zero usage for a missing database without creating it', () => { - expect(inspectConversationWorkingStateStorage(databasePath)).toEqual({ - physicalBytes: 0, - estimatedReclaimableBytes: 0, - }); - expect(fs.existsSync(databasePath)).toBe(false); - }); - - it('reports valid bounded usage for an empty database', () => { - new Database(databasePath).close(); - - const status = inspectConversationWorkingStateStorage(databasePath); - - expect(status.physicalBytes).toBe(fs.statSync(databasePath).size); - expect(status.estimatedReclaimableBytes).toBeGreaterThanOrEqual(0); - expect(status.estimatedReclaimableBytes).toBeLessThanOrEqual(status.physicalBytes); - }); - - it('accounts for live SQLite sidecars in physical usage', () => { - const db = new Database(databasePath); - db.pragma('journal_mode = WAL'); - db.exec('CREATE TABLE payloads (value BLOB)'); - db.prepare('INSERT INTO payloads VALUES (?)').run(Buffer.alloc(256 * 1024, 1)); - - const expectedPhysicalBytes = [databasePath, `${databasePath}-wal`, `${databasePath}-shm`] - .reduce((total, filePath) => total + (fs.existsSync(filePath) ? fs.statSync(filePath).size : 0), 0); - const status = inspectConversationWorkingStateStorage(databasePath); - - expect(status.physicalBytes).toBe(expectedPhysicalBytes); - expect(status.estimatedReclaimableBytes).toBeLessThanOrEqual(expectedPhysicalBytes); - db.close(); - }); - - it('estimates reclaimable space for a freelist-heavy database without reading payloads', () => { - const db = new Database(databasePath); - db.exec('CREATE TABLE payloads (id INTEGER PRIMARY KEY, value BLOB)'); - const insert = db.prepare('INSERT INTO payloads (value) VALUES (?)'); - const insertMany = db.transaction(() => { - for (let index = 0; index < 24; index += 1) { - insert.run(Buffer.alloc(128 * 1024, index)); - } - }); - insertMany(); - db.exec('DELETE FROM payloads WHERE id <= 20'); - db.close(); - - const status = inspectConversationWorkingStateStorage(databasePath); - - expect(status.estimatedReclaimableBytes).toBeGreaterThan(0); - expect(status.estimatedReclaimableBytes).toBeLessThanOrEqual(status.physicalBytes); - }); -}); diff --git a/src/main/deepagent/conversation-working-state-storage.ts b/src/main/deepagent/conversation-working-state-storage.ts deleted file mode 100644 index 84710ba2..00000000 --- a/src/main/deepagent/conversation-working-state-storage.ts +++ /dev/null @@ -1,45 +0,0 @@ -import fs from 'fs'; -import Database from 'better-sqlite3'; - -export interface ConversationWorkingStateStorageInspection { - physicalBytes: number; - estimatedReclaimableBytes: number; -} - -const SQLITE_STORAGE_SUFFIXES = ['', '-wal', '-shm'] as const; - -export function getConversationWorkingStatePhysicalBytes(databasePath: string): number { - return SQLITE_STORAGE_SUFFIXES.reduce((total, suffix) => { - const filePath = `${databasePath}${suffix}`; - try { - return total + fs.statSync(filePath).size; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') return total; - throw error; - } - }, 0); -} - -export function inspectConversationWorkingStateStorage( - databasePath: string -): ConversationWorkingStateStorageInspection { - const physicalBytes = getConversationWorkingStatePhysicalBytes(databasePath); - if (!fs.existsSync(databasePath) || physicalBytes === 0) { - return { physicalBytes, estimatedReclaimableBytes: 0 }; - } - - const db = new Database(databasePath, { readonly: true, fileMustExist: true }); - try { - const pageSize = db.pragma('page_size', { simple: true }) as number; - const pageCount = db.pragma('page_count', { simple: true }) as number; - const freelistCount = db.pragma('freelist_count', { simple: true }) as number; - const estimatedCompactedBytes = Math.max(0, pageCount - freelistCount) * pageSize; - const estimatedReclaimableBytes = Math.max( - 0, - Math.min(physicalBytes, physicalBytes - estimatedCompactedBytes) - ); - return { physicalBytes, estimatedReclaimableBytes }; - } finally { - db.close(); - } -} diff --git a/src/main/deepagent/conversation-working-state-worker-runner.test.ts b/src/main/deepagent/conversation-working-state-worker-runner.test.ts deleted file mode 100644 index f7a664a6..00000000 --- a/src/main/deepagent/conversation-working-state-worker-runner.test.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { EventEmitter } from 'events'; -import { describe, expect, it } from 'vitest'; -import { - ConversationWorkingStateWorkerRunner, - type ConversationWorkingStateWorker, -} from './conversation-working-state-worker-runner'; - -class FakeWorker extends EventEmitter implements ConversationWorkingStateWorker { - unrefCalled = false; - - unref(): void { - this.unrefCalled = true; - } -} - -const request = { - checkpointDatabasePath: '/tmp/deepagents-checkpoints.db', - liveThreadIds: ['conversation-1'], -}; - -describe('ConversationWorkingStateWorkerRunner', () => { - it('passes the reconciliation request to the Worker and returns its result', async () => { - const worker = new FakeWorker(); - let receivedPath = ''; - let receivedRequest: unknown; - const runner = new ConversationWorkingStateWorkerRunner( - () => '/app/reconciliation-worker.js', - (workerPath, workerRequest) => { - receivedPath = workerPath; - receivedRequest = workerRequest; - queueMicrotask(() => worker.emit('message', { - ok: true, - result: { deletedThreadCount: 2 }, - })); - return worker; - } - ); - - await expect(runner.run(request)).resolves.toEqual({ deletedThreadCount: 2 }); - expect(receivedPath).toBe('/app/reconciliation-worker.js'); - expect(receivedRequest).toEqual(request); - expect(worker.unrefCalled).toBe(true); - }); - - it('rejects a structured Worker failure', async () => { - const worker = new FakeWorker(); - const runner = new ConversationWorkingStateWorkerRunner( - () => '/app/reconciliation-worker.js', - () => { - queueMicrotask(() => worker.emit('message', { ok: false, error: 'database busy' })); - return worker; - } - ); - - await expect(runner.run(request)).rejects.toThrow('database busy'); - }); - - it('rejects when the Worker exits before reporting a result', async () => { - const worker = new FakeWorker(); - const runner = new ConversationWorkingStateWorkerRunner( - () => '/app/reconciliation-worker.js', - () => { - queueMicrotask(() => worker.emit('exit', 1)); - return worker; - } - ); - - await expect(runner.run(request)).rejects.toThrow('exited with code 1'); - }); -}); diff --git a/src/main/deepagent/conversation-working-state-worker-runner.ts b/src/main/deepagent/conversation-working-state-worker-runner.ts deleted file mode 100644 index e9519a6a..00000000 --- a/src/main/deepagent/conversation-working-state-worker-runner.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { Worker } from 'worker_threads'; -import type { - ConversationWorkingStateReconciliationRequest, - ConversationWorkingStateReconciliationResult, -} from './conversation-working-state-reconciliation'; - -export type ConversationWorkingStateWorkerResponse = - | { ok: true; result: ConversationWorkingStateReconciliationResult } - | { ok: false; error: string }; - -export interface ConversationWorkingStateWorker { - unref(): void; - once(event: 'message', listener: (message: ConversationWorkingStateWorkerResponse) => void): this; - once(event: 'error', listener: (error: Error) => void): this; - once(event: 'exit', listener: (code: number) => void): this; -} - -type WorkerFactory = ( - workerPath: string, - request: ConversationWorkingStateReconciliationRequest -) => ConversationWorkingStateWorker; - -const createNodeWorker: WorkerFactory = (workerPath, request) => - new Worker(workerPath, { workerData: request }); - -export interface ConversationWorkingStateReconciliationRunner { - run( - request: ConversationWorkingStateReconciliationRequest - ): Promise; -} - -export class ConversationWorkingStateWorkerRunner -implements ConversationWorkingStateReconciliationRunner { - constructor( - private readonly resolveWorkerPath: () => string, - private readonly createWorker: WorkerFactory = createNodeWorker - ) {} - - run( - request: ConversationWorkingStateReconciliationRequest - ): Promise { - return new Promise((resolve, reject) => { - const worker = this.createWorker(this.resolveWorkerPath(), request); - worker.unref(); - let settled = false; - const settle = (callback: () => void) => { - if (settled) return; - settled = true; - callback(); - }; - - worker.once('message', (message) => { - settle(() => { - if (message.ok) resolve(message.result); - else reject(new Error(message.error)); - }); - }); - worker.once('error', (error) => settle(() => reject(error))); - worker.once('exit', (code) => { - settle(() => reject(new Error( - code === 0 - ? 'Conversation Working State Worker exited without a result.' - : `Conversation Working State Worker exited with code ${code}.` - ))); - }); - }); - } -} diff --git a/src/main/deepagent/conversation-working-state.test.ts b/src/main/deepagent/conversation-working-state.test.ts index 86e6c6df..35ca736b 100644 --- a/src/main/deepagent/conversation-working-state.test.ts +++ b/src/main/deepagent/conversation-working-state.test.ts @@ -11,10 +11,10 @@ import { createConversationWorkingStateLifecycle, type ConversationWorkingStateLifecycle, } from './conversation-working-state'; -import type { ConversationWorkingStateCompactionRunnerContract } from './conversation-working-state-compaction-runner'; import { compactConversationWorkingStateStorage, type ConversationWorkingStateCompactionDependencies, + type ConversationWorkingStateCompactionRunnerContract, } from './conversation-working-state-compaction'; function checkpoint(id: string, value: string): Checkpoint { diff --git a/src/main/deepagent/conversation-working-state.ts b/src/main/deepagent/conversation-working-state.ts index e8e3ceba..4d323f84 100644 --- a/src/main/deepagent/conversation-working-state.ts +++ b/src/main/deepagent/conversation-working-state.ts @@ -11,13 +11,13 @@ import { CONVERSATION_WORKING_STATE_BLOCK_REASONS, CONVERSATION_WORKING_STATE_FAILURE_REASONS, } from '../../shared/conversation-working-state'; -import type { ConversationWorkingStateReconciliationRunner } from './conversation-working-state-worker-runner'; -import type { ConversationWorkingStateCompactionRunnerContract } from './conversation-working-state-compaction-runner'; -import { recoverInterruptedConversationWorkingStateCompaction } from './conversation-working-state-compaction'; +import type { ConversationWorkingStateReconciliationRunner } from './conversation-working-state-reconciliation'; import { getConversationWorkingStatePhysicalBytes, inspectConversationWorkingStateStorage, -} from './conversation-working-state-storage'; + recoverInterruptedConversationWorkingStateCompaction, + type ConversationWorkingStateCompactionRunnerContract, +} from './conversation-working-state-compaction'; export const CONVERSATION_WORKING_STATE_MAINTENANCE_LOCKED = 'CONVERSATION_WORKING_STATE_MAINTENANCE_LOCKED' as const; diff --git a/src/main/index.ts b/src/main/index.ts index db2516f4..f82ab6af 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -18,7 +18,7 @@ import { conversationRunStreams } from './conversation-run-stream-runtime'; import { createCapabilityJobContinuationRunner } from './capabilities/capability-job-continuation-runner'; import { hardenWindowNavigation } from './window-navigation-guard'; import { conversationWorkingStateLifecycle } from './deepagent/conversation-working-state'; -import { ConversationWorkingStateWorkerRunner } from './deepagent/conversation-working-state-worker-runner'; +import { createConversationWorkingStateReconciliationRunner } from './deepagent/conversation-working-state-reconciliation'; // Register cdf-file scheme as privileged to bypass CSP and security sandboxing for local image media. // standard:true additionally enables Chromium's media seeking/range machinery (see cdf-file-protocol.ts). @@ -39,9 +39,7 @@ import path from 'path'; let mainWindow: BrowserWindow | null = null; let isQuitting = false; -const workingStateReconciliationRunner = new ConversationWorkingStateWorkerRunner( - () => path.join(__dirname, 'conversation-working-state-reconciliation-worker.js') -); +const workingStateReconciliationRunner = createConversationWorkingStateReconciliationRunner(); function reconcileConversationWorkingStateAtStartup() { return conversationWorkingStateLifecycle.reconcileOrphansAtStartup( diff --git a/src/main/ipc-handlers.test.ts b/src/main/ipc-handlers.test.ts index b7f3bca9..69ee9936 100644 --- a/src/main/ipc-handlers.test.ts +++ b/src/main/ipc-handlers.test.ts @@ -29,6 +29,7 @@ const { deleteProjectMock, compactWorkingStateMock, getWorkingStateStorageStatusMock, + getMaintenanceBlockerMock, captureConversationSystemContextSnapshotMock, createAgentCatalogMock, agentCatalogMock, @@ -82,6 +83,7 @@ const { deleteProjectMock: vi.fn(), compactWorkingStateMock: vi.fn(), getWorkingStateStorageStatusMock: vi.fn(), + getMaintenanceBlockerMock: vi.fn(), captureConversationSystemContextSnapshotMock: vi.fn(() => ({ promptSnapshot: 'Captured Master prompt', skillSnapshot: listResolvedSkillViewsMock(), @@ -154,9 +156,27 @@ vi.mock('./conversation-run-stream-runtime', () => ({ }, })); -vi.mock('./deepagent/conversation-working-state-maintenance', () => ({ - compactConversationWorkingState: compactWorkingStateMock, - getConversationWorkingStateStorageStatus: getWorkingStateStorageStatusMock, +vi.mock('./deepagent/conversation-working-state', () => ({ + DEEPAGENT_CHECKPOINT_NAMESPACE: '', + conversationWorkingStateLifecycle: { + getStorageStatus: getWorkingStateStorageStatusMock, + getMaintenanceBlocker: getMaintenanceBlockerMock, + compact: compactWorkingStateMock, + beginRuntimeUse: vi.fn(() => () => {}), + beginCapabilityJobUse: vi.fn(() => () => {}), + acquireSaver: vi.fn(), + enterMaintenance: vi.fn(), + leaveMaintenance: vi.fn(), + deleteThread: vi.fn(), + assertConversationDeletionAllowed: vi.fn(), + reconcileOrphansAtStartup: vi.fn(), + close: vi.fn(), + }, +})); + +vi.mock('./deepagent/conversation-working-state-compaction', () => ({ + findConversationWorkingStateMaintenanceBlocker: vi.fn(() => null), + createConversationWorkingStateCompactionRunner: vi.fn(() => ({ run: vi.fn() })), })); vi.mock('./security', () => ({ @@ -258,6 +278,7 @@ describe('IPC handlers', () => { deleteConversationMock.mockReset(); deleteProjectMock.mockReset(); compactWorkingStateMock.mockReset(); + getMaintenanceBlockerMock.mockReset().mockReturnValue(null); getWorkingStateStorageStatusMock.mockReset(); getWorkingStateStorageStatusMock.mockReturnValue({ phase: 'normal', @@ -391,7 +412,11 @@ describe('IPC handlers', () => { const result = await handler({}, { skipIdleChecks: true, databasePath: '/private/cdf.db' }); - expect(compactWorkingStateMock).toHaveBeenCalledWith(); + expect(compactWorkingStateMock).toHaveBeenCalledWith( + expect.any(Function), + expect.any(Function), + expect.anything(), + ); expect(getWorkingStateStorageStatusMock).toHaveBeenCalledOnce(); expect(result).toEqual(status); expect(JSON.stringify(result)).not.toMatch(/path|sql|table|checkpoint|thread|database/i); diff --git a/src/main/ipc-handlers.ts b/src/main/ipc-handlers.ts index 22b9e369..d0410a38 100644 --- a/src/main/ipc-handlers.ts +++ b/src/main/ipc-handlers.ts @@ -86,9 +86,9 @@ import { deleteConversation, deleteProject } from './conversation-deletion'; import { conversationWorkingStateLifecycle } from './deepagent/conversation-working-state'; import { registerFlowDiagramExportResponseHandler } from './flow-diagram/flow-diagram-export-adapter'; import { - compactConversationWorkingState, - getConversationWorkingStateStorageStatus, -} from './deepagent/conversation-working-state-maintenance'; + createConversationWorkingStateCompactionRunner, + findConversationWorkingStateMaintenanceBlocker, +} from './deepagent/conversation-working-state-compaction'; import { DelegatedAgentRunRepository } from './deepagent/delegated-agent-run-repository'; import { createAgentCatalog, type CatalogAgent } from './agent-catalog'; @@ -281,12 +281,34 @@ export function registerIpcHandlers() { typedHandle('conversation:get-active-run', (_event, sessionId) => conversationRunStreams.getActive(sessionId) ); - typedHandle('working-state:get-storage-status', () => - getConversationWorkingStateStorageStatus() - ); + // Conversation Working State maintenance binding: the lifecycle stays free of + // the business database, so the blocker/live-id readers are bound here. + const workingStateCompactionRunner = createConversationWorkingStateCompactionRunner(); + const readWorkingStateMaintenanceBlocker = () => + findConversationWorkingStateMaintenanceBlocker(db); + const readLiveConversationIds = () => + (db.prepare('SELECT id FROM sessions').all() as Array<{ id: string }>) + .map((session) => session.id); + const getWorkingStateStorageStatus = () => { + const status = conversationWorkingStateLifecycle.getStorageStatus(); + if (status.phase === 'analyzing' || status.phase === 'optimizing') { + return status; + } + return { + ...status, + blockedReason: conversationWorkingStateLifecycle.getMaintenanceBlocker( + readWorkingStateMaintenanceBlocker + ), + }; + }; + typedHandle('working-state:get-storage-status', () => getWorkingStateStorageStatus()); typedHandle('working-state:optimize-storage', async () => { - await compactConversationWorkingState(); - return getConversationWorkingStateStorageStatus(); + await conversationWorkingStateLifecycle.compact( + readWorkingStateMaintenanceBlocker, + readLiveConversationIds, + workingStateCompactionRunner + ); + return getWorkingStateStorageStatus(); }); const ensureProjectForSession = (projectId: string) => { const existing = db.prepare('SELECT id FROM projects WHERE id = ?').get(projectId); From 3d3c9319b40a483c0e29f1a4220c0e3cb8d6c0e0 Mon Sep 17 00:00:00 2001 From: suntianc Date: Sun, 26 Jul 2026 10:16:13 -0700 Subject: [PATCH 2/8] =?UTF-8?q?refactor(deepagent):=20=E5=BB=BA=E7=AB=8B?= =?UTF-8?q?=20Skill=20Catalog=20=E6=B7=B1=E6=A8=A1=E5=9D=97=E7=BB=9F?= =?UTF-8?q?=E4=B8=80=20Skill=20=E7=9B=AE=E5=BD=95=E9=97=AE=E7=AD=94=20(#19?= =?UTF-8?q?8)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 新增 skill-catalog 作为唯一公开入口：规范目录解析（默认铺 Built-in 与 用户 Global 目录）、Scene 曝光谓词组合、Conversation Skill Snapshot 捕获、运行时视图与 CRUD/视图再导出 - 快照捕获组合从快照持久化模块移入 catalog，持久化职责不变 - source label 映射与 isGlobalSkillSourceKind 收敛为 skill-sources 单份， 删除 skill-manager/cdf-skills-runtime/context-aggregator/命令收集器四份拷贝 （收敛副产品：enterprise 来源统一为 'Managed Skill'，该来源无生产接线） - 迁移 8 个消费方到 catalog 接口：运行时装配、上下文聚合、快照捕获、 IPC 处理器、Agent 工具、Scene 曝光策略、命令注册与 Skill 收集器 - 命令收集器测试改在 catalog seam 上 mock；新增 catalog 谓词组合专属测试 Co-Authored-By: Claude Fable 5 --- src/main/commands/collectors/skill.test.ts | 48 +++--- src/main/commands/collectors/skill.ts | 31 +--- src/main/commands/command-registry.ts | 2 +- .../conversation-system-context-snapshot.ts | 56 +------ src/main/deepagent/agent-tools.ts | 2 +- src/main/deepagent/context-aggregator.ts | 34 +--- src/main/deepagent/runtime-assembly.ts | 16 +- src/main/deepagent/skill-catalog.test.ts | 70 ++++++++ src/main/deepagent/skill-catalog.ts | 156 ++++++++++++++++++ src/main/deepagent/skill-manager.ts | 22 +-- .../skills-runtime/cdf-skills-runtime.ts | 30 +--- .../deepagent/skills-runtime/skill-sources.ts | 33 +++- src/main/global-skill-scene-exposure.ts | 2 +- src/main/ipc-handlers.ts | 2 +- 14 files changed, 316 insertions(+), 188 deletions(-) create mode 100644 src/main/deepagent/skill-catalog.test.ts create mode 100644 src/main/deepagent/skill-catalog.ts diff --git a/src/main/commands/collectors/skill.test.ts b/src/main/commands/collectors/skill.test.ts index 040189f9..d4d0f998 100644 --- a/src/main/commands/collectors/skill.test.ts +++ b/src/main/commands/collectors/skill.test.ts @@ -1,37 +1,35 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { getBuiltInSkillDirsMock, getScopePathMock, resolveSkillSourcePlanMock, resolveSkillCatalogMock } = vi.hoisted(() => ({ - getBuiltInSkillDirsMock: vi.fn(() => ['/tmp/built-in/knowledge-base']), - getScopePathMock: vi.fn(() => '/tmp/global-skills'), - resolveSkillSourcePlanMock: vi.fn(() => ({ config: { version: 1, additionalSkillDirectories: [] }, sources: [], warnings: [] })), - resolveSkillCatalogMock: vi.fn((): any => ({ skills: [], warnings: [] })), +const { resolveProjectSkillCatalogMock } = vi.hoisted(() => ({ + resolveProjectSkillCatalogMock: vi.fn((): any => ({ skills: [], warnings: [] })), })); -vi.mock('../../deepagent/skill-manager', () => ({ - getBuiltInSkillDirs: getBuiltInSkillDirsMock, - getScopePath: getScopePathMock, -})); -vi.mock('../../deepagent/skills-runtime/skill-sources', () => ({ - resolveSkillSourcePlan: resolveSkillSourcePlanMock, - resolveSkillCatalog: resolveSkillCatalogMock, -})); +vi.mock('../../deepagent/skill-catalog', async () => { + const sources = await vi.importActual< + typeof import('../../deepagent/skills-runtime/skill-sources') + >('../../deepagent/skills-runtime/skill-sources'); + return { + resolveProjectSkillCatalog: resolveProjectSkillCatalogMock, + getSkillSourceLabel: sources.getSkillSourceLabel, + }; +}); import { collectSkillCommands } from './skill'; describe('collectSkillCommands', () => { beforeEach(() => { vi.clearAllMocks(); - resolveSkillCatalogMock.mockReturnValue({ skills: [], warnings: [] }); + resolveProjectSkillCatalogMock.mockReturnValue({ skills: [], warnings: [] }); }); - it('resolves the live catalog from Built-in and Global sources when no snapshot exists', async () => { - await expect(collectSkillCommands('/tmp/project')).resolves.toEqual([]); + it('resolves the live catalog through the Skill Catalog when no snapshot exists', async () => { + await expect(collectSkillCommands('/tmp/project', { includeNestedProjectSkills: true })) + .resolves.toEqual([]); - expect(resolveSkillSourcePlanMock).toHaveBeenCalledWith('/tmp/project', { - builtInSkillDirs: ['/tmp/built-in/knowledge-base'], - userSkillsDir: '/tmp/global-skills', - includeNestedProjectSkills: undefined, - }); + expect(resolveProjectSkillCatalogMock).toHaveBeenCalledWith( + '/tmp/project', + expect.objectContaining({ includeNestedProjectSkills: true }), + ); }); it('uses the frozen Conversation Skill Snapshot without resolving a live catalog', async () => { @@ -46,11 +44,11 @@ describe('collectSkillCommands', () => { }] }); expect(commands).toEqual([expect.objectContaining({ name: 'review', target: 'project:review' })]); - expect(resolveSkillSourcePlanMock).not.toHaveBeenCalled(); + expect(resolveProjectSkillCatalogMock).not.toHaveBeenCalled(); }); it('maps a Project Skill to an attributable command', async () => { - resolveSkillCatalogMock.mockReturnValue({ skills: [{ + resolveProjectSkillCatalogMock.mockReturnValue({ skills: [{ name: 'simplify', description: 'Simplify code', sourceKind: 'project', sourcePath: '/tmp/project/.cdf/skills', skillPath: '/tmp/project/.cdf/skills/simplify/SKILL.md', modelDiscovery: 'full', userInvocable: true, argumentHint: '', @@ -66,7 +64,7 @@ describe('collectSkillCommands', () => { }); it('maps a Global Skill to a global attributable command', async () => { - resolveSkillCatalogMock.mockReturnValue({ skills: [{ + resolveProjectSkillCatalogMock.mockReturnValue({ skills: [{ name: 'explore', description: 'Explore the repository', sourceKind: 'user', sourcePath: '/tmp/global-skills', skillPath: '/tmp/global-skills/explore/SKILL.md', modelDiscovery: 'full', userInvocable: true, @@ -82,7 +80,7 @@ describe('collectSkillCommands', () => { }); it('omits Skills whose author disables explicit invocation', async () => { - resolveSkillCatalogMock.mockReturnValue({ skills: [{ + resolveProjectSkillCatalogMock.mockReturnValue({ skills: [{ name: 'internal', description: 'Internal workflow', sourceKind: 'project', sourcePath: '/tmp/project/.cdf/skills', skillPath: '/tmp/project/.cdf/skills/internal/SKILL.md', modelDiscovery: 'full', userInvocable: false, diff --git a/src/main/commands/collectors/skill.ts b/src/main/commands/collectors/skill.ts index 9b30e2a7..7acb83e1 100644 --- a/src/main/commands/collectors/skill.ts +++ b/src/main/commands/collectors/skill.ts @@ -1,10 +1,9 @@ -import { getBuiltInSkillDirs, getScopePath } from '../../deepagent/skill-manager'; import { - resolveSkillCatalog, - resolveSkillSourcePlan, + getSkillSourceLabel, + resolveProjectSkillCatalog, type SkillCatalogOptions, type ResolvedSkillCatalogEntry, -} from '../../deepagent/skills-runtime/skill-sources'; +} from '../../deepagent/skill-catalog'; import type { CommandSource, SkillCommandSourceKind, SlashCommand } from '../../../shared/types'; import type { ConversationSkillSnapshotEntry } from '../../../shared/skills'; @@ -25,11 +24,7 @@ export async function collectSkillCommands( ): Promise { const skills = options.catalog ? options.catalog - : resolveSkillCatalog(resolveSkillSourcePlan(projectPath, { - builtInSkillDirs: getBuiltInSkillDirs(), - userSkillsDir: getScopePath(projectPath, 'global'), - includeNestedProjectSkills: options.includeNestedProjectSkills, - }), options).skills; + : resolveProjectSkillCatalog(projectPath, options).skills; return skills .filter((skill) => skill.userInvocable) @@ -52,7 +47,7 @@ function skillToCommand(skill: ResolvedSkillCatalogEntry | ConversationSkillSnap description: skill.description, source, target: `${getTargetScope(skill.sourceKind)}:${qualifiedName}`, - sourceLabel: getSourceLabel(skill), + sourceLabel: getSkillSourceLabel(skill), badge: `[${source}]`, frontmatter: { allowedTools: skill.allowedTools ?? [], @@ -92,19 +87,3 @@ function getTargetScope(sourceKind: ResolvedSkillCatalogEntry['sourceKind']): st } } -function getSourceLabel(skill: Pick): string { - switch (skill.sourceKind) { - case 'built-in': - return 'Built-in Skill'; - case 'project': - return 'Project Skill'; - case 'project-nested': - return skill.qualifier ? `Nested Project Skill: ${skill.qualifier}` : 'Nested Project Skill'; - case 'project-additional': - return skill.qualifier ? `Project Skill: ${skill.qualifier}` : 'Project Skill'; - case 'user': - return 'Global Skill'; - case 'enterprise': - return 'Enterprise Skill'; - } -} diff --git a/src/main/commands/command-registry.ts b/src/main/commands/command-registry.ts index 93a23e22..83aa91b6 100644 --- a/src/main/commands/command-registry.ts +++ b/src/main/commands/command-registry.ts @@ -3,7 +3,7 @@ import { collectMcpCommands } from './collectors/mcp'; import { collectProjectCommands } from './collectors/project'; import { collectSkillCommands, type SkillCommandCollectorOptions } from './collectors/skill'; import { collectSystemCommands } from './collectors/system'; -import type { SkillCatalogOptions } from '../deepagent/skills-runtime/skill-sources'; +import type { SkillCatalogOptions } from '../deepagent/skill-catalog'; import type { ConversationSkillSnapshotEntry } from '../../shared/skills'; import { detectConflicts } from './conflict-detector'; diff --git a/src/main/conversation-system-context-snapshot.ts b/src/main/conversation-system-context-snapshot.ts index e01e5f9b..8d3cb555 100644 --- a/src/main/conversation-system-context-snapshot.ts +++ b/src/main/conversation-system-context-snapshot.ts @@ -1,69 +1,25 @@ import type Database from 'better-sqlite3'; -import { - getBuiltInSkillDirs, - getScopePath, -} from './deepagent/skill-manager'; import { createGlobalSkillSceneExposureFilter } from './global-skill-scene-exposure'; -import { - resolveSkillCatalog, - resolveSkillSourcePlan, - type ResolvedSkillCatalogEntry, -} from './deepagent/skills-runtime/skill-sources'; +import { captureConversationSkillSnapshot } from './deepagent/skill-catalog'; import type { ProjectScene } from '../shared/types'; -import type { - ConversationSkillSnapshotEntry, - GlobalSkillSourceKind, - SkillSourceKind, -} from '../shared/skills'; +import type { ConversationSkillSnapshotEntry } from '../shared/skills'; export interface ConversationSystemContextSnapshot { promptSnapshot: string; skillSnapshot: ConversationSkillSnapshotEntry[]; } -function isGlobalSkillSourceKind(sourceKind: SkillSourceKind): sourceKind is GlobalSkillSourceKind { - return sourceKind === 'built-in' || sourceKind === 'user'; -} - - -function snapshotSkill(skill: ResolvedSkillCatalogEntry): ConversationSkillSnapshotEntry { - return { - name: skill.name, - qualifiedName: skill.qualifiedName, - qualifier: skill.qualifier, - description: skill.description, - argumentHint: skill.argumentHint, - allowedTools: skill.allowedTools, - whenToUse: skill.whenToUse, - arguments: skill.arguments, - sourceKind: skill.sourceKind, - sourcePath: skill.sourcePath, - skillPath: skill.skillPath, - modelDiscovery: skill.modelDiscovery, - userInvocable: skill.userInvocable, - }; -} - export function captureConversationSystemContextSnapshot(input: { projectPath: string; sceneId: ProjectScene; promptSnapshot: string; }): ConversationSystemContextSnapshot { - const plan = resolveSkillSourcePlan(input.projectPath, { - builtInSkillDirs: getBuiltInSkillDirs(), - userSkillsDir: getScopePath(input.projectPath, 'global'), - includeNestedProjectSkills: true, - }); - const isGlobalSkillExposed = createGlobalSkillSceneExposureFilter(input.sceneId); - const catalog = resolveSkillCatalog(plan, { - includeSkill: (source, name) => !isGlobalSkillSourceKind(source.kind) - || isGlobalSkillExposed({ sourceKind: source.kind, name }), - includeNestedProjectSkills: true, - }); - return { promptSnapshot: input.promptSnapshot, - skillSnapshot: catalog.skills.map(snapshotSkill), + skillSnapshot: captureConversationSkillSnapshot({ + projectPath: input.projectPath, + isGlobalSkillExposed: createGlobalSkillSceneExposureFilter(input.sceneId), + }), }; } diff --git a/src/main/deepagent/agent-tools.ts b/src/main/deepagent/agent-tools.ts index 9d02ec91..617ea72b 100644 --- a/src/main/deepagent/agent-tools.ts +++ b/src/main/deepagent/agent-tools.ts @@ -2,7 +2,7 @@ import { tool } from '@langchain/core/tools'; import { z } from 'zod'; import db from '../database'; import { createAgentCatalog, type CatalogAgent } from '../agent-catalog'; -import { listGlobalSkillViews } from './skill-manager'; +import { listGlobalSkillViews } from './skill-catalog'; const AGENT_NAME_REGEX = /^[A-Za-z0-9\s\-_]+$/; diff --git a/src/main/deepagent/context-aggregator.ts b/src/main/deepagent/context-aggregator.ts index 1c3aed1e..d870235e 100644 --- a/src/main/deepagent/context-aggregator.ts +++ b/src/main/deepagent/context-aggregator.ts @@ -17,13 +17,16 @@ import fs from 'fs'; import log from '../logger'; import path from 'path'; import db from '../database'; -import { getBuiltInSkillDirs, getScopePath } from './skill-manager'; +import { + buildProjectSkillsRuntime, + getSkillDisplayName, + getSkillSourceLabel, + type ResolvedSkillCatalogEntry, +} from './skill-catalog'; import { loadMcpTools } from './mcp-connector'; import { getAgentMcpServers, getConnectedMcpServers } from './mcp-visibility'; import type { MCPServer } from '../../shared/types'; import { skillReferencesToPreloadNames } from '../../shared/skill-identifiers'; -import { buildCdfSkillsRuntime } from './skills-runtime/cdf-skills-runtime'; -import type { ResolvedSkillCatalogEntry } from './skills-runtime/skill-sources'; import { getOrCaptureConversationSystemContextSnapshot } from '../conversation-system-context-snapshot'; import { createAgentCatalog } from '../agent-catalog'; import { buildProjectContext } from './project-context'; @@ -138,27 +141,6 @@ function stripSkillFrontmatter(content: string): string { : content.slice(end + '\n---'.length).replace(/^\s+/, ''); } -function getSkillDisplayName(skill: ResolvedSkillCatalogEntry): string { - return skill.qualifiedName ?? skill.name; -} - -function getSkillSourceLabel(skill: ResolvedSkillCatalogEntry): string { - switch (skill.sourceKind) { - case 'built-in': - return 'Built-in Skill'; - case 'project': - return 'Project Skill'; - case 'project-nested': - return skill.qualifier ? `Nested Project Skill: ${skill.qualifier}` : 'Nested Project Skill'; - case 'project-additional': - return skill.qualifier ? `Project Skill: ${skill.qualifier}` : 'Project Skill'; - case 'user': - return 'Global Skill'; - case 'enterprise': - return 'Managed Skill'; - } -} - function isPreloadedSkill(skill: ResolvedSkillCatalogEntry, preloadSkillNames: string[]): boolean { const preloadNames = new Set(preloadSkillNames); const displayName = getSkillDisplayName(skill); @@ -684,10 +666,8 @@ export async function aggregateCurrentSessionContext( sceneId: project.scene ?? 'general', promptSnapshot: createAgentCatalog(db, { initializeSchema: false }).resolveMaster(project.scene ?? 'general').system_prompt, }).skillSnapshot; - const skillsRuntime = buildCdfSkillsRuntime(projectPath, { + const skillsRuntime = buildProjectSkillsRuntime(projectPath, { catalog: skillSnapshot as ResolvedSkillCatalogEntry[], - builtInSkillDirs: getBuiltInSkillDirs(), - userSkillsDir: getScopePath(projectPath, 'global'), preloadSkillNames, }); for (const warning of skillsRuntime.warnings) { diff --git a/src/main/deepagent/runtime-assembly.ts b/src/main/deepagent/runtime-assembly.ts index 635b9e28..903dd1f8 100644 --- a/src/main/deepagent/runtime-assembly.ts +++ b/src/main/deepagent/runtime-assembly.ts @@ -15,11 +15,9 @@ import { import { createLangChainModel, type RuntimeProviderModelConfig } from './llm-adapter'; import { prepareAISubscriptionRuntimeModel } from '../ai-subscription-runtime'; import { - getBuiltInSkillDirs, - getScopePath, + buildProjectSkillsRuntime, resolveConversationSkillSnapshotConfig, -} from './skill-manager'; -import { buildCdfSkillsRuntime } from './skills-runtime/cdf-skills-runtime'; +} from './skill-catalog'; import { globalSkillReferenceToKey, skillReferencesToPreloadNames, @@ -30,7 +28,7 @@ import type { ConversationSkillSnapshotEntry } from '../../shared/skills'; import { buildProjectContext } from './project-context'; export { buildProjectContext } from './project-context'; -import type { ResolvedSkillCatalogEntry } from './skills-runtime/skill-sources'; +import type { ResolvedSkillCatalogEntry } from './skill-catalog'; // ============================================================================= // Provider 模型配置解析 @@ -262,7 +260,7 @@ export function buildCdfCapabilityToolsPrompt(toolNames: string[]): string { interface CdfSkillsAssemblyResult { permissions: FilesystemPermission[]; - skillsRuntime: ReturnType; + skillsRuntime: ReturnType; warnings: string[]; } @@ -281,9 +279,7 @@ export function buildCdfSkillsRuntimeAssembly( sceneId: ProjectScene = 'general', skillSnapshot?: readonly ConversationSkillSnapshotEntry[] | null, ): CdfSkillsAssemblyResult { - const skillsRuntime = buildCdfSkillsRuntime(projectPath, { - builtInSkillDirs: getBuiltInSkillDirs(), - userSkillsDir: getScopePath(projectPath, 'global'), + const skillsRuntime = buildProjectSkillsRuntime(projectPath, { preloadSkillKeys: getPreloadSkillKeys(skillNames), pathContext, sceneId, @@ -307,7 +303,7 @@ export interface DeepAgentAssemblyResult { provider: ProviderRow; permissions: FilesystemPermission[]; systemPrompt: string; - skillsRuntime: ReturnType; + skillsRuntime: ReturnType; assemblyWarnings: string[]; } diff --git a/src/main/deepagent/skill-catalog.test.ts b/src/main/deepagent/skill-catalog.test.ts new file mode 100644 index 00000000..24dce6fb --- /dev/null +++ b/src/main/deepagent/skill-catalog.test.ts @@ -0,0 +1,70 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { + captureConversationSkillSnapshot, + isGlobalSkillSourceKind, + resolveProjectSkillCatalog, +} from './skill-catalog'; + +const cleanupPaths: string[] = []; + +afterEach(() => { + for (const target of cleanupPaths.splice(0)) { + fs.rmSync(target, { recursive: true, force: true }); + } +}); + +function createProjectWithSkill(name: string): string { + const projectPath = fs.mkdtempSync(path.join(os.tmpdir(), 'cdf-skill-catalog-')); + cleanupPaths.push(projectPath); + const skillDir = path.join(projectPath, '.cdf', 'skills', name); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync( + path.join(skillDir, 'SKILL.md'), + `---\nname: ${name}\ndescription: Project-owned skill\n---\n\nInstructions\n`, + ); + return projectPath; +} + +describe('Skill Catalog', () => { + it('never routes Project Skills through the Scene exposure predicate', () => { + const projectPath = createProjectWithSkill('project-only-skill'); + + const catalog = resolveProjectSkillCatalog(projectPath, { + isGlobalSkillExposed: () => false, + }); + + expect(catalog.skills.map((skill) => skill.name)).toContain('project-only-skill'); + expect(catalog.skills.some((skill) => isGlobalSkillSourceKind(skill.sourceKind))).toBe(false); + }); + + it('applies both the includeSkill filter and the Scene exposure predicate', () => { + const projectPath = createProjectWithSkill('filtered-project-skill'); + + const catalog = resolveProjectSkillCatalog(projectPath, { + includeSkill: (_source, skillName) => skillName !== 'filtered-project-skill', + isGlobalSkillExposed: () => false, + }); + + expect(catalog.skills).toEqual([]); + }); + + it('captures a Conversation Skill Snapshot with only exposed sources', () => { + const projectPath = createProjectWithSkill('snapshot-skill'); + + const snapshot = captureConversationSkillSnapshot({ + projectPath, + isGlobalSkillExposed: () => false, + }); + + expect(snapshot.map((skill) => skill.name)).toEqual(['snapshot-skill']); + expect(snapshot[0]).toMatchObject({ + sourceKind: 'project', + skillPath: path.join(projectPath, '.cdf', 'skills', 'snapshot-skill', 'SKILL.md'), + userInvocable: true, + }); + expect(JSON.stringify(snapshot)).not.toContain('Instructions'); + }); +}); diff --git a/src/main/deepagent/skill-catalog.ts b/src/main/deepagent/skill-catalog.ts new file mode 100644 index 00000000..e461fd4f --- /dev/null +++ b/src/main/deepagent/skill-catalog.ts @@ -0,0 +1,156 @@ +import { + deletePhysicalSkill, + getBuiltInSkillDirs, + getBuiltInSkillRegistrations, + getScopePath, + importPhysicalSkillDirectory, + listGlobalSkillViews, + listPhysicalSkills, + listResolvedSkillViews, + resolveAgentSkillsConfig, + resolveConversationSkillSnapshotConfig, + savePhysicalSkill, +} from './skill-manager'; +import { + getSkillDisplayName, + getSkillSourceLabel, + isGlobalSkillSourceKind, + resolveSkillCatalog, + resolveSkillSourcePlan, + type ResolvedSkillCatalog, + type ResolvedSkillCatalogEntry, + type SkillCatalogOptions, + type SkillSourceEntry, +} from './skills-runtime/skill-sources'; +import { + buildCdfSkillsRuntime, + type CdfSkillsRuntime, + type CdfSkillsRuntimeOptions, +} from './skills-runtime/cdf-skills-runtime'; +import type { + ConversationSkillSnapshotEntry, + GlobalSkillSourceKind, +} from '../../shared/skills'; + +/** + * Skill Catalog——"存在哪些 Skill、对谁可见"的唯一回答者。 + * + * 深模块入口：discovery（source plan）、resolution（目录解析 + 遮蔽 + + * Scene 曝光过滤）、Conversation Skill Snapshot 捕获、Settings 视图、 + * 物理 Skill CRUD 与运行时 prompt 视图统一从这里出。skill-manager 与 + * skills-runtime/* 是实现文件，skill 家族之外的消费方不应再直接导入它们。 + * + * Scene Skill Exposure 策略保持在曝光策略模块（ADR-0069），以 + * `isGlobalSkillExposed` 谓词注入；Project Skill 永远不经过该谓词。 + */ + +export { + deletePhysicalSkill, + getBuiltInSkillRegistrations, + importPhysicalSkillDirectory, + listGlobalSkillViews, + listPhysicalSkills, + listResolvedSkillViews, + resolveAgentSkillsConfig, + resolveConversationSkillSnapshotConfig, + savePhysicalSkill, +}; +export { getSkillDisplayName, getSkillSourceLabel, isGlobalSkillSourceKind }; +export type { + CdfSkillsRuntime, + CdfSkillsRuntimeOptions, + ResolvedSkillCatalog, + ResolvedSkillCatalogEntry, + SkillCatalogOptions, +}; + +export type GlobalSkillExposurePredicate = ( + skill: { sourceKind: GlobalSkillSourceKind; name: string } +) => boolean; + +export interface ResolveProjectSkillCatalogOptions extends SkillCatalogOptions { + /** Scene 曝光策略谓词，只作用于 Global（built-in / user）来源。 */ + isGlobalSkillExposed?: GlobalSkillExposurePredicate; +} + +function composeIncludeSkill( + isGlobalSkillExposed: GlobalSkillExposurePredicate | undefined, + includeSkill: SkillCatalogOptions['includeSkill'], +): SkillCatalogOptions['includeSkill'] { + if (!isGlobalSkillExposed) return includeSkill; + return (source: SkillSourceEntry, skillName: string) => { + if (includeSkill && !includeSkill(source, skillName)) return false; + return !isGlobalSkillSourceKind(source.kind) + || isGlobalSkillExposed({ sourceKind: source.kind, name: skillName }); + }; +} + +/** + * 项目 Skill 目录的规范解析：Built-in 目录与用户 Global 目录默认铺入 + * source plan，消费方不再自行拼装 plan → catalog 两步。 + */ +export function resolveProjectSkillCatalog( + projectPath: string, + options: ResolveProjectSkillCatalogOptions = {} +): ResolvedSkillCatalog { + const { isGlobalSkillExposed, includeSkill, ...catalogOptions } = options; + const plan = resolveSkillSourcePlan(projectPath, { + builtInSkillDirs: getBuiltInSkillDirs(), + userSkillsDir: getScopePath(projectPath, 'global'), + includeNestedProjectSkills: options.includeNestedProjectSkills, + }); + return resolveSkillCatalog(plan, { + ...catalogOptions, + includeSkill: composeIncludeSkill(isGlobalSkillExposed, includeSkill), + }); +} + +function toConversationSkillSnapshotEntry( + skill: ResolvedSkillCatalogEntry +): ConversationSkillSnapshotEntry { + return { + name: skill.name, + qualifiedName: skill.qualifiedName, + qualifier: skill.qualifier, + description: skill.description, + argumentHint: skill.argumentHint, + allowedTools: skill.allowedTools, + whenToUse: skill.whenToUse, + arguments: skill.arguments, + sourceKind: skill.sourceKind, + sourcePath: skill.sourcePath, + skillPath: skill.skillPath, + modelDiscovery: skill.modelDiscovery, + userInvocable: skill.userInvocable, + }; +} + +/** + * 捕获 Conversation Skill Snapshot：Conversation 创建时冻结的 Skill 身份与 + * 发现元数据集合。持久化（sessions 表读写）由快照持久化模块负责。 + */ +export function captureConversationSkillSnapshot(input: { + projectPath: string; + isGlobalSkillExposed: GlobalSkillExposurePredicate; +}): ConversationSkillSnapshotEntry[] { + const catalog = resolveProjectSkillCatalog(input.projectPath, { + includeNestedProjectSkills: true, + isGlobalSkillExposed: input.isGlobalSkillExposed, + }); + return catalog.skills.map(toConversationSkillSnapshotEntry); +} + +/** + * 运行时 Skill 视图（已解析集合 + prompt + attributions），Built-in 与用户 + * Global 目录默认铺入；传入 `catalog`（冻结快照）时不做磁盘发现。 + */ +export function buildProjectSkillsRuntime( + projectPath: string, + options: CdfSkillsRuntimeOptions = {} +): CdfSkillsRuntime { + return buildCdfSkillsRuntime(projectPath, { + builtInSkillDirs: getBuiltInSkillDirs(), + userSkillsDir: getScopePath(projectPath, 'global'), + ...options, + }); +} diff --git a/src/main/deepagent/skill-manager.ts b/src/main/deepagent/skill-manager.ts index f978fcef..621d498a 100644 --- a/src/main/deepagent/skill-manager.ts +++ b/src/main/deepagent/skill-manager.ts @@ -13,6 +13,7 @@ import { getAcademicStyleRevisionSkillMarkdown, getAcademicStyleRevisionSkillRes import { getPaperSearchSkillMarkdown, getPaperSearchSkillResources } from '../paper-search-skill'; import { getPdfParsingSkillMarkdown, getPdfParsingSkillResources } from '../pdf-parsing-skill'; import { + getSkillSourceLabel, invalidateSkillSourceCaches, resolveSkillCatalog, resolveSkillSourcePlan, @@ -433,23 +434,6 @@ function getResolvedSkillScope(skill: ResolvedSkillCatalogEntry): SkillScope { return classifySkillSourceKind(skill.sourceKind) === 'global' ? 'global' : 'project'; } -function getResolvedSkillSourceLabel(skill: Pick): string { - switch (skill.sourceKind) { - case 'built-in': - return 'Built-in Skill'; - case 'project': - return 'Project Skill'; - case 'project-nested': - return skill.qualifier ? `Nested Project Skill: ${skill.qualifier}` : 'Nested Project Skill'; - case 'project-additional': - return skill.qualifier ? `Project Skill: ${skill.qualifier}` : 'Project Skill'; - case 'user': - return 'Global Skill'; - case 'enterprise': - return 'Managed Skill'; - } -} - function isResolvedSkillEditable(skill: ResolvedSkillCatalogEntry): boolean { return skill.sourceKind === 'project' || skill.sourceKind === 'user'; } @@ -465,7 +449,7 @@ function buildResolvedSkillView(skill: ResolvedSkillCatalogEntry): PhysicalSkill description: skill.description, scope: getResolvedSkillScope(skill), sourceKind: skill.sourceKind, - sourceLabel: getResolvedSkillSourceLabel(skill), + sourceLabel: getSkillSourceLabel(skill), sourcePath: skill.sourcePath, skillPath: skill.skillPath, modelDiscovery: skill.modelDiscovery, @@ -478,7 +462,7 @@ function buildResolvedSkillView(skill: ResolvedSkillCatalogEntry): PhysicalSkill name: shadowed.name, qualifiedName: shadowed.qualifiedName ?? shadowed.name, sourceKind: shadowed.sourceKind, - sourceLabel: getResolvedSkillSourceLabel(shadowed), + sourceLabel: getSkillSourceLabel(shadowed), sourcePath: shadowed.sourcePath, skillPath: shadowed.skillPath, })), diff --git a/src/main/deepagent/skills-runtime/cdf-skills-runtime.ts b/src/main/deepagent/skills-runtime/cdf-skills-runtime.ts index c20ceee4..cadb8335 100644 --- a/src/main/deepagent/skills-runtime/cdf-skills-runtime.ts +++ b/src/main/deepagent/skills-runtime/cdf-skills-runtime.ts @@ -1,6 +1,9 @@ import fs from 'fs'; import { renderCdfSkillsPrompt } from './skill-prompt'; import { + getSkillDisplayName, + getSkillSourceLabel, + isGlobalSkillSourceKind, resolveSkillCatalog, resolveSkillSourcePlan, type ResolvedSkillCatalogEntry, @@ -8,7 +11,7 @@ import { type SkillSourcePlanOptions, } from './skill-sources'; import type { SkillAttribution } from '../../../shared/types'; -import type { GlobalSkillSourceKind, SkillSourceKind } from '../../../shared/skills'; +import type { GlobalSkillSourceKind } from '../../../shared/skills'; import { resolvedGlobalSkillKey } from '../../../shared/skill-identifiers'; export interface CdfSkillsRuntimeOptions extends SkillSourcePlanOptions, SkillCatalogOptions { @@ -23,10 +26,6 @@ export interface CdfSkillsRuntimeOptions extends SkillSourcePlanOptions, SkillCa isGlobalSkillExposed?: (skill: { sourceKind: GlobalSkillSourceKind; name: string }) => boolean; } -function isGlobalSkillSourceKind(sourceKind: SkillSourceKind): sourceKind is GlobalSkillSourceKind { - return sourceKind === 'built-in' || sourceKind === 'user'; -} - export interface CdfSkillsRuntime { skills: ResolvedSkillCatalogEntry[]; prompt: string; @@ -42,27 +41,6 @@ function stripSkillFrontmatter(content: string): string { : content.slice(end + '\n---'.length).replace(/^\s+/, ''); } -function getSkillDisplayName(skill: ResolvedSkillCatalogEntry): string { - return skill.qualifiedName ?? skill.name; -} - -function getSkillSourceLabel(skill: ResolvedSkillCatalogEntry): string { - switch (skill.sourceKind) { - case 'built-in': - return 'Built-in Skill'; - case 'project': - return 'Project Skill'; - case 'project-nested': - return skill.qualifier ? `Nested Project Skill: ${skill.qualifier}` : 'Nested Project Skill'; - case 'project-additional': - return skill.qualifier ? `Project Skill: ${skill.qualifier}` : 'Project Skill'; - case 'user': - return 'Global Skill'; - case 'enterprise': - return 'Managed Skill'; - } -} - function isPreloadedSkill( skill: ResolvedSkillCatalogEntry, preloadSkillNames: string[] | undefined, diff --git a/src/main/deepagent/skills-runtime/skill-sources.ts b/src/main/deepagent/skills-runtime/skill-sources.ts index ba92f7d4..1b9f5f1c 100644 --- a/src/main/deepagent/skills-runtime/skill-sources.ts +++ b/src/main/deepagent/skills-runtime/skill-sources.ts @@ -1,7 +1,7 @@ import fs from 'fs'; import path from 'path'; import { parseSkillMetadata } from './skill-metadata'; -import type { SkillSourceKind } from '../../../shared/skills'; +import type { GlobalSkillSourceKind, SkillSourceKind } from '../../../shared/skills'; export type { SkillSourceKind } from '../../../shared/skills'; import type { SkillModelDiscovery } from '../../../shared/skills'; @@ -60,6 +60,37 @@ export interface ResolvedSkillCatalog { warnings: string[]; } +export function isGlobalSkillSourceKind( + sourceKind: SkillSourceKind +): sourceKind is GlobalSkillSourceKind { + return sourceKind === 'built-in' || sourceKind === 'user'; +} + +export function getSkillDisplayName( + skill: Pick +): string { + return skill.qualifiedName ?? skill.name; +} + +export function getSkillSourceLabel( + skill: Pick +): string { + switch (skill.sourceKind) { + case 'built-in': + return 'Built-in Skill'; + case 'project': + return 'Project Skill'; + case 'project-nested': + return skill.qualifier ? `Nested Project Skill: ${skill.qualifier}` : 'Nested Project Skill'; + case 'project-additional': + return skill.qualifier ? `Project Skill: ${skill.qualifier}` : 'Project Skill'; + case 'user': + return 'Global Skill'; + case 'enterprise': + return 'Managed Skill'; + } +} + export interface SkillSourcePlanOptions { builtInSkillDirs?: string[]; userSkillsDir?: string | null; diff --git a/src/main/global-skill-scene-exposure.ts b/src/main/global-skill-scene-exposure.ts index efa7c507..c01a4b4a 100644 --- a/src/main/global-skill-scene-exposure.ts +++ b/src/main/global-skill-scene-exposure.ts @@ -1,6 +1,6 @@ import store from './store'; import { createSceneSkillExposureService } from './scene-skill-exposure'; -import { getBuiltInSkillRegistrations } from './deepagent/skill-manager'; +import { getBuiltInSkillRegistrations } from './deepagent/skill-catalog'; import type { ProjectScene } from '../shared/types'; /** Resolves whether a Global Skill is exposed in a Project Scene. */ diff --git a/src/main/ipc-handlers.ts b/src/main/ipc-handlers.ts index d0410a38..a5e71621 100644 --- a/src/main/ipc-handlers.ts +++ b/src/main/ipc-handlers.ts @@ -28,7 +28,7 @@ import { deletePhysicalSkill, importPhysicalSkillDirectory, getBuiltInSkillRegistrations, -} from './deepagent/skill-manager'; +} from './deepagent/skill-catalog'; import type { GlobalSkillReference, SceneSkillExposureInput } from '../shared/skills'; import { isRegisteredSceneId, SCENE_REGISTRY } from '../shared/scenes'; import { createSceneSkillExposureService } from './scene-skill-exposure'; From 9c0cd24485d85ad21e7b68ac45c84061e642dd33 Mon Sep 17 00:00:00 2001 From: suntianc Date: Sun, 26 Jul 2026 10:25:31 -0700 Subject: [PATCH 3/8] =?UTF-8?q?refactor(flow-diagrams):=20=E5=BB=BA?= =?UTF-8?q?=E7=AB=8B=E6=96=87=E6=A1=A3=E5=AD=98=E5=82=A8=E7=BB=9F=E4=B8=80?= =?UTF-8?q?=E4=B8=A4=E6=9D=A1=E5=86=99=E8=B7=AF=E5=BE=84=E7=9A=84=E4=B8=80?= =?UTF-8?q?=E8=87=B4=E6=80=A7=20(#200)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 新增 flow-diagram-document-store：原子 CAS 替换、路径限域、写边界场景 校验、按 Project 写串行化、SOURCE_CHANGED 冲突附带当前内容 - 编辑器 autosave 从通用 fs:writeFile 的读-比-写 CAS（非原子，有 TOCTOU 窗口）迁移到 flow-diagram:save-document 专用 IPC，落盘改为 temp+rename 原子发布；冲突时立即保留用户未保存内容进既有冲突横幅 - 通用文件写入删除 expectedContent CAS 参数（编辑器是唯一消费方） - service 的原子替换/路径解析/哈希原语移入存储，service 只保留动作编排； 工具动作集、结果与错误码不变（ADR-0071/0072 决策不变） - 新增存储并发场景测试：CAS 冲突返回当前内容、并发写串行化、无残留 临时文件、无效场景不落盘；stale-autosave 集成用例迁至存储 seam Co-Authored-By: Claude Fable 5 --- .../flow-diagram-document-store.test.ts | 121 +++++++ .../flow-diagram-document-store.ts | 311 ++++++++++++++++++ .../flow-diagram-service.integration.test.ts | 11 +- src/main/flow-diagram/flow-diagram-service.ts | 202 +----------- src/main/ipc-handlers.ts | 16 +- src/main/services/file-system.test.ts | 8 - src/main/services/file-system.ts | 10 - src/preload/index.ts | 6 +- .../FilePanel/FlowDiagramEditor.tsx | 15 +- src/shared/flow-diagrams.ts | 11 + src/shared/ipc-contract.ts | 9 +- 11 files changed, 497 insertions(+), 223 deletions(-) create mode 100644 src/main/flow-diagram/flow-diagram-document-store.test.ts create mode 100644 src/main/flow-diagram/flow-diagram-document-store.ts diff --git a/src/main/flow-diagram/flow-diagram-document-store.test.ts b/src/main/flow-diagram/flow-diagram-document-store.test.ts new file mode 100644 index 00000000..3fe39a97 --- /dev/null +++ b/src/main/flow-diagram/flow-diagram-document-store.test.ts @@ -0,0 +1,121 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { createFlowDiagramDocumentStore } from './flow-diagram-document-store'; + +function sceneJson(elementIds: string[] = []): string { + return JSON.stringify({ + type: 'excalidraw', + version: 2, + source: 'https://cdf.local', + elements: elementIds.map((id) => ({ + id, + type: 'rectangle', + x: 10, + y: 10, + width: 100, + height: 60, + })), + appState: { viewBackgroundColor: '#ffffff' }, + files: {}, + }, null, 2); +} + +describe('FlowDiagramDocumentStore', () => { + let projectPath: string; + let filePath: string; + + beforeEach(() => { + projectPath = fs.mkdtempSync(path.join(os.tmpdir(), 'cdf-flow-doc-store-')); + filePath = path.join(projectPath, 'diagram.excalidraw'); + }); + + afterEach(() => { + fs.rmSync(projectPath, { recursive: true, force: true }); + }); + + it('atomically replaces the document when the base content still matches', async () => { + const original = sceneJson(['one']); + fs.writeFileSync(filePath, original); + const notified: string[] = []; + const store = createFlowDiagramDocumentStore({ + projectPath, + notifyFileChange: (changed) => notified.push(changed), + }); + + const next = sceneJson(['one', 'two']); + await expect(store.saveDocument('diagram.excalidraw', next, original)) + .resolves.toEqual({ ok: true }); + expect(fs.readFileSync(filePath, 'utf-8')).toBe(next); + expect(notified).toEqual([filePath]); + expect(fs.readdirSync(projectPath).filter((name) => name.includes('cdf-tmp'))).toEqual([]); + }); + + it('returns a conflict with the current content when the document changed externally', async () => { + const original = sceneJson(['one']); + const external = sceneJson(['agent']); + fs.writeFileSync(filePath, external); + const store = createFlowDiagramDocumentStore({ projectPath }); + + const result = await store.saveDocument('diagram.excalidraw', sceneJson(['mine']), original); + + expect(result).toEqual({ + ok: false, + error: { + code: 'SOURCE_CHANGED', + message: expect.stringContaining('changed'), + currentContent: external, + }, + }); + expect(fs.readFileSync(filePath, 'utf-8')).toBe(external); + }); + + it('serializes concurrent saves so the later writer observes the earlier one', async () => { + const original = sceneJson(['one']); + fs.writeFileSync(filePath, original); + const store = createFlowDiagramDocumentStore({ projectPath }); + + const [first, second] = await Promise.all([ + store.saveDocument('diagram.excalidraw', sceneJson(['first']), original), + store.saveDocument('diagram.excalidraw', sceneJson(['second']), original), + ]); + + const outcomes = [first, second]; + expect(outcomes.filter((outcome) => outcome.ok)).toHaveLength(1); + const conflict = outcomes.find((outcome) => !outcome.ok); + expect(conflict).toMatchObject({ ok: false, error: { code: 'SOURCE_CHANGED' } }); + const finalContent = fs.readFileSync(filePath, 'utf-8'); + expect([sceneJson(['first']), sceneJson(['second'])]).toContain(finalContent); + }); + + it('writes without a CAS guard when no base content is supplied', async () => { + fs.writeFileSync(filePath, sceneJson(['whatever'])); + const store = createFlowDiagramDocumentStore({ projectPath }); + + const next = sceneJson(['fresh']); + await expect(store.saveDocument('diagram.excalidraw', next, null)) + .resolves.toEqual({ ok: true }); + expect(fs.readFileSync(filePath, 'utf-8')).toBe(next); + }); + + it('rejects invalid scenes at the write boundary without touching the document', async () => { + const original = sceneJson(['one']); + fs.writeFileSync(filePath, original); + const store = createFlowDiagramDocumentStore({ projectPath }); + + const result = await store.saveDocument('diagram.excalidraw', '{"not":"excalidraw"}', original); + + expect(result.ok).toBe(false); + expect(fs.readFileSync(filePath, 'utf-8')).toBe(original); + }); + + it('rejects documents outside the Project and non-diagram extensions', async () => { + const store = createFlowDiagramDocumentStore({ projectPath }); + + await expect(store.saveDocument('../outside.excalidraw', sceneJson(), null)) + .resolves.toMatchObject({ ok: false, error: { code: 'PATH_OUTSIDE_PROJECT' } }); + await expect(store.saveDocument('notes.txt', sceneJson(), null)) + .resolves.toMatchObject({ ok: false, error: { code: 'INVALID_EXTENSION' } }); + }); +}); diff --git a/src/main/flow-diagram/flow-diagram-document-store.ts b/src/main/flow-diagram/flow-diagram-document-store.ts new file mode 100644 index 00000000..6defeedb --- /dev/null +++ b/src/main/flow-diagram/flow-diagram-document-store.ts @@ -0,0 +1,311 @@ +import crypto from 'crypto'; +import fs from 'fs'; +import path from 'path'; +import { runProjectFileMutation } from '../services/project-file-mutation'; +import { + FlowDiagramSceneError, + parseFlowDiagramScene, +} from './flow-diagram-scene'; +import type { FlowDiagramDocumentSaveResult } from '../../shared/flow-diagrams'; + +/** + * Flow Diagram 文档存储：`.excalidraw` 文档一致性的唯一拥有者。 + * + * 用户编辑器 autosave 与 Agent `manage_flow_diagram` 编辑共享同一套 + * 原子 compare-and-swap 替换原语与按 Project 的写串行化；写边界执行 + * 场景校验，冲突以带当前内容的结构化结果返回（ADR-0071 / #200）。 + */ + +export class FlowDiagramOperationError extends Error { + constructor( + public readonly code: string, + message: string, + ) { + super(message); + this.name = 'FlowDiagramOperationError'; + } +} + +const PROTECTED_SEGMENTS = new Set(['.git', '.cdf', 'node_modules', 'out', 'dist']); + +function isWithin(rootPath: string, candidatePath: string): boolean { + const relative = path.relative(rootPath, candidatePath); + return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative)); +} + +function nearestExistingAncestor(candidatePath: string): string { + let current = candidatePath; + while (!fs.existsSync(current)) { + const parent = path.dirname(current); + if (parent === current) break; + current = parent; + } + return current; +} + +export function resolveProjectOwnedPath( + projectPath: string, + requestedPath: string, + expectedExtension: string, +): string { + const trimmed = requestedPath.trim(); + if (!trimmed) { + throw new FlowDiagramOperationError('PATH_REQUIRED', 'A non-empty Project path is required.'); + } + if (trimmed.startsWith('~')) { + throw new FlowDiagramOperationError('PATH_OUTSIDE_PROJECT', 'Home-relative paths are not allowed.'); + } + const resolvedProjectPath = fs.realpathSync(projectPath); + const target = path.resolve(projectPath, trimmed); + if (!isWithin(path.resolve(projectPath), target)) { + throw new FlowDiagramOperationError( + 'PATH_OUTSIDE_PROJECT', + 'The requested path is outside the current Project.', + ); + } + const relative = path.relative(projectPath, target); + const segments = relative.toLowerCase().split(path.sep).filter(Boolean); + if ( + segments.some((segment) => PROTECTED_SEGMENTS.has(segment)) + || segments.some((segment) => segment === '.env' || segment.startsWith('.env.')) + ) { + throw new FlowDiagramOperationError( + 'PROTECTED_PATH', + 'The requested path is protected and cannot be used for a Flow Diagram.', + ); + } + if (path.extname(target).toLowerCase() !== expectedExtension) { + throw new FlowDiagramOperationError( + 'INVALID_EXTENSION', + `The requested path must end with ${expectedExtension}.`, + ); + } + + const ancestor = nearestExistingAncestor(target); + const realAncestor = fs.realpathSync(ancestor); + if (!isWithin(resolvedProjectPath, realAncestor)) { + throw new FlowDiagramOperationError( + 'PATH_OUTSIDE_PROJECT', + 'The requested path resolves outside the current Project.', + ); + } + if (fs.existsSync(target) && fs.lstatSync(target).isSymbolicLink()) { + throw new FlowDiagramOperationError('SYMLINK_NOT_ALLOWED', 'Flow Diagram paths cannot be symlinks.'); + } + return target; +} + +export function hashBytes(bytes: Buffer): string { + return crypto.createHash('sha256').update(bytes).digest('hex'); +} + +function temporaryPathFor(targetPath: string): string { + return path.join( + path.dirname(targetPath), + `.${path.basename(targetPath)}.cdf-tmp-${process.pid}-${crypto.randomBytes(6).toString('hex')}`, + ); +} + +export async function replaceFileAtomicallyIfUnchanged( + filePath: string, + bytes: Buffer, + expectedBytes: Buffer | null, +): Promise { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + const temporaryPath = temporaryPathFor(filePath); + try { + const handle = await fs.promises.open(temporaryPath, 'wx', 0o600); + try { + await handle.writeFile(bytes); + await handle.sync(); + } finally { + await handle.close(); + } + + if (expectedBytes === null) { + try { + await fs.promises.link(temporaryPath, filePath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EEXIST') { + throw new FlowDiagramOperationError( + 'SOURCE_CHANGED', + 'The Flow Diagram changed before the operation could be applied.', + ); + } + throw error; + } + return; + } + + let currentBytes: Buffer; + try { + currentBytes = fs.readFileSync(filePath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + throw new FlowDiagramOperationError( + 'SOURCE_CHANGED', + 'The Flow Diagram changed before the operation could be applied.', + ); + } + throw error; + } + if (!currentBytes.equals(expectedBytes)) { + throw new FlowDiagramOperationError( + 'SOURCE_CHANGED', + 'The Flow Diagram changed before the operation could be applied.', + ); + } + // CDF mutations for this Project hold the shared coordinator lock. rename is + // the single atomic publication step, so readers never observe partial bytes. + await fs.promises.rename(temporaryPath, filePath); + } finally { + await fs.promises.rm(temporaryPath, { force: true }); + } +} + +/** Unconditional atomic replacement: same temp + rename publication, no CAS guard. */ +async function replaceFileAtomically(filePath: string, bytes: Buffer): Promise { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + const temporaryPath = temporaryPathFor(filePath); + try { + const handle = await fs.promises.open(temporaryPath, 'wx', 0o600); + try { + await handle.writeFile(bytes); + await handle.sync(); + } finally { + await handle.close(); + } + await fs.promises.rename(temporaryPath, filePath); + } finally { + await fs.promises.rm(temporaryPath, { force: true }); + } +} + +export async function removeFileAtomicallyIfUnchanged( + filePath: string, + expectedBytes: Buffer, +): Promise { + let currentBytes: Buffer; + try { + currentBytes = fs.readFileSync(filePath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + throw new FlowDiagramOperationError( + 'SOURCE_CHANGED', + 'The Flow Diagram changed before the operation could be completed.', + ); + } + throw error; + } + if (!currentBytes.equals(expectedBytes)) { + throw new FlowDiagramOperationError( + 'SOURCE_CHANGED', + 'The Flow Diagram changed before the operation could be completed.', + ); + } + await fs.promises.unlink(filePath); +} + +export async function writeNewFileAtomically(filePath: string, bytes: Buffer): Promise { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + const temporaryPath = temporaryPathFor(filePath); + try { + await fs.promises.writeFile(temporaryPath, bytes, { flag: 'wx', mode: 0o600 }); + try { + await fs.promises.link(temporaryPath, filePath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EEXIST') { + throw new FlowDiagramOperationError( + 'FILE_EXISTS', + 'The requested output already exists; no file was overwritten.', + ); + } + throw error; + } + } finally { + await fs.promises.rm(temporaryPath, { force: true }); + } +} + +export interface CreateFlowDiagramDocumentStoreOptions { + projectPath: string; + notifyFileChange?: (filePath: string) => void; +} + +export interface FlowDiagramDocumentStore { + /** + * Editor autosave entry: validates the scene, then atomically replaces the + * document when its bytes still equal `expectedContent`. `null` skips the + * CAS guard but keeps the atomic temp + rename publication. A conflict + * returns the current on-disk content so the caller can surface it. + */ + saveDocument( + filePath: string, + content: string, + expectedContent: string | null, + ): Promise; +} + +export function createFlowDiagramDocumentStore( + options: CreateFlowDiagramDocumentStoreOptions, +): FlowDiagramDocumentStore { + const projectPath = path.resolve(options.projectPath); + const notify = (filePath: string) => options.notifyFileChange?.(filePath); + + return { + async saveDocument(filePath, content, expectedContent) { + let target: string; + try { + target = resolveProjectOwnedPath(projectPath, filePath, '.excalidraw'); + } catch (error) { + return failureResult(error, 'PATH_OUTSIDE_PROJECT'); + } + try { + parseFlowDiagramScene(content); + } catch (error) { + return failureResult(error, 'INVALID_SCENE'); + } + + return runProjectFileMutation(projectPath, async () => { + try { + const bytes = Buffer.from(content, 'utf-8'); + if (expectedContent === null) { + await replaceFileAtomically(target, bytes); + } else { + await replaceFileAtomicallyIfUnchanged( + target, + bytes, + Buffer.from(expectedContent, 'utf-8'), + ); + } + notify(target); + return { ok: true } as const; + } catch (error) { + if (error instanceof FlowDiagramOperationError && error.code === 'SOURCE_CHANGED') { + let currentContent: string | null = null; + try { + currentContent = fs.readFileSync(target, 'utf-8'); + } catch { + currentContent = null; + } + return { + ok: false as const, + error: { code: 'SOURCE_CHANGED', message: error.message, currentContent }, + }; + } + return failureResult(error, 'WRITE_FAILED'); + } + }); + }, + }; +} + +function failureResult(error: unknown, fallbackCode: string): FlowDiagramDocumentSaveResult { + if (error instanceof FlowDiagramOperationError || error instanceof FlowDiagramSceneError) { + return { ok: false, error: { code: error.code, message: error.message } }; + } + return { + ok: false, + error: { code: fallbackCode, message: 'The Flow Diagram document could not be saved safely.' }, + }; +} diff --git a/src/main/flow-diagram/flow-diagram-service.integration.test.ts b/src/main/flow-diagram/flow-diagram-service.integration.test.ts index 3cdbcec0..23c516b7 100644 --- a/src/main/flow-diagram/flow-diagram-service.integration.test.ts +++ b/src/main/flow-diagram/flow-diagram-service.integration.test.ts @@ -9,7 +9,7 @@ import { type FlowDiagramService, } from './flow-diagram-service'; import type { ExcalidrawScene } from './flow-diagram-scene'; -import { writeFile as writeProjectFile } from '../services/file-system'; +import { createFlowDiagramDocumentStore } from './flow-diagram-document-store'; function scene(elements: Array> = []): ExcalidrawScene { return { @@ -450,8 +450,8 @@ describe('FlowDiagramService integration', () => { operations: [{ op: 'add', elements: [rectangle('agent')] }], }); await revisionStartedPromise; - const staleAutosave = writeProjectFile( - projectPath, + const documentStore = createFlowDiagramDocumentStore({ projectPath }); + const staleAutosave = documentStore.saveDocument( filePath, original.toString('utf8'), original.toString('utf8'), @@ -459,7 +459,10 @@ describe('FlowDiagramService integration', () => { releaseRevision(); await expect(agentEdit).resolves.toMatchObject({ ok: true }); - await expect(staleAutosave).rejects.toMatchObject({ code: 'ECONFLICT' }); + await expect(staleAutosave).resolves.toMatchObject({ + ok: false, + error: { code: 'SOURCE_CHANGED' }, + }); expect(fs.readFileSync(filePath, 'utf8')).toContain('"id": "agent"'); }); diff --git a/src/main/flow-diagram/flow-diagram-service.ts b/src/main/flow-diagram/flow-diagram-service.ts index 461108ca..3925ca51 100644 --- a/src/main/flow-diagram/flow-diagram-service.ts +++ b/src/main/flow-diagram/flow-diagram-service.ts @@ -1,6 +1,13 @@ -import crypto from 'crypto'; import fs from 'fs'; import path from 'path'; +import { + FlowDiagramOperationError, + hashBytes, + removeFileAtomicallyIfUnchanged, + replaceFileAtomicallyIfUnchanged, + resolveProjectOwnedPath, + writeNewFileAtomically, +} from './flow-diagram-document-store'; import { createFlowDiagramScene, EXCALIDRAW_SDK_VERSION, @@ -93,85 +100,6 @@ export interface CreateFlowDiagramServiceOptions { notifyFileChange?: (filePath: string) => void; } -class FlowDiagramOperationError extends Error { - constructor( - public readonly code: string, - message: string, - ) { - super(message); - this.name = 'FlowDiagramOperationError'; - } -} - -const PROTECTED_SEGMENTS = new Set(['.git', '.cdf', 'node_modules', 'out', 'dist']); - -function isWithin(rootPath: string, candidatePath: string): boolean { - const relative = path.relative(rootPath, candidatePath); - return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative)); -} - -function nearestExistingAncestor(candidatePath: string): string { - let current = candidatePath; - while (!fs.existsSync(current)) { - const parent = path.dirname(current); - if (parent === current) break; - current = parent; - } - return current; -} - -function resolveProjectOwnedPath( - projectPath: string, - requestedPath: string, - expectedExtension: string, -): string { - const trimmed = requestedPath.trim(); - if (!trimmed) { - throw new FlowDiagramOperationError('PATH_REQUIRED', 'A non-empty Project path is required.'); - } - if (trimmed.startsWith('~')) { - throw new FlowDiagramOperationError('PATH_OUTSIDE_PROJECT', 'Home-relative paths are not allowed.'); - } - const resolvedProjectPath = fs.realpathSync(projectPath); - const target = path.resolve(projectPath, trimmed); - if (!isWithin(path.resolve(projectPath), target)) { - throw new FlowDiagramOperationError( - 'PATH_OUTSIDE_PROJECT', - 'The requested path is outside the current Project.', - ); - } - const relative = path.relative(projectPath, target); - const segments = relative.toLowerCase().split(path.sep).filter(Boolean); - if ( - segments.some((segment) => PROTECTED_SEGMENTS.has(segment)) - || segments.some((segment) => segment === '.env' || segment.startsWith('.env.')) - ) { - throw new FlowDiagramOperationError( - 'PROTECTED_PATH', - 'The requested path is protected and cannot be used for a Flow Diagram.', - ); - } - if (path.extname(target).toLowerCase() !== expectedExtension) { - throw new FlowDiagramOperationError( - 'INVALID_EXTENSION', - `The requested path must end with ${expectedExtension}.`, - ); - } - - const ancestor = nearestExistingAncestor(target); - const realAncestor = fs.realpathSync(ancestor); - if (!isWithin(resolvedProjectPath, realAncestor)) { - throw new FlowDiagramOperationError( - 'PATH_OUTSIDE_PROJECT', - 'The requested path resolves outside the current Project.', - ); - } - if (fs.existsSync(target) && fs.lstatSync(target).isSymbolicLink()) { - throw new FlowDiagramOperationError('SYMLINK_NOT_ALLOWED', 'Flow Diagram paths cannot be symlinks.'); - } - return target; -} - function safeName(value: string | undefined): string { const normalized = (value ?? 'flow-diagram') .normalize('NFKD') @@ -217,120 +145,6 @@ function fileData(projectPath: string, filePath: string): Record { - fs.mkdirSync(path.dirname(filePath), { recursive: true }); - const temporaryPath = temporaryPathFor(filePath); - try { - const handle = await fs.promises.open(temporaryPath, 'wx', 0o600); - try { - await handle.writeFile(bytes); - await handle.sync(); - } finally { - await handle.close(); - } - - if (expectedBytes === null) { - try { - await fs.promises.link(temporaryPath, filePath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'EEXIST') { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be applied.', - ); - } - throw error; - } - return; - } - - let currentBytes: Buffer; - try { - currentBytes = fs.readFileSync(filePath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be applied.', - ); - } - throw error; - } - if (!currentBytes.equals(expectedBytes)) { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be applied.', - ); - } - // CDF mutations for this Project hold the shared coordinator lock. rename is - // the single atomic publication step, so readers never observe partial bytes. - await fs.promises.rename(temporaryPath, filePath); - } finally { - await fs.promises.rm(temporaryPath, { force: true }); - } -} - -async function removeFileAtomicallyIfUnchanged( - filePath: string, - expectedBytes: Buffer, -): Promise { - let currentBytes: Buffer; - try { - currentBytes = fs.readFileSync(filePath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be completed.', - ); - } - throw error; - } - if (!currentBytes.equals(expectedBytes)) { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be completed.', - ); - } - await fs.promises.unlink(filePath); -} - -async function writeNewFileAtomically(filePath: string, bytes: Buffer): Promise { - fs.mkdirSync(path.dirname(filePath), { recursive: true }); - const temporaryPath = temporaryPathFor(filePath); - try { - await fs.promises.writeFile(temporaryPath, bytes, { flag: 'wx', mode: 0o600 }); - try { - await fs.promises.link(temporaryPath, filePath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'EEXIST') { - throw new FlowDiagramOperationError( - 'FILE_EXISTS', - 'The requested output already exists; no file was overwritten.', - ); - } - throw error; - } - } finally { - await fs.promises.rm(temporaryPath, { force: true }); - } -} - function cloneScene(scene: ExcalidrawScene): ExcalidrawScene { return structuredClone(scene); } diff --git a/src/main/ipc-handlers.ts b/src/main/ipc-handlers.ts index a5e71621..ff8b18c4 100644 --- a/src/main/ipc-handlers.ts +++ b/src/main/ipc-handlers.ts @@ -85,6 +85,7 @@ import { conversationRunStreams } from './conversation-run-stream-runtime'; import { deleteConversation, deleteProject } from './conversation-deletion'; import { conversationWorkingStateLifecycle } from './deepagent/conversation-working-state'; import { registerFlowDiagramExportResponseHandler } from './flow-diagram/flow-diagram-export-adapter'; +import { createFlowDiagramDocumentStore } from './flow-diagram/flow-diagram-document-store'; import { createConversationWorkingStateCompactionRunner, findConversationWorkingStateMaintenanceBlocker, @@ -1247,12 +1248,12 @@ export function registerIpcHandlers() { } }); - typedHandle('fs:writeFile', async (_, rootPath, filePath, content, expectedContent) => { + typedHandle('fs:writeFile', async (_, rootPath, filePath, content) => { if (!isRegisteredProjectRoot(rootPath)) { return { ok: false, error: { code: 'EACCES', message: 'rootPath is not a registered project root' } }; } try { - await writeFile(rootPath, filePath, content, expectedContent); + await writeFile(rootPath, filePath, content); notifyFileChange(filePath); return { ok: true }; } catch (err: any) { @@ -1260,6 +1261,17 @@ export function registerIpcHandlers() { } }); + typedHandle('flow-diagram:save-document', async (_, rootPath, filePath, content, expectedContent) => { + if (!isRegisteredProjectRoot(rootPath)) { + return { ok: false, error: { code: 'EACCES', message: 'rootPath is not a registered project root' } }; + } + const documentStore = createFlowDiagramDocumentStore({ + projectPath: rootPath, + notifyFileChange, + }); + return documentStore.saveDocument(filePath, content, expectedContent); + }); + typedHandle('fs:createFile', async (_, rootPath, filePath) => { if (!isRegisteredProjectRoot(rootPath)) { return { ok: false, error: { code: 'EACCES', message: 'rootPath is not a registered project root' } }; diff --git a/src/main/services/file-system.test.ts b/src/main/services/file-system.test.ts index a1f15e5e..00cdc094 100644 --- a/src/main/services/file-system.test.ts +++ b/src/main/services/file-system.test.ts @@ -138,14 +138,6 @@ describe('FileSystemService', () => { expect(fs.readFileSync(filePath, 'utf-8')).toBe('hello'); }); - it('rejects a stale compare-and-swap save without overwriting disk content', async () => { - const filePath = path.join(tmpDir, 'hello.ts'); - await expect(writeFile(tmpDir, filePath, 'stale edit', 'older content')).rejects.toMatchObject({ - code: 'ECONFLICT', - }); - expect(fs.readFileSync(filePath, 'utf-8')).toBe('console.log("hello");'); - }); - it('rejects path traversal', async () => { await expect(writeFile(tmpDir, '/etc/passwd', 'hack')).rejects.toThrow(); }); diff --git a/src/main/services/file-system.ts b/src/main/services/file-system.ts index af6d694f..0ffac9d6 100644 --- a/src/main/services/file-system.ts +++ b/src/main/services/file-system.ts @@ -118,22 +118,12 @@ export async function writeFile( rootPath: string, filePath: string, content: string, - expectedContent?: string, ): Promise { await runProjectFileMutation(rootPath, async () => { const resolved = resolveProjectFile(rootPath, filePath); if (isProtectedPath(resolved)) { throw new Error(`Cannot write to protected path: ${filePath}`); } - if (expectedContent !== undefined) { - const currentContent = await fsp.readFile(resolved, 'utf-8'); - if (currentContent !== expectedContent) { - throw Object.assign( - new Error('File changed on disk before this save could be applied.'), - { code: 'ECONFLICT' }, - ); - } - } await fsp.writeFile(resolved, content, 'utf-8'); }); } diff --git a/src/preload/index.ts b/src/preload/index.ts index c3ae9747..71467f43 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -161,6 +161,8 @@ const api = { }, }, flowDiagram: { + saveDocument: (rootPath: string, filePath: string, content: string, expectedContent: string | null) => + typedInvoke('flow-diagram:save-document', rootPath, filePath, content, expectedContent), onExportRequest: (callback: (request: FlowDiagramExportRequest) => void) => { const listener = (_event: IpcRendererEvent, request: FlowDiagramExportRequest) => callback(request); ipcRenderer.on(FLOW_DIAGRAM_EXPORT_REQUEST_CHANNEL, listener); @@ -180,8 +182,8 @@ const api = { typedInvoke('fs:getFileInfo', rootPath, filePath), onDirectoryChange: (callback: (data: IpcEventPayload<'fs:directoryChange'>) => void) => typedOn('fs:directoryChange', callback), - writeFile: (rootPath: string, filePath: string, content: string, expectedContent?: string) => - typedInvoke('fs:writeFile', rootPath, filePath, content, expectedContent), + writeFile: (rootPath: string, filePath: string, content: string) => + typedInvoke('fs:writeFile', rootPath, filePath, content), createFile: (rootPath: string, filePath: string) => typedInvoke('fs:createFile', rootPath, filePath), createDirectory: (rootPath: string, dirPath: string) => diff --git a/src/renderer/src/components/FilePanel/FlowDiagramEditor.tsx b/src/renderer/src/components/FilePanel/FlowDiagramEditor.tsx index 8b1abd36..85ac1429 100644 --- a/src/renderer/src/components/FilePanel/FlowDiagramEditor.tsx +++ b/src/renderer/src/components/FilePanel/FlowDiagramEditor.tsx @@ -73,19 +73,30 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl return saveQueueRef.current; } - const expectedDiskContent = queuedDiskContentRef.current ?? lastDiskContentRef.current ?? undefined; + const expectedDiskContent = queuedDiskContentRef.current ?? lastDiskContentRef.current ?? null; lastQueuedContentRef.current = contentToSave; queuedDiskContentRef.current = contentToSave; const operation = saveQueueRef.current.then(async () => { setSaveState('saving'); try { - const result = await window.electronAPI.fs.writeFile( + const result = await window.electronAPI.flowDiagram.saveDocument( rootPath, filePath, contentToSave, expectedDiskContent, ); if (!result.ok) { + if (result.error.code === 'SOURCE_CHANGED') { + // The document changed externally: preserve this attempt for the + // conflict banner instead of silently overwriting either side. + if (!conflictedContentRef.current) { + conflictedContentRef.current = contentToSave; + setConflictedContent(contentToSave); + } + setTabDirty(filePath, true); + setSaveState('dirty'); + return false; + } console.error('[FlowDiagramEditor] Save failed:', result.error.message); setSaveState('error'); return false; diff --git a/src/shared/flow-diagrams.ts b/src/shared/flow-diagrams.ts index 4f7ea97f..7bb344e6 100644 --- a/src/shared/flow-diagrams.ts +++ b/src/shared/flow-diagrams.ts @@ -21,3 +21,14 @@ export type FlowDiagramExportResponse = export const FLOW_DIAGRAM_EXPORT_REQUEST_CHANNEL = 'flow-diagram:export-request'; export const FLOW_DIAGRAM_EXPORT_RESPONSE_CHANNEL = 'flow-diagram:export-response'; + +/** + * Flow Diagram 文档存储的保存结果。SOURCE_CHANGED 冲突附带当前磁盘内容， + * 供编辑器直接呈现冲突而无需二次读取。 + */ +export type FlowDiagramDocumentSaveResult = + | { ok: true } + | { + ok: false; + error: { code: string; message: string; currentContent?: string | null }; + }; diff --git a/src/shared/ipc-contract.ts b/src/shared/ipc-contract.ts index 3782bf0f..232d9fa8 100644 --- a/src/shared/ipc-contract.ts +++ b/src/shared/ipc-contract.ts @@ -32,6 +32,7 @@ import type { } from './conversations'; import type { ContextAggregate } from './context'; import type { BinaryFileInfo, DirectoryEntry, FileContent, FileError, FileInfo } from './filesystem'; +import type { FlowDiagramDocumentSaveResult } from './flow-diagrams'; import type { KnowledgeEntryCreateInput, KnowledgeEntrySearchOptions, @@ -257,9 +258,14 @@ export interface IpcInvokeContract { }; 'fs:getFileInfo': { args: [rootPath: string, filePath: string]; result: FsData }; 'fs:writeFile': { - args: [rootPath: string, filePath: string, content: string, expectedContent?: string]; + args: [rootPath: string, filePath: string, content: string]; result: FsAck; }; + // Flow Diagram 文档写路径走文档存储：原子 CAS + 场景校验 + 冲突结构化返回。 + 'flow-diagram:save-document': { + args: [rootPath: string, filePath: string, content: string, expectedContent: string | null]; + result: FlowDiagramDocumentSaveResult; + }; 'fs:createFile': { args: [rootPath: string, filePath: string]; result: FsAck }; 'fs:createDirectory': { args: [rootPath: string, dirPath: string]; result: FsAck }; 'fs:renameEntry': { args: [rootPath: string, oldPath: string, newName: string]; result: FsAck }; @@ -442,6 +448,7 @@ export const IPC_INVOKE_CHANNELS = [ 'fs:readFile', 'fs:getFileInfo', 'fs:writeFile', + 'flow-diagram:save-document', 'fs:createFile', 'fs:createDirectory', 'fs:renameEntry', From ca37fb19426346d158d080715635c5ba6e269498 Mon Sep 17 00:00:00 2001 From: suntianc Date: Sun, 26 Jul 2026 10:36:38 -0700 Subject: [PATCH 4/8] =?UTF-8?q?test(renderer):=20FilePanel=20=E6=B5=81?= =?UTF-8?q?=E7=A8=8B=E5=9B=BE=20autosave=20=E7=94=A8=E4=BE=8B=E8=BF=81?= =?UTF-8?q?=E8=87=B3=E6=96=87=E6=A1=A3=E5=AD=98=E5=82=A8=E9=80=9A=E9=81=93?= =?UTF-8?q?=20(#200)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 编辑器写路径已改走 flow-diagram:save-document，参数形状与原 fs.writeFile CAS 版本一致，mock 复用、断言不变。 Co-Authored-By: Claude Fable 5 --- src/renderer/src/components/FilePanel/FilePanel.test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/renderer/src/components/FilePanel/FilePanel.test.tsx b/src/renderer/src/components/FilePanel/FilePanel.test.tsx index a170adeb..63063f3d 100644 --- a/src/renderer/src/components/FilePanel/FilePanel.test.tsx +++ b/src/renderer/src/components/FilePanel/FilePanel.test.tsx @@ -144,6 +144,9 @@ beforeEach(() => { (window as unknown as { electronAPI: unknown }).electronAPI = { store: { get: vi.fn().mockResolvedValue(false) }, + // 流程图 autosave 走文档存储通道 (#200)；参数形状与 fs.writeFile 的 + // CAS 版本一致（rootPath, filePath, content, expectedContent），共用断言。 + flowDiagram: { saveDocument: writeFile }, fs: { readDirectory: vi.fn().mockResolvedValue({ ok: true, data: [] }), readFile, From a070f095297ecf6a84b80a44f0a8b2e1841813a4 Mon Sep 17 00:00:00 2001 From: suntianc Date: Sun, 26 Jul 2026 10:36:38 -0700 Subject: [PATCH 5/8] =?UTF-8?q?refactor(deepagent):=20=E6=8A=BD=E5=8F=96?= =?UTF-8?q?=20createDelegatedRuntimeAdapter=20=E8=A1=A5=E5=85=A8=20delegat?= =?UTF-8?q?ed-*=20=E6=A8=A1=E5=9D=97=E6=97=8F=20(#201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ~130 行 delegatedRuntimeAdapter 闭包从装配文件抽为显式工厂，父→子 继承契约收敛为 DelegatedParentContext 窄接口：审批模式（ADR-0063）、 工具 scope 收窄基线（ADR-0062）、Conversation Skill Snapshot 传播、 模型选择输入、文件系统限域与会话标识 - coordinator ↔ adapter 的闭包延迟绑定改为 resolveApprovalCoordinator 延迟解析注入；韧性中间件工厂由装配层传入，无运行时循环依赖 - 图构建 / MCP 装载 / 内建工具 / 装配为可注入执行依赖，生产默认真实实现 - 新增工厂接缝单测：scope 只收窄、快照排除 MCP、Skill 预载 ⊆ 快照、 每次运行独立 graph/checkpointer、结构化结果解析与中断失败路径 - 为 delegated-run-failure 失败分类补专属单测（原无测试） - 行为零变更；parallel-delegated-runtime 与 delegated contract 集成测试全绿 Co-Authored-By: Claude Fable 5 --- .../deepagent/delegated-run-failure.test.ts | 25 ++ .../delegated-runtime-adapter.test.ts | 248 +++++++++++++ .../deepagent/delegated-runtime-adapter.ts | 340 ++++++++++++++++++ src/main/deepagent/runtime.ts | 250 ++----------- 4 files changed, 635 insertions(+), 228 deletions(-) create mode 100644 src/main/deepagent/delegated-run-failure.test.ts create mode 100644 src/main/deepagent/delegated-runtime-adapter.test.ts create mode 100644 src/main/deepagent/delegated-runtime-adapter.ts diff --git a/src/main/deepagent/delegated-run-failure.test.ts b/src/main/deepagent/delegated-run-failure.test.ts new file mode 100644 index 00000000..ffed627f --- /dev/null +++ b/src/main/deepagent/delegated-run-failure.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from 'vitest'; +import { classifyDelegatedRunFailure } from './delegated-run-failure'; + +describe('classifyDelegatedRunFailure', () => { + it.each([ + ['Request timed out after 60s', 'TIMEOUT'], + ['fetch failed: socket hang up', 'NETWORK'], + ['terminated', 'NETWORK'], + ['429 rate limit exceeded', 'RATE_LIMIT'], + ['permission denied for tool bash', 'PERMISSION_DENIED'], + ['Unauthorized: invalid API key', 'PERMISSION_DENIED'], + ['The operation was aborted', 'INTERRUPTED'], + ['Delegated tool approval is not available for this run', 'DELEGATED_RUNTIME_FAILED'], + ['something unexpected exploded', 'DELEGATED_RUNTIME_FAILED'], + ])('classifies "%s" as %s', (message, code) => { + expect(classifyDelegatedRunFailure(new Error(message))).toEqual({ code, message }); + }); + + it('stringifies non-Error failures and keeps the default code', () => { + expect(classifyDelegatedRunFailure('boom')).toEqual({ + code: 'DELEGATED_RUNTIME_FAILED', + message: 'boom', + }); + }); +}); diff --git a/src/main/deepagent/delegated-runtime-adapter.test.ts b/src/main/deepagent/delegated-runtime-adapter.test.ts new file mode 100644 index 00000000..06d6487a --- /dev/null +++ b/src/main/deepagent/delegated-runtime-adapter.test.ts @@ -0,0 +1,248 @@ +import os from 'os'; +import path from 'path'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('electron', () => ({ + app: { + getPath: vi.fn(() => path.join(os.tmpdir(), 'cdf-delegated-adapter-test-user-data')), + }, +})); + +vi.mock('../database', () => ({ + default: { + prepare: vi.fn(() => ({ + all: vi.fn(() => []), + get: vi.fn(), + run: vi.fn(), + })), + }, +})); + +vi.mock('../store', () => ({ + default: { + get: vi.fn(), + set: vi.fn(), + }, +})); + +vi.mock('./conversation-working-state', () => ({ + DEEPAGENT_CHECKPOINT_NAMESPACE: '', +})); + +vi.mock('./runtime-assembly', () => ({ + assembleDeepAgentRuntime: vi.fn(), + extractPathMentionContext: vi.fn(() => [] as string[]), +})); + +import { + createDelegatedRuntimeAdapter, + type CreateDelegatedRuntimeAdapterOptions, + type DelegatedParentContext, +} from './delegated-runtime-adapter'; +import type { DelegatedRuntimeRequest } from './delegated-agent-run-coordinator'; +import type { DelegatedAgentConfigurationSnapshot } from './delegated-agent-configuration-snapshot'; +import type { CatalogAgent } from '../agent-catalog'; +import type { MCPServer } from '../../shared/types'; + +function catalogAgent(config: Record | null): CatalogAgent { + return { + id: 'agent-1', + role: 'custom', + name: 'Researcher', + slug: 'researcher', + description: null, + provider_id: null, + system_prompt: 'child prompt', + config, + mcpServerExclusionIds: [], + skillNames: [], + created_at: 0, + updated_at: 0, + }; +} + +function snapshotFor( + config: Record | null, + overrides: Partial = {}, +): DelegatedAgentConfigurationSnapshot { + return { + target: catalogAgent(config), + mcpServerExclusionIds: [], + globalSkillPreloadRefs: [], + ...overrides, + }; +} + +function mcpServer(id: string): MCPServer { + return { id, name: id } as unknown as MCPServer; +} + +describe('createDelegatedRuntimeAdapter', () => { + const projectPath = path.join(os.tmpdir(), 'cdf-delegated-adapter-project'); + let parentContext: DelegatedParentContext; + let invokeMock: ReturnType; + let createAgentGraphMock: ReturnType; + let loadMcpToolsMock: ReturnType; + let assembleRuntimeMock: ReturnType; + let resolveApprovalCoordinatorMock: ReturnType; + let options: CreateDelegatedRuntimeAdapterOptions; + + beforeEach(() => { + invokeMock = vi.fn(async () => ({ + structuredResponse: { status: 'success', artifacts: [], summary: 'done' }, + })); + createAgentGraphMock = vi.fn(() => ({ invoke: invokeMock })); + loadMcpToolsMock = vi.fn(async () => ({ client: null, tools: [{ name: 'mcp_search' }] })); + assembleRuntimeMock = vi.fn(async () => ({ + model: { id: 'child-model' }, + provider: { id: 'openai' }, + permissions: [], + skillsRuntime: { skills: [], prompt: '', warnings: [], attributions: [] }, + systemPrompt: 'assembled child prompt', + assemblyWarnings: [], + })); + resolveApprovalCoordinatorMock = vi.fn(() => ({ + runToolAction: vi.fn(async ({ execute }: { execute: () => Promise }) => execute()), + })); + parentContext = { + approvalMode: 'strict', + parentBuiltInToolNames: ['bash', 'fetch'], + parentMcpServerIds: ['mcp-a', 'mcp-b'], + allMcpServers: [mcpServer('mcp-a'), mcpServer('mcp-b')], + skillSnapshot: [], + providerId: 'openai', + parentOverrides: { allowedTools: ['bash'] }, + project: { name: 'Demo', path: projectPath, scene: 'general' }, + agentFileRoots: [projectPath], + sessionId: 'session-1', + } as DelegatedParentContext; + options = { + resolveApprovalCoordinator: + resolveApprovalCoordinatorMock as unknown as CreateDelegatedRuntimeAdapterOptions['resolveApprovalCoordinator'], + createResilienceMiddleware: vi.fn(() => []) as unknown as + CreateDelegatedRuntimeAdapterOptions['createResilienceMiddleware'], + dependencies: { + assembleRuntime: assembleRuntimeMock as unknown as never, + createAgentGraph: createAgentGraphMock as unknown as never, + loadMcpTools: loadMcpToolsMock as unknown as never, + createBuiltInTools: vi.fn(() => [ + { name: 'bash' }, + { name: 'fetch' }, + { name: 'write_file' }, + ]) as unknown as never, + loadRegistryTools: vi.fn(() => []) as unknown as never, + }, + }; + }); + + function requestFor(snapshot: DelegatedAgentConfigurationSnapshot | undefined): DelegatedRuntimeRequest { + return { + delegatedRunId: 'delegated-1', + parentAgentRunId: 'run-1', + targetAgentSlug: 'researcher', + goal: 'summarize the repo', + input: { messages: [] }, + configurationSnapshot: snapshot, + } as unknown as DelegatedRuntimeRequest; + } + + it('rejects a request without a configuration snapshot', async () => { + const adapter = createDelegatedRuntimeAdapter(parentContext, options); + + await expect(adapter.run(requestFor(undefined))) + .rejects.toThrow('Delegated target Agent not found: researcher'); + expect(createAgentGraphMock).not.toHaveBeenCalled(); + }); + + it('narrows the child tool scope to the parent baseline (ADR-0062)', async () => { + const adapter = createDelegatedRuntimeAdapter(parentContext, options); + + await adapter.run(requestFor(snapshotFor({ + toolScope: { mode: 'narrow', builtInTools: ['bash', 'write_file'], mcpServerIds: [] }, + }))); + + const graphConfig = createAgentGraphMock.mock.calls[0][0] as { tools: Array<{ name: string }> }; + const toolNames = graphConfig.tools.map((tool) => tool.name); + // write_file 不在父基线内、fetch 未被选中——子集合只能收窄不能扩大。 + expect(toolNames).toContain('bash'); + expect(toolNames).not.toContain('write_file'); + expect(toolNames).not.toContain('fetch'); + expect(loadMcpToolsMock).toHaveBeenCalledWith('agent-1', [], parentContext.allMcpServers); + }); + + it('filters snapshot-excluded MCP servers from the inherited child scope', async () => { + const adapter = createDelegatedRuntimeAdapter(parentContext, options); + + await adapter.run(requestFor(snapshotFor(null, { mcpServerExclusionIds: ['mcp-b'] }))); + + const [, childServers] = loadMcpToolsMock.mock.calls[0] as [string, MCPServer[], MCPServer[]]; + expect(childServers.map((server) => server.id)).toEqual(['mcp-a']); + }); + + it('propagates the Conversation Skill Snapshot and preload refs into child assembly', async () => { + const skillSnapshot = [{ name: 'paper-search' }] as unknown as + DelegatedParentContext['skillSnapshot']; + const adapter = createDelegatedRuntimeAdapter({ ...parentContext, skillSnapshot }, options); + + await adapter.run(requestFor(snapshotFor(null, { + globalSkillPreloadRefs: ['built-in:paper-search'], + }))); + + const [target, providerId, project, skillNames, , , , passedSnapshot] = + assembleRuntimeMock.mock.calls[0]; + expect(target.id).toBe('agent-1'); + expect(providerId).toBe('openai'); + expect(project.path).toBe(projectPath); + expect(skillNames).toEqual(['built-in:paper-search']); + expect(passedSnapshot).toBe(skillSnapshot); + }); + + it('builds an isolated graph per run and resolves the coordinator lazily (ADR-0061)', async () => { + const adapter = createDelegatedRuntimeAdapter(parentContext, options); + expect(resolveApprovalCoordinatorMock).not.toHaveBeenCalled(); + + await adapter.run(requestFor(snapshotFor(null))); + await adapter.run(requestFor(snapshotFor(null))); + + expect(createAgentGraphMock).toHaveBeenCalledTimes(2); + const [firstConfig, secondConfig] = createAgentGraphMock.mock.calls.map( + (call) => call[0] as { checkpointer: unknown; backend: unknown }, + ); + expect(firstConfig.checkpointer).not.toBe(secondConfig.checkpointer); + expect(firstConfig.backend).not.toBe(secondConfig.backend); + expect(resolveApprovalCoordinatorMock).toHaveBeenCalledTimes(2); + }); + + it('returns the structured child result when it matches the delegated contract', async () => { + const adapter = createDelegatedRuntimeAdapter(parentContext, options); + + await expect(adapter.run(requestFor(snapshotFor(null)))).resolves.toEqual({ + status: 'success', + artifacts: [], + summary: 'done', + }); + }); + + it('falls back to a truncated last-message summary when structured output is invalid', async () => { + invokeMock.mockResolvedValueOnce({ + structuredResponse: { nope: true }, + messages: [{ content: 'x'.repeat(3000) }], + }); + const adapter = createDelegatedRuntimeAdapter(parentContext, options); + + const result = await adapter.run(requestFor(snapshotFor(null))); + + expect(result.status).toBe('success'); + expect(result.summary).toHaveLength(2000); + }); + + it('surfaces unresolved child interrupts as a failure instead of hanging approval', async () => { + invokeMock.mockResolvedValueOnce({ + __interrupt__: [{ value: 'approval requested' }], + }); + const adapter = createDelegatedRuntimeAdapter(parentContext, options); + + await expect(adapter.run(requestFor(snapshotFor(null)))) + .rejects.toThrow('Delegated tool approval is not available for this run'); + }); +}); diff --git a/src/main/deepagent/delegated-runtime-adapter.ts b/src/main/deepagent/delegated-runtime-adapter.ts new file mode 100644 index 00000000..43346ad8 --- /dev/null +++ b/src/main/deepagent/delegated-runtime-adapter.ts @@ -0,0 +1,340 @@ +import crypto from 'crypto'; +import { MemorySaver } from '@langchain/langgraph'; +import { createMiddleware } from 'langchain'; +import { createDeepAgent, CompositeBackend, StateBackend } from 'deepagents'; +import log from '../logger'; +import { assembleDeepAgentRuntime, extractPathMentionContext } from './runtime-assembly'; +import { + createBuiltInTools, + getRuntimeToolNames, + loadMcpTools, + loadRegistryTools, + resolveInterruptOn, +} from './shared-infra'; +import { readAgentToolScope, selectDelegatedToolScope } from './agent-tool-scope'; +import { resolveDelegatedModelOverrides } from './delegated-model-selection'; +import { ProjectConfinedFilesystemBackend } from './project-confined-backend'; +import { DEEPAGENT_CHECKPOINT_NAMESPACE } from './conversation-working-state'; +import { subagentStepStorage } from './subagent-step-storage'; +import type { + DelegatedAgentRunCoordinator, + DelegatedRuntimeAdapter, + DelegatedRuntimeRequest, +} from './delegated-agent-run-coordinator'; +import { + DELEGATED_TASK_RESULT_SCHEMA, + type ApprovalMode, + type ChatRuntimeOverrides, + type DelegatedTaskResult, + type MCPServer, + type ProjectScene, +} from '../../shared/types'; +import type { ConversationSkillSnapshotEntry } from '../../shared/skills'; + +/** + * Delegated Agent Run 的隔离运行时构造（ADR-0061 实现本体）。 + * + * 父运行向子运行的继承契约收敛为 DelegatedParentContext 的显式字段： + * 审批模式（ADR-0063）、工具 scope 收窄基线（ADR-0062）、Conversation + * Skill Snapshot 传播、模型选择输入与文件系统限域。coordinator 持有本 + * adapter，而 adapter 仅在执行期经延迟解析取回审批门控入口——原先靠 + * 装配闭包捕获形成的双向绑定改为显式注入。 + */ + +export interface DelegatedParentContext { + /** ADR-0063: Delegated Runs inherit the one Conversation approval mode. */ + approvalMode: ApprovalMode; + /** ADR-0062 收窄基线：父运行的内建工具名集合。 */ + parentBuiltInToolNames: string[]; + /** ADR-0062 收窄基线：父运行可见的 MCP server 标识。 */ + parentMcpServerIds: string[]; + /** 全量已连接 MCP 目录；子候选集 = 全量 - 配置快照排除项。 */ + allMcpServers: MCPServer[]; + /** Conversation Skill Snapshot：子运行 Skill 预载只能从中选择。 */ + skillSnapshot: readonly ConversationSkillSnapshotEntry[] | null; + /** 父 provider 标识，作为委派模型选择的回退输入。 */ + providerId: string; + /** 父模型 overrides（含 allowedTools 韧性约束）。 */ + parentOverrides: ChatRuntimeOverrides | undefined; + project: { name: string; path: string; scene?: ProjectScene }; + /** 文件系统限域根（含 Project 根与既定附加根）。 */ + agentFileRoots: string[]; + sessionId: string; +} + +type DeepAgentMiddleware = NonNullable< + NonNullable[0]>['middleware'] +>; + +export type DelegatedResilienceMiddlewareFactory = ( + ...allowedToolSets: Array +) => DeepAgentMiddleware; + +export interface DelegatedRuntimeExecutionDependencies { + assembleRuntime: typeof assembleDeepAgentRuntime; + createAgentGraph: typeof createDeepAgent; + loadMcpTools: typeof loadMcpTools; + createBuiltInTools: typeof createBuiltInTools; + loadRegistryTools: typeof loadRegistryTools; +} + +export interface CreateDelegatedRuntimeAdapterOptions { + /** + * 审批门控入口的延迟解析：coordinator 构造时持有 adapter，adapter 仅在 + * run() 执行期取回 coordinator，二者不再互相捕获。 + */ + resolveApprovalCoordinator: () => DelegatedAgentRunCoordinator; + /** 子运行韧性中间件工厂（工具/模型重试与失败观察归属装配层）。 */ + createResilienceMiddleware: DelegatedResilienceMiddlewareFactory; + dependencies?: Partial; +} + +function createDelegatedToolApprovalMiddleware( + coordinator: DelegatedAgentRunCoordinator, + delegatedRunId: string, + gatedToolNames: Set, +) { + return createMiddleware({ + name: 'DelegatedToolApprovalMiddleware', + wrapToolCall: async (request, handler) => { + const runtimeTool = request as { tool?: { name?: string } }; + const toolName = request.toolCall?.name || runtimeTool.tool?.name || 'unknown'; + const actionId = request.toolCall?.id || crypto.randomUUID(); + return coordinator.runToolAction({ + delegatedRunId, + action: { + id: actionId, + name: toolName, + args: (request.toolCall as { args?: unknown })?.args, + }, + requiresApproval: gatedToolNames.has(toolName), + execute: async () => handler(request), + }); + }, + }); +} + +function createDelegatedProgressCallbacks(request: DelegatedRuntimeRequest) { + const onStep = request.onStep; + if (!onStep) return undefined; + + let tokenBuffer: string[] = []; + const emitText = (text: string) => { + if (!text) return; + onStep({ + type: 'text_chunk', + ts: Date.now(), + content: text, + delegatedRunId: request.delegatedRunId, + }); + }; + + return [{ + handleLLMStart() { + tokenBuffer = []; + }, + handleLLMNewToken(token: string) { + if (token) tokenBuffer.push(token); + }, + handleLLMEnd(output: unknown) { + const value = output as { + generations?: Array>; + }; + const generation = value.generations?.[0]?.[0]; + const toolCalls = generation?.message?.additional_kwargs?.tool_calls + ?? generation?.message?.tool_calls; + const content = generation?.message?.content; + const hasToolCalls = (Array.isArray(toolCalls) && toolCalls.length > 0) + || (Array.isArray(content) && content.some((part) => ( + !!part && typeof part === 'object' && (part as { type?: unknown }).type === 'tool_use' + ))); + if (hasToolCalls) { + tokenBuffer = []; + return; + } + if (tokenBuffer.length > 0) { + for (const token of tokenBuffer) emitText(token); + tokenBuffer = []; + return; + } + if (typeof content === 'string') { + emitText(content); + } else if (Array.isArray(content)) { + for (const part of content) { + if (part && typeof part === 'object' && (part as { type?: unknown }).type === 'text') { + const text = (part as { text?: unknown }).text; + if (typeof text === 'string') emitText(text); + } + } + } else if (typeof generation?.text === 'string') { + emitText(generation.text); + } + }, + }]; +} + +export function createDelegatedRuntimeAdapter( + parentContext: DelegatedParentContext, + options: CreateDelegatedRuntimeAdapterOptions, +): DelegatedRuntimeAdapter { + const { + approvalMode, + parentBuiltInToolNames, + parentMcpServerIds, + allMcpServers, + skillSnapshot, + providerId, + parentOverrides, + project, + agentFileRoots, + sessionId, + } = parentContext; + const deps: DelegatedRuntimeExecutionDependencies = { + assembleRuntime: assembleDeepAgentRuntime, + createAgentGraph: createDeepAgent, + loadMcpTools, + createBuiltInTools, + loadRegistryTools, + ...options.dependencies, + }; + + return { + run: async (request) => { + const snapshot = request.configurationSnapshot; + if (!snapshot) { + throw new Error(`Delegated target Agent not found: ${request.targetAgentSlug}`); + } + const target = snapshot.target; + + // Every Delegated Agent Run owns fresh mutable execution state. Agent + // configuration is reused, but model/graph/backend/checkpoint/tools are not. + const childBackend = new CompositeBackend(new StateBackend(), { + "/": new ProjectConfinedFilesystemBackend({ + rootDir: "/", + virtualMode: false, + allowedRoots: agentFileRoots, + projectRoot: project.path, + }), + }); + const childBuiltInTools = deps.createBuiltInTools(project.path, sessionId); + try { + childBuiltInTools.push(...deps.loadRegistryTools()); + } catch (error) { + log.warn('[runtime] Failed to load delegated built-in tools from registry:', error); + } + const targetToolScope = readAgentToolScope(target.config); + const childScope = selectDelegatedToolScope({ + agentConfig: target.config, + parentBuiltInToolNames, + childBuiltInTools, + parentMcpServerIds, + childMcpServers: allMcpServers.filter( + (server) => !snapshot.mcpServerExclusionIds.includes(server.id), + ), + }); + const childMcpRuntime = await deps.loadMcpTools(target.id, childScope.mcpServers, allMcpServers); + const childSkillNames = snapshot.globalSkillPreloadRefs; + const childToolNames = getRuntimeToolNames([ + ...childMcpRuntime.tools, + ...childScope.builtInTools, + ]); + const childOverrides = resolveDelegatedModelOverrides({ + targetProviderId: target.provider_id, + targetConfig: target.config, + parentProviderId: providerId, + parentOverrides, + }); + const childAssembly = await deps.assembleRuntime( + target, + providerId, + project, + childSkillNames, + extractPathMentionContext(request.goal), + childToolNames, + childOverrides, + skillSnapshot, + ); + for (const warning of childAssembly.assemblyWarnings) { + log.warn('[runtime] Ignored invalid delegated Agent Skill runtime input:', warning); + } + + const childInterruptOn = resolveInterruptOn( + approvalMode, + getRuntimeToolNames(childMcpRuntime.tools), + ); + const gatedToolNames = new Set(Object.keys(childInterruptOn)); + const childAgent = deps.createAgentGraph({ + model: childAssembly.model, + backend: childBackend, + systemPrompt: childAssembly.systemPrompt || undefined, + permissions: childAssembly.permissions, + tools: [...childMcpRuntime.tools, ...childScope.builtInTools], + middleware: [ + createDelegatedToolApprovalMiddleware( + options.resolveApprovalCoordinator(), + request.delegatedRunId, + gatedToolNames, + ), + ...options.createResilienceMiddleware( + parentOverrides?.allowedTools, + targetToolScope.mode === 'narrow' + ? [ + ...(targetToolScope.builtInTools ?? []), + ...getRuntimeToolNames(childMcpRuntime.tools), + ] + : undefined, + ), + ], + responseFormat: DELEGATED_TASK_RESULT_SCHEMA as unknown as NonNullable< + NonNullable[0]>['responseFormat'] + >, + checkpointer: new MemorySaver(), + }); + const progressCallbacks = createDelegatedProgressCallbacks(request); + const invokeChild = () => childAgent.invoke( + request.input as Parameters[0], + { + signal: request.signal, + callbacks: progressCallbacks, + configurable: { + thread_id: request.delegatedRunId, + checkpoint_ns: DEEPAGENT_CHECKPOINT_NAMESPACE, + delegatedRunId: request.delegatedRunId, + }, + }, + ); + const childResult = await (request.onStep + ? subagentStepStorage.run({ onStep: request.onStep }, invokeChild) + : invokeChild()) as unknown as { + structuredResponse?: unknown; + messages?: Array<{ content?: unknown }>; + __interrupt__?: unknown; + interrupts?: unknown; + }; + const childInterrupts = childResult.__interrupt__ ?? childResult.interrupts; + if (Array.isArray(childInterrupts) && childInterrupts.length > 0) { + throw new Error('Delegated tool approval is not available for this run'); + } + const structured = DELEGATED_TASK_RESULT_SCHEMA.safeParse(childResult?.structuredResponse); + if (structured.success) return structured.data; + + const messages = Array.isArray(childResult?.messages) ? childResult.messages : []; + const lastMessage = messages[messages.length - 1]; + const content = typeof lastMessage?.content === 'string' + ? lastMessage.content + : JSON.stringify(lastMessage?.content ?? 'Task completed'); + return { + status: 'success', + artifacts: [], + summary: content.slice(0, 2_000), + } satisfies DelegatedTaskResult; + }, + }; +} diff --git a/src/main/deepagent/runtime.ts b/src/main/deepagent/runtime.ts index 524e74a8..9ec75a72 100644 --- a/src/main/deepagent/runtime.ts +++ b/src/main/deepagent/runtime.ts @@ -1,7 +1,7 @@ import fs from 'fs'; import path from 'path'; import type { SqliteSaver } from '@langchain/langgraph-checkpoint-sqlite'; -import { isGraphInterrupt, MemorySaver } from '@langchain/langgraph'; +import { isGraphInterrupt } from '@langchain/langgraph'; import { createMiddleware, modelRetryMiddleware, ToolMessage, toolRetryMiddleware } from 'langchain'; import db from '../database'; import log from '../logger'; @@ -35,16 +35,14 @@ import { DelegatedAgentRunRepository } from './delegated-agent-run-repository'; import { DelegatedAgentRunCoordinator, type DelegatedRuntimeAdapter, - type DelegatedRuntimeRequest, } from './delegated-agent-run-coordinator'; import type { DelegatedAgentRun, DelegatedTaskResult } from '../../shared/types'; import { createDelegatedSubagentAdapter } from './delegated-subagent-adapter'; +import { createDelegatedRuntimeAdapter } from './delegated-runtime-adapter'; import { captureDelegatedAgentConfigurationSnapshot, type DelegatedAgentConfigurationSnapshot, } from './delegated-agent-configuration-snapshot'; -import { readAgentToolScope, selectDelegatedToolScope } from './agent-tool-scope'; -import { resolveDelegatedModelOverrides } from './delegated-model-selection'; import { conversationWorkingStateLifecycle, DEEPAGENT_CHECKPOINT_NAMESPACE } from './conversation-working-state'; import { ProjectConfinedFilesystemBackend, computeAgentFileRoots } from './project-confined-backend'; import { getOrCaptureConversationSystemContextSnapshot } from '../conversation-system-context-snapshot'; @@ -62,7 +60,7 @@ const WORKFLOW_RUN_PROMPT = ` - 每完成一个阶段并对照验收标准自检通过后，必须调用 advance_stage 工具提交结构化验收报告（逐条自评 + 产物清单 + 总结）；这会触发阶段门禁并推进到下一阶段。不要只用文字宣布"完成"就停下——不调用 advance_stage 工作流不会前进。 - 阶段内先用 create_task 一次性规划任务图并用 set_task_dependencies 标注依赖，再用 parallel_tasks 派子 Agent 执行，用 update_task_status / list_tasks 跟踪进度。 - 阶段游标由主进程在门禁通过后权威推进，你无需自行编号或跳跃阶段。`; -import { DELEGATED_TASK_RESULT_SCHEMA, type ApprovalMode, type ChatRuntimeOverrides } from '../../shared/types'; +import { type ApprovalMode, type ChatRuntimeOverrides } from '../../shared/types'; import { getCurrentStreamAccumulator } from './stream-accumulator'; import { subagentStepStorage } from './subagent-step-storage'; @@ -396,98 +394,6 @@ export function createSubagentResilienceMiddleware(...allowedToolSets: Array, -) { - return createMiddleware({ - name: 'DelegatedToolApprovalMiddleware', - wrapToolCall: async (request, handler) => { - const runtimeTool = request as { tool?: { name?: string } }; - const toolName = request.toolCall?.name || runtimeTool.tool?.name || 'unknown'; - const actionId = request.toolCall?.id || crypto.randomUUID(); - return coordinator.runToolAction({ - delegatedRunId, - action: { - id: actionId, - name: toolName, - args: (request.toolCall as { args?: unknown })?.args, - }, - requiresApproval: gatedToolNames.has(toolName), - execute: async () => handler(request), - }); - }, - }); -} - - -function createDelegatedProgressCallbacks(request: DelegatedRuntimeRequest) { - const onStep = request.onStep; - if (!onStep) return undefined; - - let tokenBuffer: string[] = []; - const emitText = (text: string) => { - if (!text) return; - onStep({ - type: 'text_chunk', - ts: Date.now(), - content: text, - delegatedRunId: request.delegatedRunId, - }); - }; - - return [{ - handleLLMStart() { - tokenBuffer = []; - }, - handleLLMNewToken(token: string) { - if (token) tokenBuffer.push(token); - }, - handleLLMEnd(output: unknown) { - const value = output as { - generations?: Array>; - }; - const generation = value.generations?.[0]?.[0]; - const toolCalls = generation?.message?.additional_kwargs?.tool_calls - ?? generation?.message?.tool_calls; - const content = generation?.message?.content; - const hasToolCalls = (Array.isArray(toolCalls) && toolCalls.length > 0) - || (Array.isArray(content) && content.some((part) => ( - !!part && typeof part === 'object' && (part as { type?: unknown }).type === 'tool_use' - ))); - if (hasToolCalls) { - tokenBuffer = []; - return; - } - if (tokenBuffer.length > 0) { - for (const token of tokenBuffer) emitText(token); - tokenBuffer = []; - return; - } - if (typeof content === 'string') { - emitText(content); - } else if (Array.isArray(content)) { - for (const part of content) { - if (part && typeof part === 'object' && (part as { type?: unknown }).type === 'text') { - const text = (part as { text?: unknown }).text; - if (typeof text === 'string') emitText(text); - } - } - } else if (typeof generation?.text === 'string') { - emitText(generation.text); - } - }, - }]; -} - export async function createDeepAgentRuntime( projectId: string, sessionId: string, @@ -625,138 +531,26 @@ async function buildDeepAgentRuntime( const delegatedTargets = new Map(); const delegatedRunRepository = new DelegatedAgentRunRepository(db); - const delegatedRuntimeAdapter: DelegatedRuntimeAdapter = { - run: async (request) => { - const snapshot = request.configurationSnapshot; - if (!snapshot) { - throw new Error(`Delegated target Agent not found: ${request.targetAgentSlug}`); - } - const target = snapshot.target; - - // Every Delegated Agent Run owns fresh mutable execution state. Agent - // configuration is reused, but model/graph/backend/checkpoint/tools are not. - const childBackend = new CompositeBackend(new StateBackend(), { - "/": new ProjectConfinedFilesystemBackend({ - rootDir: "/", - virtualMode: false, - allowedRoots: agentFileRoots, - projectRoot: project.path, - }), - }); - const childBuiltInTools = createBuiltInTools(project.path, sessionId); - try { - childBuiltInTools.push(...loadRegistryTools()); - } catch (error) { - log.warn('[runtime] Failed to load delegated built-in tools from registry:', error); - } - const targetToolScope = readAgentToolScope(target.config); - const childScope = selectDelegatedToolScope({ - agentConfig: target.config, - parentBuiltInToolNames: builtInToolNames, - childBuiltInTools, - parentMcpServerIds: mcpServers.map((server) => server.id), - childMcpServers: allMcpServers.filter( - (server) => !snapshot.mcpServerExclusionIds.includes(server.id), - ), - }); - const childMcpRuntime = await loadMcpTools(target.id, childScope.mcpServers, allMcpServers); - const childSkillNames = snapshot.globalSkillPreloadRefs; - const childToolNames = getRuntimeToolNames([ - ...childMcpRuntime.tools, - ...childScope.builtInTools, - ]); - const childOverrides = resolveDelegatedModelOverrides({ - targetProviderId: target.provider_id, - targetConfig: target.config, - parentProviderId: provider.id, - parentOverrides: overrides, - }); - const childAssembly = await assembleDeepAgentRuntime( - target, - provider.id, - project, - childSkillNames, - extractPathMentionContext(request.goal), - childToolNames, - childOverrides, - skillSnapshot, - ); - for (const warning of childAssembly.assemblyWarnings) { - log.warn('[runtime] Ignored invalid delegated Agent Skill runtime input:', warning); - } - - const childInterruptOn = resolveInterruptOn( - currentApprovalMode, - getRuntimeToolNames(childMcpRuntime.tools), - ); - const gatedToolNames = new Set(Object.keys(childInterruptOn)); - const childAgent = createDeepAgent({ - model: childAssembly.model, - backend: childBackend, - systemPrompt: childAssembly.systemPrompt || undefined, - permissions: childAssembly.permissions, - tools: [...childMcpRuntime.tools, ...childScope.builtInTools], - middleware: [ - createDelegatedToolApprovalMiddleware( - delegatedRunCoordinator, - request.delegatedRunId, - gatedToolNames, - ), - ...createSubagentResilienceMiddleware( - overrides?.allowedTools, - targetToolScope.mode === 'narrow' - ? [ - ...(targetToolScope.builtInTools ?? []), - ...getRuntimeToolNames(childMcpRuntime.tools), - ] - : undefined, - ), - ], - responseFormat: DELEGATED_TASK_RESULT_SCHEMA as unknown as NonNullable< - NonNullable[0]>['responseFormat'] - >, - checkpointer: new MemorySaver(), - }); - const progressCallbacks = createDelegatedProgressCallbacks(request); - const invokeChild = () => childAgent.invoke( - request.input as Parameters[0], - { - signal: request.signal, - callbacks: progressCallbacks, - configurable: { - thread_id: request.delegatedRunId, - checkpoint_ns: DEEPAGENT_CHECKPOINT_NAMESPACE, - delegatedRunId: request.delegatedRunId, - }, - }, - ); - const childResult = await (request.onStep - ? subagentStepStorage.run({ onStep: request.onStep }, invokeChild) - : invokeChild()) as unknown as { - structuredResponse?: unknown; - messages?: Array<{ content?: unknown }>; - __interrupt__?: unknown; - interrupts?: unknown; - }; - const childInterrupts = childResult.__interrupt__ ?? childResult.interrupts; - if (Array.isArray(childInterrupts) && childInterrupts.length > 0) { - throw new Error('Delegated tool approval is not available for this run'); - } - const structured = DELEGATED_TASK_RESULT_SCHEMA.safeParse(childResult?.structuredResponse); - if (structured.success) return structured.data; - - const messages = Array.isArray(childResult?.messages) ? childResult.messages : []; - const lastMessage = messages[messages.length - 1]; - const content = typeof lastMessage?.content === 'string' - ? lastMessage.content - : JSON.stringify(lastMessage?.content ?? 'Task completed'); - return { - status: 'success', - artifacts: [], - summary: content.slice(0, 2_000), - } satisfies DelegatedTaskResult; + // ADR-0061/0062/0063：隔离运行时构造收敛在 delegated-runtime-adapter， + // 父→子继承契约是显式的窄接口；coordinator 经延迟解析注入审批门控。 + const delegatedRuntimeAdapter: DelegatedRuntimeAdapter = createDelegatedRuntimeAdapter( + { + approvalMode: currentApprovalMode, + parentBuiltInToolNames: builtInToolNames, + parentMcpServerIds: mcpServers.map((server) => server.id), + allMcpServers, + skillSnapshot, + providerId: provider.id, + parentOverrides: overrides, + project, + agentFileRoots, + sessionId, }, - }; + { + resolveApprovalCoordinator: () => delegatedRunCoordinator, + createResilienceMiddleware: createSubagentResilienceMiddleware, + }, + ); const delegatedRunCoordinator = new DelegatedAgentRunCoordinator( delegatedRunRepository, delegatedRuntimeAdapter, From 272daae370b8f39089b19d6aa10c5c8d6616f53d Mon Sep 17 00:00:00 2001 From: suntianc Date: Sun, 26 Jul 2026 10:51:57 -0700 Subject: [PATCH 6/8] =?UTF-8?q?fix(review):=20=E8=90=BD=E5=AE=9E=20#200/#2?= =?UTF-8?q?01=20=E5=AE=A1=E6=9F=A5=E5=8F=91=E7=8E=B0=E7=9A=84=E5=87=A0?= =?UTF-8?q?=E5=A4=84=E7=BC=BA=E9=99=B7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 抽出 FLOW_DIAGRAM_SOURCE_CHANGED 常量取代两次裸 'SOURCE_CHANGED' 字面量 比较（消除 FlowDiagramEditor 与文档存储间的 Primitive Obsession） - 编辑器消费文档存储冲突返回的 currentContent：把磁盘 CAS 基线重定位到 外部当前内容，避免下一次保存重复触发同一个不可见冲突（落地此前未消费 的 Speculative Generality 字段） - 为 delegated-runtime-adapter 注入 resolveInterruptOn 执行依赖，补 ADR-0063 审批模式继承锚点测试：strict/agent_decides/bypass 三种父模式均断言子门控 由父模式解析，堵住"门控回归无法归因到结构改动还是模式"的缺口 Co-Authored-By: Claude Fable 5 --- .../delegated-runtime-adapter.test.ts | 20 +++++++++++++++++++ .../deepagent/delegated-runtime-adapter.ts | 5 ++++- .../flow-diagram-document-store.ts | 12 ++++++++--- .../FilePanel/FlowDiagramEditor.tsx | 10 +++++++--- src/shared/flow-diagrams.ts | 5 ++++- 5 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/main/deepagent/delegated-runtime-adapter.test.ts b/src/main/deepagent/delegated-runtime-adapter.test.ts index 06d6487a..494710a9 100644 --- a/src/main/deepagent/delegated-runtime-adapter.test.ts +++ b/src/main/deepagent/delegated-runtime-adapter.test.ts @@ -84,6 +84,7 @@ describe('createDelegatedRuntimeAdapter', () => { let createAgentGraphMock: ReturnType; let loadMcpToolsMock: ReturnType; let assembleRuntimeMock: ReturnType; + let resolveInterruptOnMock: ReturnType; let resolveApprovalCoordinatorMock: ReturnType; let options: CreateDelegatedRuntimeAdapterOptions; @@ -101,6 +102,7 @@ describe('createDelegatedRuntimeAdapter', () => { systemPrompt: 'assembled child prompt', assemblyWarnings: [], })); + resolveInterruptOnMock = vi.fn(() => ({ mcp_search: { allowedDecisions: ['approve', 'reject'] } })); resolveApprovalCoordinatorMock = vi.fn(() => ({ runToolAction: vi.fn(async ({ execute }: { execute: () => Promise }) => execute()), })); @@ -131,6 +133,7 @@ describe('createDelegatedRuntimeAdapter', () => { { name: 'write_file' }, ]) as unknown as never, loadRegistryTools: vi.fn(() => []) as unknown as never, + resolveInterruptOn: resolveInterruptOnMock as unknown as never, }, }; }); @@ -245,4 +248,21 @@ describe('createDelegatedRuntimeAdapter', () => { await expect(adapter.run(requestFor(snapshotFor(null)))) .rejects.toThrow('Delegated tool approval is not available for this run'); }); + + it.each(['strict', 'agent_decides', 'bypass'] as const)( + 'propagates the parent approval mode into the child approval gate (ADR-0063: %s)', + async (approvalMode) => { + const adapter = createDelegatedRuntimeAdapter({ ...parentContext, approvalMode }, options); + + await adapter.run(requestFor(snapshotFor(null))); + + // The adapter must derive the child gate from the parent's approval mode, + // not from a hardcoded default. ADR-0063: child inherits one mode. + expect(resolveInterruptOnMock).toHaveBeenCalledTimes(1); + expect(resolveInterruptOnMock).toHaveBeenCalledWith( + approvalMode, + expect.arrayContaining(['mcp_search']), + ); + }, + ); }); diff --git a/src/main/deepagent/delegated-runtime-adapter.ts b/src/main/deepagent/delegated-runtime-adapter.ts index 43346ad8..5d733b0e 100644 --- a/src/main/deepagent/delegated-runtime-adapter.ts +++ b/src/main/deepagent/delegated-runtime-adapter.ts @@ -76,6 +76,8 @@ export interface DelegatedRuntimeExecutionDependencies { loadMcpTools: typeof loadMcpTools; createBuiltInTools: typeof createBuiltInTools; loadRegistryTools: typeof loadRegistryTools; + /** ADR-0063: 解析子运行审批门控集合；生产默认用 shared-infra 真实现。 */ + resolveInterruptOn: typeof resolveInterruptOn; } export interface CreateDelegatedRuntimeAdapterOptions { @@ -202,6 +204,7 @@ export function createDelegatedRuntimeAdapter( loadMcpTools, createBuiltInTools, loadRegistryTools, + resolveInterruptOn, ...options.dependencies, }; @@ -265,7 +268,7 @@ export function createDelegatedRuntimeAdapter( log.warn('[runtime] Ignored invalid delegated Agent Skill runtime input:', warning); } - const childInterruptOn = resolveInterruptOn( + const childInterruptOn = deps.resolveInterruptOn( approvalMode, getRuntimeToolNames(childMcpRuntime.tools), ); diff --git a/src/main/flow-diagram/flow-diagram-document-store.ts b/src/main/flow-diagram/flow-diagram-document-store.ts index 6defeedb..266cd391 100644 --- a/src/main/flow-diagram/flow-diagram-document-store.ts +++ b/src/main/flow-diagram/flow-diagram-document-store.ts @@ -6,7 +6,10 @@ import { FlowDiagramSceneError, parseFlowDiagramScene, } from './flow-diagram-scene'; -import type { FlowDiagramDocumentSaveResult } from '../../shared/flow-diagrams'; +import { + FLOW_DIAGRAM_SOURCE_CHANGED, + type FlowDiagramDocumentSaveResult, +} from '../../shared/flow-diagrams'; /** * Flow Diagram 文档存储：`.excalidraw` 文档一致性的唯一拥有者。 @@ -281,7 +284,10 @@ export function createFlowDiagramDocumentStore( notify(target); return { ok: true } as const; } catch (error) { - if (error instanceof FlowDiagramOperationError && error.code === 'SOURCE_CHANGED') { + if ( + error instanceof FlowDiagramOperationError + && error.code === FLOW_DIAGRAM_SOURCE_CHANGED + ) { let currentContent: string | null = null; try { currentContent = fs.readFileSync(target, 'utf-8'); @@ -290,7 +296,7 @@ export function createFlowDiagramDocumentStore( } return { ok: false as const, - error: { code: 'SOURCE_CHANGED', message: error.message, currentContent }, + error: { code: FLOW_DIAGRAM_SOURCE_CHANGED, message: error.message, currentContent }, }; } return failureResult(error, 'WRITE_FAILED'); diff --git a/src/renderer/src/components/FilePanel/FlowDiagramEditor.tsx b/src/renderer/src/components/FilePanel/FlowDiagramEditor.tsx index 85ac1429..d78ef8cb 100644 --- a/src/renderer/src/components/FilePanel/FlowDiagramEditor.tsx +++ b/src/renderer/src/components/FilePanel/FlowDiagramEditor.tsx @@ -8,6 +8,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip import { reloadProjectFile } from '../../lib/openProjectFile'; import { registerProjectFileFlush } from '../../lib/projectFileFlush'; import { useFileStore } from '../../stores/fileStore'; +import { FLOW_DIAGRAM_SOURCE_CHANGED } from '@shared/flow-diagrams'; import { restoreFlowDiagram, serializeFlowDiagram, @@ -86,9 +87,12 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl expectedDiskContent, ); if (!result.ok) { - if (result.error.code === 'SOURCE_CHANGED') { - // The document changed externally: preserve this attempt for the - // conflict banner instead of silently overwriting either side. + if (result.error.code === FLOW_DIAGRAM_SOURCE_CHANGED) { + // The document changed externally: the store returns the current + // on-disk content so we can relink our CAS baseline without a + // second read, then surface the unsaved attempt as a conflict. + const currentContent = result.error.currentContent ?? null; + if (currentContent != null) lastDiskContentRef.current = currentContent; if (!conflictedContentRef.current) { conflictedContentRef.current = contentToSave; setConflictedContent(contentToSave); diff --git a/src/shared/flow-diagrams.ts b/src/shared/flow-diagrams.ts index 7bb344e6..526aa9a7 100644 --- a/src/shared/flow-diagrams.ts +++ b/src/shared/flow-diagrams.ts @@ -22,9 +22,12 @@ export type FlowDiagramExportResponse = export const FLOW_DIAGRAM_EXPORT_REQUEST_CHANNEL = 'flow-diagram:export-request'; export const FLOW_DIAGRAM_EXPORT_RESPONSE_CHANNEL = 'flow-diagram:export-response'; +/** 文档在保存基线之后被外部改写时的冲突码（编辑器与文档存储共享）。 */ +export const FLOW_DIAGRAM_SOURCE_CHANGED = 'SOURCE_CHANGED' as const; + /** * Flow Diagram 文档存储的保存结果。SOURCE_CHANGED 冲突附带当前磁盘内容， - * 供编辑器直接呈现冲突而无需二次读取。 + * 编辑器用它重定位下一次保存的 CAS 基线，无需二次读取。 */ export type FlowDiagramDocumentSaveResult = | { ok: true } From e7bfa058d551cab048808e96f8506605057349ab Mon Sep 17 00:00:00 2001 From: suntianc Date: Wed, 29 Jul 2026 05:36:25 -0700 Subject: [PATCH 7/8] =?UTF-8?q?fix(review):=20=E5=AE=8C=E6=88=90=20#238-#2?= =?UTF-8?q?44=20=E8=AF=84=E5=AE=A1=E4=BF=AE=E5=A4=8D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CONTEXT.md | 4 + src/main/cdf-file-protocol.test.ts | 102 +-- src/main/cdf-file-protocol.ts | 92 ++- src/main/commands/collectors/skill.test.ts | 21 + .../delegated-runtime-adapter.test.ts | 83 ++- .../deepagent/delegated-runtime-adapter.ts | 19 +- src/main/deepagent/runtime.ts | 6 +- .../skills-runtime/skill-sources.test.ts | 20 + .../flow-diagram-document-store.test.ts | 250 +++++++- .../flow-diagram-document-store.ts | 579 ++++++++++++++---- .../flow-diagram-service.integration.test.ts | 47 +- src/main/flow-diagram/flow-diagram-service.ts | 297 +++------ .../flow-diagram/manage-flow-diagram-tool.ts | 6 +- src/main/ipc-handlers.ts | 37 +- src/main/services/file-watcher.test.ts | 45 ++ src/main/services/file-watcher.ts | 24 + src/preload/index.test.ts | 28 + src/preload/index.ts | 22 +- .../AgentLibrary/AgentEditDialog.test.tsx | 6 +- .../AgentLibrary/SkillPreloadSection.test.tsx | 130 ++++ .../AgentLibrary/SkillPreloadSection.tsx | 99 +-- .../src/components/FilePanel/EditorPane.tsx | 11 +- .../components/FilePanel/FilePanel.test.tsx | 236 ++++++- .../src/components/FilePanel/FilePanel.tsx | 1 + .../FilePanel/FlowDiagramEditor.tsx | 432 +++++++------ src/renderer/src/lib/openProjectFile.ts | 21 +- src/renderer/src/stores/fileStore.ts | 16 +- src/shared/flow-diagrams.ts | 38 +- src/shared/ipc-contract.ts | 23 +- 29 files changed, 1963 insertions(+), 732 deletions(-) create mode 100644 src/main/services/file-watcher.test.ts create mode 100644 src/renderer/src/components/AgentLibrary/SkillPreloadSection.test.tsx diff --git a/CONTEXT.md b/CONTEXT.md index 91f91a11..de8c8c83 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -240,6 +240,10 @@ _Avoid_: main view, page, layout mode A user-visible, Project-owned Excalidraw document composed of independently editable shapes, text, and connectors that serves as the shared source for Agent generation and user editing. Subsequent Agent changes apply to the current document rather than regenerating a separate copy; the capability is available across Scenes. _Avoid_: Scientific Figure, generated image, flattened flowchart, hidden artifact, Research Scene tool +**Flow Diagram Document Version**: +An opaque identity of the exact current contents of an Editable Flow Diagram, used to detect whether a save or Agent edit is still based on that document. It is not ordered and does not represent history. +_Avoid_: Flow Diagram Revision, timestamp, save counter, version number, Project commit + **Flow Diagram Revision**: A durable snapshot of an Editable Flow Diagram captured immediately before an Agent modifies it and retained independently of the user's Project version control. It is available to Agent operations and automatic recovery without exposing version management or manual rollback controls to the user. _Avoid_: Project commit, copied backup file, Excalidraw undo entry, user-facing version history, full-Project snapshot diff --git a/src/main/cdf-file-protocol.test.ts b/src/main/cdf-file-protocol.test.ts index b06e26fc..48294172 100644 --- a/src/main/cdf-file-protocol.test.ts +++ b/src/main/cdf-file-protocol.test.ts @@ -7,7 +7,6 @@ import { pathToFileURL } from 'node:url'; import { contentTypeForPath, createCdfFileResponse, - isPathWithinRoots, parseRangeHeader, resolveCdfFilePath, } from './cdf-file-protocol'; @@ -36,40 +35,6 @@ describe('resolveCdfFilePath', () => { }); }); -describe('isPathWithinRoots', () => { - // Regression (#204 回归): under standard:true Chromium folds the first path - // segment into the URL host and lowercases it, so every macOS request arrives - // as /users/... while allowedRoots hold /Users/.... On a case-insensitive - // filesystem both address the same file, so containment must not be - // case-sensitive — otherwise every historical image/audio 403s. - it('accepts a host-casefolded path on a case-insensitive filesystem', () => { - expect(isPathWithinRoots( - '/users/suntc/Library/Application Support/cdf/default-project/.cdf/artifacts/images/a.png', - ['/Users/suntc/Library/Application Support/cdf'], - true, - )).toBe(true); - }); - - it('still rejects casing differences in case-sensitive mode', () => { - expect(isPathWithinRoots( - '/users/suntc/Library/Application Support/cdf/a.png', - ['/Users/suntc/Library/Application Support/cdf'], - false, - )).toBe(false); - }); - - it('still rejects escapes and unrelated roots regardless of casing mode', () => { - expect(isPathWithinRoots('/Users/suntc/other/a.png', ['/Users/suntc/Library'], true)).toBe(false); - expect(isPathWithinRoots('/Users/suntc/Library/../.ssh/id_rsa', ['/Users/suntc/Library'], true)).toBe(false); - expect(isPathWithinRoots('/Users/suntc/LibraryEvil/a.png', ['/Users/suntc/Library'], true)).toBe(false); - }); - - it('defaults to case-insensitive containment on macOS/Windows', () => { - const expected = process.platform === 'darwin' || process.platform === 'win32'; - expect(isPathWithinRoots('/users/x/a.png', ['/Users/x'])).toBe(expected); - }); -}); - describe('parseRangeHeader', () => { it('returns null when there is no Range header', () => { expect(parseRangeHeader(null, 1000)).toBeNull(); @@ -199,6 +164,73 @@ describe('createCdfFileResponse', () => { expect(res.status).toBe(403); }); + it('rejects a sibling directory that differs from the allowed root only by casing when both can exist', async () => { + const allowedRoot = path.join(tempDir, 'Allowed'); + const siblingRoot = path.join(tempDir, 'allowed'); + fs.mkdirSync(allowedRoot); + if (fs.existsSync(siblingRoot)) { + // The current volume is case-insensitive; the macOS host-fold regression below + // exercises that filesystem identity. Case-sensitive CI/volumes continue here. + return; + } + fs.mkdirSync(siblingRoot); + const siblingFile = path.join(siblingRoot, 'clip.mp4'); + fs.writeFileSync(siblingFile, CONTENT); + + const res = await createCdfFileResponse({ + url: cdfUrl(siblingFile), + rangeHeader: null, + allowedRoots: [allowedRoot], + }); + + expect(res.status).toBe(403); + }); + + it('returns 403 for a path whose directory only shares the allowed root prefix', async () => { + const allowedRoot = path.join(tempDir, 'Library'); + const prefixedSibling = path.join(tempDir, 'LibraryEvil'); + fs.mkdirSync(allowedRoot); + fs.mkdirSync(prefixedSibling); + const siblingFile = path.join(prefixedSibling, 'clip.mp4'); + fs.writeFileSync(siblingFile, CONTENT); + + const res = await createCdfFileResponse({ + url: cdfUrl(siblingFile), + rangeHeader: null, + allowedRoots: [allowedRoot], + }); + + expect(res.status).toBe(403); + }); + + it.runIf(process.platform !== 'win32')( + 'returns 403 for ordinary and Range requests through a symlink that points outside the root', + async () => { + const allowedRoot = path.join(tempDir, 'allowed'); + const outsidePath = path.join(tempDir, 'outside.mp4'); + const linkedPath = path.join(allowedRoot, 'linked.mp4'); + fs.mkdirSync(allowedRoot); + fs.writeFileSync(outsidePath, CONTENT); + fs.symlinkSync(outsidePath, linkedPath); + + const [ordinaryResponse, rangeResponse] = await Promise.all([ + createCdfFileResponse({ + url: cdfUrl(linkedPath), + rangeHeader: null, + allowedRoots: [allowedRoot], + }), + createCdfFileResponse({ + url: cdfUrl(linkedPath), + rangeHeader: 'bytes=0-5', + allowedRoots: [allowedRoot], + }), + ]); + + expect(ordinaryResponse.status).toBe(403); + expect(rangeResponse.status).toBe(403); + }, + ); + // Regression (#204 回归): simulate Chromium's host casefolding — the URL path // casing differs from the allowedRoots casing, but the case-insensitive macOS // filesystem still resolves the same file. Must serve 200, not 403. diff --git a/src/main/cdf-file-protocol.ts b/src/main/cdf-file-protocol.ts index 0c3e912f..625d47c7 100644 --- a/src/main/cdf-file-protocol.ts +++ b/src/main/cdf-file-protocol.ts @@ -145,33 +145,59 @@ export interface CdfFileRequest { allowedRoots: string[]; } +interface CanonicalPath { + path: string; + exists: boolean; +} + /** - * macOS/Windows 的默认文件系统大小写不敏感：`/users/…` 与 `/Users/…` 指向同一文件。 - * standard scheme 下 Chromium 会把第一段路径折进 URL host 并小写化 - * （`cdf-file:///Users/…` → `cdf-file://users/…`），因此白名单包含性判断必须 - * 同样按大小写不敏感比较，否则所有 `/Users/…` 资源都会被 403。 + * 把路径解析成文件系统认可的真实路径。目标不存在时，从最近的已有父目录继续解析， + * 这样既能让允许根内的缺失文件返回 404，也不会把 `..` 或父目录软链接误判为允许路径。 */ -const CASE_INSENSITIVE_FILESYSTEM = process.platform === 'darwin' || process.platform === 'win32'; +async function resolveCanonicalPath(filePath: string): Promise { + let current = path.resolve(filePath); + const missingSegments: string[] = []; + + while (true) { + try { + const canonicalParent = await fs.promises.realpath(current); + return { + path: path.join(canonicalParent, ...missingSegments), + exists: missingSegments.length === 0, + }; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== 'ENOENT' && code !== 'ENOTDIR') return null; + + const parent = path.dirname(current); + if (parent === current) return null; + missingSegments.unshift(path.basename(current)); + current = parent; + } + } +} /** - * 判断解析后的绝对路径是否落在任一允许根内。先 `path.resolve` 折叠 `..`， - * 再用 `path.relative` 做包含性判断，杜绝 `/root/../../etc/passwd` 之类逃逸。 - * `caseInsensitive` 默认跟随平台文件系统语义（darwin/win32 不敏感）。 + * `realpath` 已经按当前卷和目录的真实大小写返回路径，因此这里只做逐段精确比较。 + * Windows 盘符本身不区分大小写，但启用 per-directory case sensitivity 的目录段必须区分。 */ -export function isPathWithinRoots( - filePath: string, - allowedRoots: string[], - caseInsensitive: boolean = CASE_INSENSITIVE_FILESYSTEM, -): boolean { - const resolved = path.resolve(filePath); - const target = caseInsensitive ? resolved.toLowerCase() : resolved; - return allowedRoots.some((root) => { - if (!root) return false; - const normalizedRoot = path.resolve(root); - const comparableRoot = caseInsensitive ? normalizedRoot.toLowerCase() : normalizedRoot; - const rel = path.relative(comparableRoot, target); - return rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel)); - }); +function isCanonicalPathWithinRoot(target: string, root: string): boolean { + const normalizedTarget = path.resolve(target); + const normalizedRoot = path.resolve(root); + const targetRoot = path.parse(normalizedTarget).root; + const allowedRoot = path.parse(normalizedRoot).root; + const sameVolume = + process.platform === 'win32' + ? targetRoot.toLowerCase() === allowedRoot.toLowerCase() + : targetRoot === allowedRoot; + if (!sameVolume) return false; + + const targetSegments = normalizedTarget.slice(targetRoot.length).split(path.sep).filter(Boolean); + const rootSegments = normalizedRoot.slice(allowedRoot.length).split(path.sep).filter(Boolean); + return ( + rootSegments.length <= targetSegments.length + && rootSegments.every((segment, index) => segment === targetSegments[index]) + ); } /** @@ -181,14 +207,28 @@ export function isPathWithinRoots( */ export async function createCdfFileResponse(request: CdfFileRequest): Promise { const filePath = resolveCdfFilePath(request.url); + const canonicalTarget = await resolveCanonicalPath(filePath); + if (!canonicalTarget) { + return new Response('File not found', { status: 404 }); + } - if (!isPathWithinRoots(filePath, request.allowedRoots)) { + const canonicalRoots = ( + await Promise.all( + request.allowedRoots + .filter(Boolean) + .map(async (root) => (await resolveCanonicalPath(root))?.path ?? null) + ) + ).filter((root): root is string => root !== null); + if (!canonicalRoots.some((root) => isCanonicalPathWithinRoot(canonicalTarget.path, root))) { return new Response('Forbidden', { status: 403 }); } + if (!canonicalTarget.exists) { + return new Response('File not found', { status: 404 }); + } let stat: fs.Stats; try { - stat = await fs.promises.stat(filePath); + stat = await fs.promises.stat(canonicalTarget.path); } catch { return new Response('File not found', { status: 404 }); } @@ -207,7 +247,7 @@ export async function createCdfFileResponse(request: CdfFileRequest): Promise { }); }); + it('maps an Enterprise Skill to the canonical Managed Skill attribution', async () => { + resolveProjectSkillCatalogMock.mockReturnValue({ skills: [{ + name: 'compliance', qualifiedName: 'acme:compliance', qualifier: 'acme', + description: 'Apply managed compliance policy', sourceKind: 'enterprise', + sourcePath: '/tmp/managed-skills', skillPath: '/tmp/managed-skills/compliance/SKILL.md', + modelDiscovery: 'full', userInvocable: true, + }], warnings: [] }); + + const commands = await collectSkillCommands('/tmp/project'); + + expect(commands[0]).toMatchObject({ + name: 'acme:compliance', + qualifiedName: 'acme:compliance', + skillName: 'compliance', + skillSourceKind: 'enterprise', + source: 'skill:global', + target: 'enterprise:acme:compliance', + sourceLabel: 'Managed Skill', + }); + }); + it('omits Skills whose author disables explicit invocation', async () => { resolveProjectSkillCatalogMock.mockReturnValue({ skills: [{ name: 'internal', description: 'Internal workflow', sourceKind: 'project', diff --git a/src/main/deepagent/delegated-runtime-adapter.test.ts b/src/main/deepagent/delegated-runtime-adapter.test.ts index 494710a9..06545539 100644 --- a/src/main/deepagent/delegated-runtime-adapter.test.ts +++ b/src/main/deepagent/delegated-runtime-adapter.test.ts @@ -85,7 +85,8 @@ describe('createDelegatedRuntimeAdapter', () => { let loadMcpToolsMock: ReturnType; let assembleRuntimeMock: ReturnType; let resolveInterruptOnMock: ReturnType; - let resolveApprovalCoordinatorMock: ReturnType; + let runDelegatedToolActionMock: ReturnType; + let resolveRunDelegatedToolActionMock: ReturnType; let options: CreateDelegatedRuntimeAdapterOptions; beforeEach(() => { @@ -103,9 +104,10 @@ describe('createDelegatedRuntimeAdapter', () => { assemblyWarnings: [], })); resolveInterruptOnMock = vi.fn(() => ({ mcp_search: { allowedDecisions: ['approve', 'reject'] } })); - resolveApprovalCoordinatorMock = vi.fn(() => ({ - runToolAction: vi.fn(async ({ execute }: { execute: () => Promise }) => execute()), - })); + runDelegatedToolActionMock = vi.fn( + async ({ execute }: { execute: () => Promise }) => execute(), + ); + resolveRunDelegatedToolActionMock = vi.fn(() => runDelegatedToolActionMock); parentContext = { approvalMode: 'strict', parentBuiltInToolNames: ['bash', 'fetch'], @@ -119,8 +121,9 @@ describe('createDelegatedRuntimeAdapter', () => { sessionId: 'session-1', } as DelegatedParentContext; options = { - resolveApprovalCoordinator: - resolveApprovalCoordinatorMock as unknown as CreateDelegatedRuntimeAdapterOptions['resolveApprovalCoordinator'], + resolveRunDelegatedToolAction: + resolveRunDelegatedToolActionMock as unknown as + CreateDelegatedRuntimeAdapterOptions['resolveRunDelegatedToolAction'], createResilienceMiddleware: vi.fn(() => []) as unknown as CreateDelegatedRuntimeAdapterOptions['createResilienceMiddleware'], dependencies: { @@ -200,9 +203,9 @@ describe('createDelegatedRuntimeAdapter', () => { expect(passedSnapshot).toBe(skillSnapshot); }); - it('builds an isolated graph per run and resolves the coordinator lazily (ADR-0061)', async () => { + it('builds an isolated graph per run and resolves the tool-action callback lazily (ADR-0061)', async () => { const adapter = createDelegatedRuntimeAdapter(parentContext, options); - expect(resolveApprovalCoordinatorMock).not.toHaveBeenCalled(); + expect(resolveRunDelegatedToolActionMock).not.toHaveBeenCalled(); await adapter.run(requestFor(snapshotFor(null))); await adapter.run(requestFor(snapshotFor(null))); @@ -213,7 +216,69 @@ describe('createDelegatedRuntimeAdapter', () => { ); expect(firstConfig.checkpointer).not.toBe(secondConfig.checkpointer); expect(firstConfig.backend).not.toBe(secondConfig.backend); - expect(resolveApprovalCoordinatorMock).toHaveBeenCalledTimes(2); + expect(resolveRunDelegatedToolActionMock).toHaveBeenCalledTimes(2); + }); + + it('routes gated and ungated tool calls through the same narrow callback', async () => { + const adapter = createDelegatedRuntimeAdapter(parentContext, options); + await adapter.run(requestFor(snapshotFor(null))); + const graphConfig = createAgentGraphMock.mock.calls[0][0] as { + middleware: Array<{ + wrapToolCall: ( + request: unknown, + handler: (request: unknown) => Promise, + ) => Promise; + }>; + }; + const approvalMiddleware = graphConfig.middleware[0]; + const handler = vi.fn(async () => 'handled'); + + await expect(approvalMiddleware.wrapToolCall( + { toolCall: { id: 'gated-1', name: 'mcp_search', args: { query: 'cdf' } } }, + handler, + )).resolves.toBe('handled'); + await expect(approvalMiddleware.wrapToolCall( + { toolCall: { id: 'open-1', name: 'bash', args: { command: 'pwd' } } }, + handler, + )).resolves.toBe('handled'); + + expect(runDelegatedToolActionMock).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + delegatedRunId: 'delegated-1', + action: { id: 'gated-1', name: 'mcp_search', args: { query: 'cdf' } }, + requiresApproval: true, + }), + ); + expect(runDelegatedToolActionMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + delegatedRunId: 'delegated-1', + action: { id: 'open-1', name: 'bash', args: { command: 'pwd' } }, + requiresApproval: false, + }), + ); + }); + + it('propagates a rejected tool-action callback without invoking the tool handler', async () => { + runDelegatedToolActionMock.mockRejectedValueOnce(new Error('approval gateway failed')); + const adapter = createDelegatedRuntimeAdapter(parentContext, options); + await adapter.run(requestFor(snapshotFor(null))); + const graphConfig = createAgentGraphMock.mock.calls[0][0] as { + middleware: Array<{ + wrapToolCall: ( + request: unknown, + handler: (request: unknown) => Promise, + ) => Promise; + }>; + }; + const handler = vi.fn(async () => 'handled'); + + await expect(graphConfig.middleware[0].wrapToolCall( + { toolCall: { id: 'gated-1', name: 'mcp_search', args: {} } }, + handler, + )).rejects.toThrow('approval gateway failed'); + expect(handler).not.toHaveBeenCalled(); }); it('returns the structured child result when it matches the delegated contract', async () => { diff --git a/src/main/deepagent/delegated-runtime-adapter.ts b/src/main/deepagent/delegated-runtime-adapter.ts index 5d733b0e..80c9666e 100644 --- a/src/main/deepagent/delegated-runtime-adapter.ts +++ b/src/main/deepagent/delegated-runtime-adapter.ts @@ -1,4 +1,5 @@ import crypto from 'crypto'; +import type { ToolMessage } from '@langchain/core/messages'; import { MemorySaver } from '@langchain/langgraph'; import { createMiddleware } from 'langchain'; import { createDeepAgent, CompositeBackend, StateBackend } from 'deepagents'; @@ -17,10 +18,10 @@ import { ProjectConfinedFilesystemBackend } from './project-confined-backend'; import { DEEPAGENT_CHECKPOINT_NAMESPACE } from './conversation-working-state'; import { subagentStepStorage } from './subagent-step-storage'; import type { - DelegatedAgentRunCoordinator, DelegatedRuntimeAdapter, DelegatedRuntimeRequest, } from './delegated-agent-run-coordinator'; +import type { DelegatedToolActionInput } from './delegated-tool-approval-scheduler'; import { DELEGATED_TASK_RESULT_SCHEMA, type ApprovalMode, @@ -70,6 +71,10 @@ export type DelegatedResilienceMiddlewareFactory = ( ...allowedToolSets: Array ) => DeepAgentMiddleware; +export type RunDelegatedToolAction = ( + input: DelegatedToolActionInput, +) => Promise; + export interface DelegatedRuntimeExecutionDependencies { assembleRuntime: typeof assembleDeepAgentRuntime; createAgentGraph: typeof createDeepAgent; @@ -82,17 +87,17 @@ export interface DelegatedRuntimeExecutionDependencies { export interface CreateDelegatedRuntimeAdapterOptions { /** - * 审批门控入口的延迟解析：coordinator 构造时持有 adapter，adapter 仅在 - * run() 执行期取回 coordinator，二者不再互相捕获。 + * 审批门控窄能力的延迟解析：coordinator 构造时持有 adapter，adapter 仅在 + * run() 执行期取回执行单次 tool action 的 callback，不感知 coordinator。 */ - resolveApprovalCoordinator: () => DelegatedAgentRunCoordinator; + resolveRunDelegatedToolAction: () => RunDelegatedToolAction; /** 子运行韧性中间件工厂（工具/模型重试与失败观察归属装配层）。 */ createResilienceMiddleware: DelegatedResilienceMiddlewareFactory; dependencies?: Partial; } function createDelegatedToolApprovalMiddleware( - coordinator: DelegatedAgentRunCoordinator, + runDelegatedToolAction: RunDelegatedToolAction, delegatedRunId: string, gatedToolNames: Set, ) { @@ -102,7 +107,7 @@ function createDelegatedToolApprovalMiddleware( const runtimeTool = request as { tool?: { name?: string } }; const toolName = request.toolCall?.name || runtimeTool.tool?.name || 'unknown'; const actionId = request.toolCall?.id || crypto.randomUUID(); - return coordinator.runToolAction({ + return runDelegatedToolAction({ delegatedRunId, action: { id: actionId, @@ -281,7 +286,7 @@ export function createDelegatedRuntimeAdapter( tools: [...childMcpRuntime.tools, ...childScope.builtInTools], middleware: [ createDelegatedToolApprovalMiddleware( - options.resolveApprovalCoordinator(), + options.resolveRunDelegatedToolAction(), request.delegatedRunId, gatedToolNames, ), diff --git a/src/main/deepagent/runtime.ts b/src/main/deepagent/runtime.ts index 9ec75a72..07af10c8 100644 --- a/src/main/deepagent/runtime.ts +++ b/src/main/deepagent/runtime.ts @@ -532,7 +532,7 @@ async function buildDeepAgentRuntime( const delegatedRunRepository = new DelegatedAgentRunRepository(db); // ADR-0061/0062/0063：隔离运行时构造收敛在 delegated-runtime-adapter， - // 父→子继承契约是显式的窄接口；coordinator 经延迟解析注入审批门控。 + // 父→子继承契约是显式的窄接口；coordinator 经延迟解析只注入单次审批执行 callback。 const delegatedRuntimeAdapter: DelegatedRuntimeAdapter = createDelegatedRuntimeAdapter( { approvalMode: currentApprovalMode, @@ -547,7 +547,9 @@ async function buildDeepAgentRuntime( sessionId, }, { - resolveApprovalCoordinator: () => delegatedRunCoordinator, + resolveRunDelegatedToolAction: () => ( + (input) => delegatedRunCoordinator.runToolAction(input) + ), createResilienceMiddleware: createSubagentResilienceMiddleware, }, ); diff --git a/src/main/deepagent/skills-runtime/skill-sources.test.ts b/src/main/deepagent/skills-runtime/skill-sources.test.ts index 92d7f734..f074a9c4 100644 --- a/src/main/deepagent/skills-runtime/skill-sources.test.ts +++ b/src/main/deepagent/skills-runtime/skill-sources.test.ts @@ -3,6 +3,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; import { + getSkillSourceLabel, invalidateSkillSourceCaches, resolveSkillCatalog, resolveSkillSourcePlan, @@ -31,6 +32,25 @@ function writeSkill( ); } +describe('getSkillSourceLabel', () => { + it.each([ + [{ sourceKind: 'built-in' as const }, 'Built-in Skill'], + [{ sourceKind: 'project' as const }, 'Project Skill'], + [ + { sourceKind: 'project-nested' as const, qualifier: 'apps/web' }, + 'Nested Project Skill: apps/web', + ], + [ + { sourceKind: 'project-additional' as const, qualifier: 'docs' }, + 'Project Skill: docs', + ], + [{ sourceKind: 'user' as const }, 'Global Skill'], + [{ sourceKind: 'enterprise' as const }, 'Managed Skill'], + ])('maps $sourceKind to its canonical label', (skill, expectedLabel) => { + expect(getSkillSourceLabel(skill)).toBe(expectedLabel); + }); +}); + describe('resolveSkillSourcePlan', () => { const tempProjectPath = path.join(os.tmpdir(), `cdf-skill-source-test-${Math.random().toString(36).slice(2)}`); const tempHomePath = path.join(os.tmpdir(), `cdf-skill-source-home-${Math.random().toString(36).slice(2)}`); diff --git a/src/main/flow-diagram/flow-diagram-document-store.test.ts b/src/main/flow-diagram/flow-diagram-document-store.test.ts index 3fe39a97..b64e925e 100644 --- a/src/main/flow-diagram/flow-diagram-document-store.test.ts +++ b/src/main/flow-diagram/flow-diagram-document-store.test.ts @@ -2,6 +2,10 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import type { + FlowDiagramDocumentChangeEvent, + FlowDiagramDocumentVersion, +} from '../../shared/flow-diagrams'; import { createFlowDiagramDocumentStore } from './flow-diagram-document-store'; function sceneJson(elementIds: string[] = []): string { @@ -24,41 +28,163 @@ function sceneJson(elementIds: string[] = []): string { describe('FlowDiagramDocumentStore', () => { let projectPath: string; + let stateRoot: string; let filePath: string; beforeEach(() => { projectPath = fs.mkdtempSync(path.join(os.tmpdir(), 'cdf-flow-doc-store-')); + stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'cdf-flow-doc-state-')); filePath = path.join(projectPath, 'diagram.excalidraw'); }); afterEach(() => { fs.rmSync(projectPath, { recursive: true, force: true }); + fs.rmSync(stateRoot, { recursive: true, force: true }); + }); + + it('reads the authoritative document with an opaque content version', async () => { + const original = sceneJson(['one']); + fs.writeFileSync(filePath, original); + const store = createFlowDiagramDocumentStore({ projectPath }); + + const result = await store.readDocument('diagram.excalidraw'); + + expect(result).toEqual({ + ok: true, + document: { + content: original, + version: expect.stringMatching(/^[a-f0-9]{64}$/), + }, + }); + }); + + it('creates a new document without overwriting an existing path', async () => { + const store = createFlowDiagramDocumentStore({ projectPath, stateRoot }); + const content = sceneJson(['one']); + + const created = await store.createDocument('diagram.excalidraw', content); + const duplicate = await store.createDocument('diagram.excalidraw', sceneJson(['two'])); + + expect(created).toEqual({ + ok: true, + filePath, + document: { + content, + version: expect.stringMatching(/^[a-f0-9]{64}$/), + }, + }); + expect(duplicate).toMatchObject({ ok: false, error: { code: 'FILE_EXISTS' } }); + expect(fs.readFileSync(filePath, 'utf-8')).toBe(content); + }); + + it('allocates collision-safe paths for generated document names inside the Store', async () => { + const store = createFlowDiagramDocumentStore({ projectPath, stateRoot }); + + const first = await store.createDocument( + 'diagrams/generated.excalidraw', + sceneJson(['one']), + { collisionSafe: true }, + ); + const second = await store.createDocument( + 'diagrams/generated.excalidraw', + sceneJson(['two']), + { collisionSafe: true }, + ); + + expect(first).toMatchObject({ + ok: true, + filePath: path.join(projectPath, 'diagrams', 'generated.excalidraw'), + }); + expect(second).toMatchObject({ + ok: true, + filePath: path.join(projectPath, 'diagrams', 'generated-2.excalidraw'), + }); }); it('atomically replaces the document when the base content still matches', async () => { const original = sceneJson(['one']); fs.writeFileSync(filePath, original); const notified: string[] = []; + const documentChanges: FlowDiagramDocumentChangeEvent[] = []; const store = createFlowDiagramDocumentStore({ projectPath, notifyFileChange: (changed) => notified.push(changed), + notifyDocumentChange: (change) => documentChanges.push(change), }); + const initial = await store.readDocument('diagram.excalidraw'); + expect(initial.ok).toBe(true); + if (!initial.ok) throw new Error('expected readable document'); const next = sceneJson(['one', 'two']); - await expect(store.saveDocument('diagram.excalidraw', next, original)) - .resolves.toEqual({ ok: true }); + const result = await store.saveDocument( + 'diagram.excalidraw', + next, + initial.document.version, + 'renderer-save-1', + ); + + expect(result).toEqual({ + ok: true, + document: { + content: next, + version: expect.stringMatching(/^[a-f0-9]{64}$/), + }, + }); + if (result.ok) { + expect(result.document.version).not.toBe(initial.document.version); + } expect(fs.readFileSync(filePath, 'utf-8')).toBe(next); expect(notified).toEqual([filePath]); + expect(documentChanges).toEqual([{ + filePath, + version: result.ok ? result.document.version : null, + mutationId: 'renderer-save-1', + }]); expect(fs.readdirSync(projectPath).filter((name) => name.includes('cdf-tmp'))).toEqual([]); }); + it('does not report a published document as failed when a notifier throws', async () => { + const original = sceneJson(['one']); + const next = sceneJson(['two']); + fs.writeFileSync(filePath, original); + const store = createFlowDiagramDocumentStore({ + projectPath, + notifyFileChange: () => { + throw new Error('watcher unavailable'); + }, + notifyDocumentChange: () => { + throw new Error('window closed'); + }, + }); + const initial = await store.readDocument('diagram.excalidraw'); + expect(initial.ok).toBe(true); + if (!initial.ok) throw new Error('expected readable document'); + + const result = await store.saveDocument( + 'diagram.excalidraw', + next, + initial.document.version, + ); + + expect(result).toMatchObject({ ok: true, document: { content: next } }); + expect(fs.readFileSync(filePath, 'utf-8')).toBe(next); + }); + it('returns a conflict with the current content when the document changed externally', async () => { const original = sceneJson(['one']); const external = sceneJson(['agent']); - fs.writeFileSync(filePath, external); + fs.writeFileSync(filePath, original); const store = createFlowDiagramDocumentStore({ projectPath }); + const initial = await store.readDocument('diagram.excalidraw'); + expect(initial.ok).toBe(true); + if (!initial.ok) throw new Error('expected readable document'); + fs.writeFileSync(filePath, external); - const result = await store.saveDocument('diagram.excalidraw', sceneJson(['mine']), original); + const result = await store.saveDocument( + 'diagram.excalidraw', + sceneJson(['mine']), + initial.document.version, + ); expect(result).toEqual({ ok: false, @@ -66,6 +192,7 @@ describe('FlowDiagramDocumentStore', () => { code: 'SOURCE_CHANGED', message: expect.stringContaining('changed'), currentContent: external, + currentVersion: expect.stringMatching(/^[a-f0-9]{64}$/), }, }); expect(fs.readFileSync(filePath, 'utf-8')).toBe(external); @@ -75,10 +202,13 @@ describe('FlowDiagramDocumentStore', () => { const original = sceneJson(['one']); fs.writeFileSync(filePath, original); const store = createFlowDiagramDocumentStore({ projectPath }); + const initial = await store.readDocument('diagram.excalidraw'); + expect(initial.ok).toBe(true); + if (!initial.ok) throw new Error('expected readable document'); const [first, second] = await Promise.all([ - store.saveDocument('diagram.excalidraw', sceneJson(['first']), original), - store.saveDocument('diagram.excalidraw', sceneJson(['second']), original), + store.saveDocument('diagram.excalidraw', sceneJson(['first']), initial.document.version), + store.saveDocument('diagram.excalidraw', sceneJson(['second']), initial.document.version), ]); const outcomes = [first, second]; @@ -89,22 +219,109 @@ describe('FlowDiagramDocumentStore', () => { expect([sceneJson(['first']), sceneJson(['second'])]).toContain(finalContent); }); - it('writes without a CAS guard when no base content is supplied', async () => { - fs.writeFileSync(filePath, sceneJson(['whatever'])); - const store = createFlowDiagramDocumentStore({ projectPath }); + it('revalidates after the controlled pre-publication race seam', async () => { + const original = sceneJson(['one']); + const external = sceneJson(['external']); + fs.writeFileSync(filePath, original); + const initialStore = createFlowDiagramDocumentStore({ projectPath }); + const initial = await initialStore.readDocument('diagram.excalidraw'); + expect(initial.ok).toBe(true); + if (!initial.ok) throw new Error('expected readable document'); + const store = createFlowDiagramDocumentStore({ + projectPath, + beforePublish: () => { + fs.writeFileSync(filePath, external); + }, + }); - const next = sceneJson(['fresh']); - await expect(store.saveDocument('diagram.excalidraw', next, null)) - .resolves.toEqual({ ok: true }); - expect(fs.readFileSync(filePath, 'utf-8')).toBe(next); + const result = await store.saveDocument( + 'diagram.excalidraw', + sceneJson(['mine']), + initial.document.version, + ); + + expect(result).toEqual({ + ok: false, + error: { + code: 'SOURCE_CHANGED', + message: expect.stringContaining('changed'), + currentContent: external, + currentVersion: expect.stringMatching(/^[a-f0-9]{64}$/), + }, + }); + expect(fs.readFileSync(filePath, 'utf-8')).toBe(external); + expect(fs.readdirSync(projectPath).filter((name) => name.includes('cdf-tmp'))).toEqual([]); + }); + + it('owns Agent revision recording and rollback as one document boundary', async () => { + const original = sceneJson(['one']); + const edited = sceneJson(['one', 'agent']); + fs.writeFileSync(filePath, original); + const store = createFlowDiagramDocumentStore({ projectPath, stateRoot }); + const initial = await store.readDocument('diagram.excalidraw'); + expect(initial.ok).toBe(true); + if (!initial.ok) throw new Error('expected readable document'); + + const editResult = await store.applyAgentEdit( + 'diagram.excalidraw', + edited, + initial.document.version, + ); + const rollbackResult = await store.rollbackDocument('diagram.excalidraw'); + + expect(editResult).toMatchObject({ + ok: true, + filePath, + document: { content: edited }, + }); + expect(rollbackResult).toMatchObject({ + ok: true, + filePath, + document: { content: original }, + }); + expect(fs.readFileSync(filePath, 'utf-8')).toBe(original); + }); + + it('creates derived exports without exposing file writes to the service', async () => { + fs.writeFileSync(filePath, sceneJson(['one'])); + const store = createFlowDiagramDocumentStore({ projectPath, stateRoot }); + const bytes = Buffer.from(''); + + const created = await store.createExport({ + sourceFilePath: 'diagram.excalidraw', + format: 'svg', + bytes, + }); + const next = await store.createExport({ + sourceFilePath: 'diagram.excalidraw', + format: 'svg', + bytes, + }); + + expect(created).toEqual({ + ok: true, + filePath: path.join(projectPath, 'diagram.svg'), + }); + expect(next).toEqual({ + ok: true, + filePath: path.join(projectPath, 'diagram-2.svg'), + }); + expect(fs.readFileSync(path.join(projectPath, 'diagram.svg'))).toEqual(bytes); }); it('rejects invalid scenes at the write boundary without touching the document', async () => { const original = sceneJson(['one']); fs.writeFileSync(filePath, original); const store = createFlowDiagramDocumentStore({ projectPath }); + const initial = await store.readDocument('diagram.excalidraw'); + expect(initial.ok).toBe(true); + if (!initial.ok) throw new Error('expected readable document'); - const result = await store.saveDocument('diagram.excalidraw', '{"not":"excalidraw"}', original); + const result = await store.saveDocument( + 'diagram.excalidraw', + '{"not":"excalidraw"}', + initial.document.version, + ); expect(result.ok).toBe(false); expect(fs.readFileSync(filePath, 'utf-8')).toBe(original); @@ -112,10 +329,11 @@ describe('FlowDiagramDocumentStore', () => { it('rejects documents outside the Project and non-diagram extensions', async () => { const store = createFlowDiagramDocumentStore({ projectPath }); + const version = 'untrusted-version' as FlowDiagramDocumentVersion; - await expect(store.saveDocument('../outside.excalidraw', sceneJson(), null)) + await expect(store.saveDocument('../outside.excalidraw', sceneJson(), version)) .resolves.toMatchObject({ ok: false, error: { code: 'PATH_OUTSIDE_PROJECT' } }); - await expect(store.saveDocument('notes.txt', sceneJson(), null)) + await expect(store.saveDocument('notes.txt', sceneJson(), version)) .resolves.toMatchObject({ ok: false, error: { code: 'INVALID_EXTENSION' } }); }); }); diff --git a/src/main/flow-diagram/flow-diagram-document-store.ts b/src/main/flow-diagram/flow-diagram-document-store.ts index 266cd391..e025bf33 100644 --- a/src/main/flow-diagram/flow-diagram-document-store.ts +++ b/src/main/flow-diagram/flow-diagram-document-store.ts @@ -1,22 +1,32 @@ import crypto from 'crypto'; import fs from 'fs'; import path from 'path'; +import log from '../logger'; import { runProjectFileMutation } from '../services/project-file-mutation'; import { FlowDiagramSceneError, parseFlowDiagramScene, } from './flow-diagram-scene'; +import { + createFlowDiagramRevisionStore, + type FlowDiagramRevisionStore, +} from './flow-diagram-revision-store'; import { FLOW_DIAGRAM_SOURCE_CHANGED, + type FlowDiagramDocumentChangeEvent, + type FlowDiagramDocumentReadResult, type FlowDiagramDocumentSaveResult, + type FlowDiagramDocumentSnapshot, + type FlowDiagramDocumentVersion, + type FlowDiagramExportFormat, } from '../../shared/flow-diagrams'; /** * Flow Diagram 文档存储：`.excalidraw` 文档一致性的唯一拥有者。 * - * 用户编辑器 autosave 与 Agent `manage_flow_diagram` 编辑共享同一套 - * 原子 compare-and-swap 替换原语与按 Project 的写串行化；写边界执行 - * 场景校验，冲突以带当前内容的结构化结果返回（ADR-0071 / #200）。 + * CDF 内部写入按 Project 串行化并使用 opaque content version；发布前会 + * 再次校验版本，随后以 rename 原子发布完整文档。任意外部程序不参与同一 + * 协议，因此外部冲突检测是 best-effort，而非跨平台严格 CAS（ADR-0073）。 */ export class FlowDiagramOperationError extends Error { @@ -46,7 +56,7 @@ function nearestExistingAncestor(candidatePath: string): string { return current; } -export function resolveProjectOwnedPath( +function resolveProjectOwnedPath( projectPath: string, requestedPath: string, expectedExtension: string, @@ -98,8 +108,8 @@ export function resolveProjectOwnedPath( return target; } -export function hashBytes(bytes: Buffer): string { - return crypto.createHash('sha256').update(bytes).digest('hex'); +export function hashBytes(bytes: Buffer): FlowDiagramDocumentVersion { + return crypto.createHash('sha256').update(bytes).digest('hex') as FlowDiagramDocumentVersion; } function temporaryPathFor(targetPath: string): string { @@ -109,65 +119,25 @@ function temporaryPathFor(targetPath: string): string { ); } -export async function replaceFileAtomicallyIfUnchanged( - filePath: string, - bytes: Buffer, - expectedBytes: Buffer | null, -): Promise { - fs.mkdirSync(path.dirname(filePath), { recursive: true }); - const temporaryPath = temporaryPathFor(filePath); - try { - const handle = await fs.promises.open(temporaryPath, 'wx', 0o600); - try { - await handle.writeFile(bytes); - await handle.sync(); - } finally { - await handle.close(); - } - - if (expectedBytes === null) { - try { - await fs.promises.link(temporaryPath, filePath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'EEXIST') { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be applied.', - ); - } - throw error; - } - return; - } - - let currentBytes: Buffer; - try { - currentBytes = fs.readFileSync(filePath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be applied.', - ); - } - throw error; - } - if (!currentBytes.equals(expectedBytes)) { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be applied.', - ); - } - // CDF mutations for this Project hold the shared coordinator lock. rename is - // the single atomic publication step, so readers never observe partial bytes. - await fs.promises.rename(temporaryPath, filePath); - } finally { - await fs.promises.rm(temporaryPath, { force: true }); +function collisionSafePath(basePath: string): string { + if (!fs.existsSync(basePath)) return basePath; + const extension = path.extname(basePath); + const stem = basePath.slice(0, -extension.length); + for (let suffix = 2; suffix < 10_000; suffix += 1) { + const candidate = `${stem}-${suffix}${extension}`; + if (!fs.existsSync(candidate)) return candidate; } + throw new FlowDiagramOperationError( + 'PATH_COLLISION', + 'Could not allocate a collision-safe Flow Diagram path.', + ); } -/** Unconditional atomic replacement: same temp + rename publication, no CAS guard. */ -async function replaceFileAtomically(filePath: string, bytes: Buffer): Promise { +async function withPreparedTemporaryFile( + filePath: string, + bytes: Buffer, + publish: (temporaryPath: string) => Promise, +): Promise { fs.mkdirSync(path.dirname(filePath), { recursive: true }); const temporaryPath = temporaryPathFor(filePath); try { @@ -178,42 +148,14 @@ async function replaceFileAtomically(filePath: string, bytes: Buffer): Promise { - let currentBytes: Buffer; - try { - currentBytes = fs.readFileSync(filePath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be completed.', - ); - } - throw error; - } - if (!currentBytes.equals(expectedBytes)) { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be completed.', - ); - } - await fs.promises.unlink(filePath); -} - -export async function writeNewFileAtomically(filePath: string, bytes: Buffer): Promise { - fs.mkdirSync(path.dirname(filePath), { recursive: true }); - const temporaryPath = temporaryPathFor(filePath); - try { - await fs.promises.writeFile(temporaryPath, bytes, { flag: 'wx', mode: 0o600 }); +async function writeNewFileAtomically(filePath: string, bytes: Buffer): Promise { + await withPreparedTemporaryFile(filePath, bytes, async (temporaryPath) => { try { await fs.promises.link(temporaryPath, filePath); } catch (error) { @@ -225,38 +167,399 @@ export async function writeNewFileAtomically(filePath: string, bytes: Buffer): P } throw error; } - } finally { - await fs.promises.rm(temporaryPath, { force: true }); - } + }); } export interface CreateFlowDiagramDocumentStoreOptions { projectPath: string; + stateRoot?: string; + revisionStore?: FlowDiagramRevisionStore; notifyFileChange?: (filePath: string) => void; + notifyDocumentChange?: (event: FlowDiagramDocumentChangeEvent) => void; + /** Controlled race seam used to verify the documented external-writer guarantee. */ + beforePublish?: (context: { + filePath: string; + expectedVersion: FlowDiagramDocumentVersion; + }) => void | Promise; } +export type FlowDiagramDocumentMutationResult = + | { + ok: true; + filePath: string; + document: FlowDiagramDocumentSnapshot; + } + | { + ok: false; + error: { + code: string; + message: string; + currentContent?: string | null; + currentVersion?: FlowDiagramDocumentVersion | null; + }; + }; + +export type FlowDiagramExportWriteResult = + | { ok: true; filePath: string } + | { ok: false; error: { code: string; message: string } }; + export interface FlowDiagramDocumentStore { + /** Reads and validates the authoritative document together with its opaque byte identity. */ + readDocument(filePath: string): Promise; + + /** Creates exactly one new Project-owned document and never overwrites an existing path. */ + createDocument( + filePath: string, + content: string, + options?: { collisionSafe?: boolean }, + ): Promise; + + /** Records the current revision and applies one Agent-produced candidate document. */ + applyAgentEdit( + filePath: string, + content: string, + expectedVersion: FlowDiagramDocumentVersion, + ): Promise; + + /** Restores and consumes the latest applicable Agent revision. */ + rollbackDocument(filePath: string): Promise; + + /** Publishes one derived export without overwriting an explicit output path. */ + createExport(input: { + sourceFilePath: string; + requestedOutputPath?: string; + format: FlowDiagramExportFormat; + bytes: Buffer; + }): Promise; + /** - * Editor autosave entry: validates the scene, then atomically replaces the - * document when its bytes still equal `expectedContent`. `null` skips the - * CAS guard but keeps the atomic temp + rename publication. A conflict - * returns the current on-disk content so the caller can surface it. + * Editor autosave entry: validates the scene, then publishes only while the + * authoritative document still has `expectedVersion`. */ saveDocument( filePath: string, content: string, - expectedContent: string | null, + expectedVersion: FlowDiagramDocumentVersion, + mutationId?: string, ): Promise; } +function readCurrentBytes(filePath: string): Buffer { + try { + return fs.readFileSync(filePath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + throw new FlowDiagramOperationError( + FLOW_DIAGRAM_SOURCE_CHANGED, + 'The Flow Diagram changed before the operation could be applied.', + ); + } + throw error; + } +} + +function assertCurrentVersion( + filePath: string, + expectedVersion: FlowDiagramDocumentVersion, +): void { + if (hashBytes(readCurrentBytes(filePath)) !== expectedVersion) { + throw new FlowDiagramOperationError( + FLOW_DIAGRAM_SOURCE_CHANGED, + 'The Flow Diagram changed before the operation could be applied.', + ); + } +} + +async function replaceFileAtomicallyIfVersionMatches( + filePath: string, + bytes: Buffer, + expectedVersion: FlowDiagramDocumentVersion, + beforePublish?: CreateFlowDiagramDocumentStoreOptions['beforePublish'], +): Promise { + await withPreparedTemporaryFile(filePath, bytes, async (temporaryPath) => { + assertCurrentVersion(filePath, expectedVersion); + await beforePublish?.({ filePath, expectedVersion }); + // Revalidate after the controlled race seam, immediately before the one + // atomic publication step. See ADR-0073 for the remaining external gap. + assertCurrentVersion(filePath, expectedVersion); + await fs.promises.rename(temporaryPath, filePath); + }); +} + export function createFlowDiagramDocumentStore( options: CreateFlowDiagramDocumentStoreOptions, ): FlowDiagramDocumentStore { const projectPath = path.resolve(options.projectPath); - const notify = (filePath: string) => options.notifyFileChange?.(filePath); + const notifyFile = (filePath: string) => { + try { + options.notifyFileChange?.(filePath); + } catch (error) { + log.warn('[flow-diagram-store] file notification failed after publication:', error); + } + }; + const notifyDocument = ( + filePath: string, + version: FlowDiagramDocumentVersion, + mutationId?: string, + ) => { + notifyFile(filePath); + try { + options.notifyDocumentChange?.({ filePath, version, mutationId }); + } catch (error) { + log.warn('[flow-diagram-store] document notification failed after publication:', error); + } + }; + let revisionStore = options.revisionStore; + const revisions = (): FlowDiagramRevisionStore => { + if (!revisionStore) { + if (!options.stateRoot) { + throw new FlowDiagramOperationError( + 'REVISION_STORE_UNAVAILABLE', + 'Flow Diagram revision storage is not configured.', + ); + } + revisionStore = createFlowDiagramRevisionStore(projectPath, options.stateRoot); + } + return revisionStore; + }; return { - async saveDocument(filePath, content, expectedContent) { + async readDocument(filePath) { + try { + const target = resolveProjectOwnedPath(projectPath, filePath, '.excalidraw'); + const bytes = fs.readFileSync(target); + const content = bytes.toString('utf-8'); + parseFlowDiagramScene(content); + return { + ok: true, + document: { content, version: hashBytes(bytes) }, + }; + } catch (error) { + return failureResult(error, 'READ_FAILED'); + } + }, + + async createDocument(filePath, content, createOptions) { + let requestedTarget: string; + try { + requestedTarget = resolveProjectOwnedPath(projectPath, filePath, '.excalidraw'); + parseFlowDiagramScene(content); + } catch (error) { + return failureResult(error, 'CREATE_FAILED'); + } + + return runProjectFileMutation(projectPath, async () => { + const bytes = Buffer.from(content, 'utf-8'); + try { + const target = createOptions?.collisionSafe + ? collisionSafePath(requestedTarget) + : requestedTarget; + await writeNewFileAtomically(target, bytes); + const version = hashBytes(bytes); + notifyDocument(target, version); + return { + ok: true, + filePath: target, + document: { content, version }, + }; + } catch (error) { + return failureResult(error, 'CREATE_FAILED'); + } + }); + }, + + async applyAgentEdit(filePath, content, expectedVersion) { + let target: string; + let candidateBytes: Buffer; + try { + target = resolveProjectOwnedPath(projectPath, filePath, '.excalidraw'); + parseFlowDiagramScene(content); + candidateBytes = Buffer.from(content, 'utf-8'); + } catch (error) { + return failureResult(error, 'INVALID_SCENE'); + } + + return runProjectFileMutation(projectPath, async () => { + let currentBytes: Buffer; + try { + currentBytes = readCurrentBytes(target); + parseFlowDiagramScene(currentBytes); + assertCurrentVersion(target, expectedVersion); + } catch (error) { + return error instanceof FlowDiagramOperationError + && error.code === FLOW_DIAGRAM_SOURCE_CHANGED + ? sourceChangedResult(target, error.message) + : failureResult(error, 'READ_FAILED'); + } + + let revisionToken: string; + try { + revisionToken = await revisions().record(target, currentBytes, candidateBytes); + } catch (error) { + return failureResult( + new FlowDiagramOperationError('REVISION_FAILED', safeErrorMessage(error)), + 'REVISION_FAILED', + ); + } + + try { + await replaceFileAtomicallyIfVersionMatches( + target, + candidateBytes, + expectedVersion, + options.beforePublish, + ); + } catch (error) { + try { + await revisions().consumeLatest(target, revisionToken); + } catch (cleanupError) { + return failureResult( + new FlowDiagramOperationError('REVISION_FAILED', safeErrorMessage(cleanupError)), + 'REVISION_FAILED', + ); + } + return error instanceof FlowDiagramOperationError + && error.code === FLOW_DIAGRAM_SOURCE_CHANGED + ? sourceChangedResult(target, error.message) + : failureResult(error, 'WRITE_FAILED'); + } + + const version = hashBytes(candidateBytes); + notifyDocument(target, version); + return { + ok: true, + filePath: target, + document: { content, version }, + }; + }); + }, + + async rollbackDocument(filePath) { + let target: string; + try { + target = resolveProjectOwnedPath(projectPath, filePath, '.excalidraw'); + } catch (error) { + return failureResult(error, 'ROLLBACK_FAILED'); + } + + return runProjectFileMutation(projectPath, async () => { + let currentBytes: Buffer; + try { + currentBytes = readCurrentBytes(target); + parseFlowDiagramScene(currentBytes); + } catch (error) { + return failureResult(error, 'ROLLBACK_FAILED'); + } + + let revision; + try { + revision = await revisions().peekLatest(target); + if (!revision) { + throw new FlowDiagramOperationError( + 'NO_REVISION', + 'No applicable Agent edit revision is available for this Flow Diagram.', + ); + } + parseFlowDiagramScene(revision.sourceBytes); + } catch (error) { + return failureResult(error, 'ROLLBACK_FAILED'); + } + + const currentVersion = hashBytes(currentBytes); + if (currentVersion !== revision.appliedSourceHash) { + return sourceChangedResult( + target, + 'The Flow Diagram changed after the latest Agent edit; rollback was not applied.', + ); + } + + try { + await replaceFileAtomicallyIfVersionMatches( + target, + revision.sourceBytes, + currentVersion, + options.beforePublish, + ); + } catch (error) { + return error instanceof FlowDiagramOperationError + && error.code === FLOW_DIAGRAM_SOURCE_CHANGED + ? sourceChangedResult(target, error.message) + : failureResult(error, 'ROLLBACK_FAILED'); + } + + const rollbackVersion = hashBytes(revision.sourceBytes); + try { + await revisions().consumeLatest(target, revision.token); + } catch (error) { + try { + await replaceFileAtomicallyIfVersionMatches( + target, + currentBytes, + rollbackVersion, + ); + } catch { + return failureResult( + new FlowDiagramOperationError( + 'ROLLBACK_RESTORE_FAILED', + 'Rollback failed and the previous source could not be restored.', + ), + 'ROLLBACK_RESTORE_FAILED', + ); + } + return failureResult( + new FlowDiagramOperationError('REVISION_FAILED', safeErrorMessage(error)), + 'REVISION_FAILED', + ); + } + + const content = revision.sourceBytes.toString('utf-8'); + notifyDocument(target, rollbackVersion); + return { + ok: true, + filePath: target, + document: { content, version: rollbackVersion }, + }; + }); + }, + + async createExport(input) { + let requestedTarget: string; + try { + const source = resolveProjectOwnedPath( + projectPath, + input.sourceFilePath, + '.excalidraw', + ); + const extension = `.${input.format}`; + requestedTarget = input.requestedOutputPath + ? resolveProjectOwnedPath(projectPath, input.requestedOutputPath, extension) + : resolveProjectOwnedPath( + projectPath, + path.join( + path.dirname(path.relative(projectPath, source)), + `${path.basename(source, '.excalidraw')}${extension}`, + ), + extension, + ); + } catch (error) { + return failureResult(error, 'EXPORT_FAILED'); + } + + return runProjectFileMutation(projectPath, async () => { + try { + const target = input.requestedOutputPath + ? requestedTarget + : collisionSafePath(requestedTarget); + await writeNewFileAtomically(target, input.bytes); + notifyFile(target); + return { ok: true, filePath: target }; + } catch (error) { + return failureResult(error, 'EXPORT_FAILED'); + } + }); + }, + + async saveDocument(filePath, content, expectedVersion, mutationId) { let target: string; try { target = resolveProjectOwnedPath(projectPath, filePath, '.excalidraw'); @@ -272,31 +575,40 @@ export function createFlowDiagramDocumentStore( return runProjectFileMutation(projectPath, async () => { try { const bytes = Buffer.from(content, 'utf-8'); - if (expectedContent === null) { - await replaceFileAtomically(target, bytes); - } else { - await replaceFileAtomicallyIfUnchanged( - target, - bytes, - Buffer.from(expectedContent, 'utf-8'), - ); - } - notify(target); - return { ok: true } as const; + await replaceFileAtomicallyIfVersionMatches( + target, + bytes, + expectedVersion, + options.beforePublish, + ); + const version = hashBytes(bytes); + notifyDocument(target, version, mutationId); + return { + ok: true, + document: { content, version }, + } as const; } catch (error) { if ( error instanceof FlowDiagramOperationError && error.code === FLOW_DIAGRAM_SOURCE_CHANGED ) { let currentContent: string | null = null; + let currentVersion: FlowDiagramDocumentVersion | null = null; try { - currentContent = fs.readFileSync(target, 'utf-8'); + const currentBytes = fs.readFileSync(target); + currentContent = currentBytes.toString('utf-8'); + currentVersion = hashBytes(currentBytes); } catch { currentContent = null; } return { ok: false as const, - error: { code: FLOW_DIAGRAM_SOURCE_CHANGED, message: error.message, currentContent }, + error: { + code: FLOW_DIAGRAM_SOURCE_CHANGED, + message: error.message, + currentContent, + currentVersion, + }, }; } return failureResult(error, 'WRITE_FAILED'); @@ -306,7 +618,10 @@ export function createFlowDiagramDocumentStore( }; } -function failureResult(error: unknown, fallbackCode: string): FlowDiagramDocumentSaveResult { +function failureResult( + error: unknown, + fallbackCode: string, +): Extract { if (error instanceof FlowDiagramOperationError || error instanceof FlowDiagramSceneError) { return { ok: false, error: { code: error.code, message: error.message } }; } @@ -315,3 +630,33 @@ function failureResult(error: unknown, fallbackCode: string): FlowDiagramDocumen error: { code: fallbackCode, message: 'The Flow Diagram document could not be saved safely.' }, }; } + +function safeErrorMessage(error: unknown): string { + return error instanceof Error + ? error.message + : 'The Flow Diagram operation could not be completed safely.'; +} + +function sourceChangedResult( + filePath: string, + message: string, +): FlowDiagramDocumentMutationResult { + let currentContent: string | null = null; + let currentVersion: FlowDiagramDocumentVersion | null = null; + try { + const currentBytes = fs.readFileSync(filePath); + currentContent = currentBytes.toString('utf-8'); + currentVersion = hashBytes(currentBytes); + } catch { + currentContent = null; + } + return { + ok: false, + error: { + code: FLOW_DIAGRAM_SOURCE_CHANGED, + message, + currentContent, + currentVersion, + }, + }; +} diff --git a/src/main/flow-diagram/flow-diagram-service.integration.test.ts b/src/main/flow-diagram/flow-diagram-service.integration.test.ts index 23c516b7..614b31a3 100644 --- a/src/main/flow-diagram/flow-diagram-service.integration.test.ts +++ b/src/main/flow-diagram/flow-diagram-service.integration.test.ts @@ -430,7 +430,7 @@ describe('FlowDiagramService integration', () => { const revisionGate = new Promise((resolve) => { releaseRevision = resolve; }); - const coordinatedService = createFlowDiagramService({ + const coordinatedStore = createFlowDiagramDocumentStore({ projectPath, stateRoot, revisionStore: { @@ -443,6 +443,14 @@ describe('FlowDiagramService integration', () => { consumeLatest: vi.fn(), }, }); + const coordinatedService = createFlowDiagramService({ + projectPath, + stateRoot, + documentStore: coordinatedStore, + }); + const initial = await coordinatedStore.readDocument(filePath); + expect(initial.ok).toBe(true); + if (!initial.ok) throw new Error('expected readable document'); const agentEdit = coordinatedService.execute({ action: 'edit', @@ -454,7 +462,7 @@ describe('FlowDiagramService integration', () => { const staleAutosave = documentStore.saveDocument( filePath, original.toString('utf8'), - original.toString('utf8'), + initial.document.version, ); releaseRevision(); @@ -511,7 +519,7 @@ describe('FlowDiagramService integration', () => { writeScene(filePath, scene([rectangle('original')])); const external = Buffer.from(`${JSON.stringify(scene([rectangle('external')]), null, 2)}\n`); const consumeLatest = vi.fn(async () => undefined); - const guardedService = createFlowDiagramService({ + const guardedStore = createFlowDiagramDocumentStore({ projectPath, stateRoot, revisionStore: { @@ -523,6 +531,11 @@ describe('FlowDiagramService integration', () => { consumeLatest, }, }); + const guardedService = createFlowDiagramService({ + projectPath, + stateRoot, + documentStore: guardedStore, + }); expect(await guardedService.execute({ action: 'edit', @@ -539,7 +552,7 @@ describe('FlowDiagramService integration', () => { const external = Buffer.from(`${JSON.stringify(scene([rectangle('external')]), null, 2)}\n`); const previous = Buffer.from(`${JSON.stringify(scene([rectangle('previous')]), null, 2)}\n`); const consumeLatest = vi.fn(); - const guardedService = createFlowDiagramService({ + const guardedStore = createFlowDiagramDocumentStore({ projectPath, stateRoot, revisionStore: { @@ -555,6 +568,11 @@ describe('FlowDiagramService integration', () => { consumeLatest, }, }); + const guardedService = createFlowDiagramService({ + projectPath, + stateRoot, + documentStore: guardedStore, + }); expect(await guardedService.execute({ action: 'rollback', @@ -568,7 +586,7 @@ describe('FlowDiagramService integration', () => { const filePath = path.join(projectPath, 'diagram.excalidraw'); const original = writeScene(filePath, scene([rectangle('one')])); - const revisionFailure = createFlowDiagramService({ + const revisionFailureStore = createFlowDiagramDocumentStore({ projectPath, stateRoot, revisionStore: { @@ -577,6 +595,11 @@ describe('FlowDiagramService integration', () => { consumeLatest: vi.fn(), }, }); + const revisionFailure = createFlowDiagramService({ + projectPath, + stateRoot, + documentStore: revisionFailureStore, + }); expect(await revisionFailure.execute({ action: 'edit', file_path: filePath, @@ -584,10 +607,15 @@ describe('FlowDiagramService integration', () => { })).toMatchObject({ ok: false, error: { code: 'REVISION_FAILED' } }); expect(fs.readFileSync(filePath)).toEqual(original); + const replacementFailureStore = createFlowDiagramDocumentStore({ + projectPath, + stateRoot, + beforePublish: vi.fn(async () => { throw new Error('replace interrupted'); }), + }); const replacementFailure = createFlowDiagramService({ projectPath, stateRoot, - replaceFile: vi.fn(async () => { throw new Error('replace interrupted'); }), + documentStore: replacementFailureStore, }); expect(await replacementFailure.execute({ action: 'edit', @@ -701,11 +729,16 @@ describe('FlowDiagramService integration', () => { })), consumeLatest: vi.fn(async () => { throw new Error('manifest write failed'); }), }; - const rollbackService = createFlowDiagramService({ + const rollbackDocumentStore = createFlowDiagramDocumentStore({ projectPath, stateRoot, revisionStore: failingStore, }); + const rollbackService = createFlowDiagramService({ + projectPath, + stateRoot, + documentStore: rollbackDocumentStore, + }); expect(await rollbackService.execute({ action: 'rollback', file_path: filePath })).toMatchObject({ ok: false, diff --git a/src/main/flow-diagram/flow-diagram-service.ts b/src/main/flow-diagram/flow-diagram-service.ts index 3925ca51..932b2d0b 100644 --- a/src/main/flow-diagram/flow-diagram-service.ts +++ b/src/main/flow-diagram/flow-diagram-service.ts @@ -1,12 +1,8 @@ -import fs from 'fs'; import path from 'path'; import { + createFlowDiagramDocumentStore, + type FlowDiagramDocumentStore, FlowDiagramOperationError, - hashBytes, - removeFileAtomicallyIfUnchanged, - replaceFileAtomicallyIfUnchanged, - resolveProjectOwnedPath, - writeNewFileAtomically, } from './flow-diagram-document-store'; import { createFlowDiagramScene, @@ -24,11 +20,7 @@ import { type FlowDiagramExportArtifact, type FlowDiagramExportFormat, } from './flow-diagram-export-renderer'; -import { - createFlowDiagramRevisionStore, - type FlowDiagramRevisionStore, -} from './flow-diagram-revision-store'; -import { runProjectFileMutation } from '../services/project-file-mutation'; +import type { FlowDiagramDocumentChangeEvent } from '../../shared/flow-diagrams'; export type FlowDiagramEditOperation = | { op: 'add'; elements: Array> } @@ -86,18 +78,13 @@ export interface FlowDiagramService { export interface CreateFlowDiagramServiceOptions { projectPath: string; stateRoot: string; - revisionStore?: FlowDiagramRevisionStore; - replaceFile?: (filePath: string, bytes: Buffer) => Promise; - replaceFileIfUnchanged?: ( - filePath: string, - bytes: Buffer, - expectedBytes: Buffer | null, - ) => Promise; + documentStore?: FlowDiagramDocumentStore; renderExport?: ( scene: ExcalidrawScene, format: FlowDiagramExportFormat, ) => Promise; notifyFileChange?: (filePath: string) => void; + notifyDocumentChange?: (event: FlowDiagramDocumentChangeEvent) => void; } function safeName(value: string | undefined): string { @@ -111,20 +98,6 @@ function safeName(value: string | undefined): string { return normalized || 'flow-diagram'; } -function collisionSafePath(basePath: string): string { - if (!fs.existsSync(basePath)) return basePath; - const extension = path.extname(basePath); - const stem = basePath.slice(0, -extension.length); - for (let suffix = 2; suffix < 10_000; suffix += 1) { - const candidate = `${stem}-${suffix}${extension}`; - if (!fs.existsSync(candidate)) return candidate; - } - throw new FlowDiagramOperationError( - 'PATH_COLLISION', - 'Could not allocate a collision-safe Flow Diagram path.', - ); -} - function artifactFor(projectPath: string, filePath: string): FlowDiagramArtifactDisplay { const relativePath = path.relative(projectPath, filePath).replace(/\\/g, '/'); const title = path.basename(filePath, '.excalidraw'); @@ -387,52 +360,39 @@ export function createFlowDiagramService( options: CreateFlowDiagramServiceOptions, ): FlowDiagramService { const projectPath = path.resolve(options.projectPath); - const revisionStore = options.revisionStore - ?? createFlowDiagramRevisionStore(projectPath, options.stateRoot); - const replaceFileIfUnchanged = options.replaceFileIfUnchanged ?? ( - options.replaceFile - ? async (filePath: string, bytes: Buffer, expectedBytes: Buffer | null) => { - if ( - (expectedBytes === null && fs.existsSync(filePath)) - || (expectedBytes !== null && ( - !fs.existsSync(filePath) || !fs.readFileSync(filePath).equals(expectedBytes) - )) - ) { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed before the operation could be applied.', - ); - } - await options.replaceFile!(filePath, bytes); - } - : replaceFileAtomicallyIfUnchanged - ); + const documentStore = options.documentStore ?? createFlowDiagramDocumentStore({ + projectPath, + stateRoot: options.stateRoot, + notifyFileChange: options.notifyFileChange, + notifyDocumentChange: options.notifyDocumentChange, + }); const renderExport = options.renderExport ?? renderFlowDiagramExport; - const notify = (filePath: string) => options.notifyFileChange?.(filePath); - const executeUnlocked = async (input: FlowDiagramActionInput): Promise => { + const execute = async (input: FlowDiagramActionInput): Promise => { const action = input.action; if (action === 'read_format') return success(action, formatDescription()); if (action === 'create') { try { - const target = input.file_path - ? resolveProjectOwnedPath(projectPath, input.file_path, '.excalidraw') - : collisionSafePath(resolveProjectOwnedPath( - projectPath, - path.join('diagrams', `${safeName(input.name)}.excalidraw`), - '.excalidraw', - )); - if (input.file_path && fs.existsSync(target)) { - throw new FlowDiagramOperationError( - 'FILE_EXISTS', - 'The requested Flow Diagram already exists; no file was overwritten.', + const created = createFlowDiagramScene(input.elements ?? []); + const requestedPath = input.file_path + ?? path.join('diagrams', `${safeName(input.name)}.excalidraw`); + const result = await documentStore.createDocument( + requestedPath, + serializeFlowDiagramScene(created).toString('utf-8'), + { collisionSafe: !input.file_path }, + ); + if (!result.ok) { + return failure( + action, + new FlowDiagramOperationError(result.error.code, result.error.message), + 'CREATE_FAILED', ); } - const created = createFlowDiagramScene(input.elements ?? []); - await writeNewFileAtomically(target, serializeFlowDiagramScene(created)); - notify(target); - return success(action, { ...fileData(projectPath, target), scene: created }); + return success(action, { + ...fileData(projectPath, result.filePath), + scene: created, + }); } catch (error) { return failure(action, error, 'CREATE_FAILED'); } @@ -449,156 +409,70 @@ export function createFlowDiagramService( ); } - let target: string; - try { - target = resolveProjectOwnedPath(projectPath, input.file_path, '.excalidraw'); - if ( - action !== 'rollback' - && (!fs.existsSync(target) || !fs.statSync(target).isFile()) - ) { - throw new FlowDiagramOperationError('SOURCE_NOT_FOUND', 'The Flow Diagram source file does not exist.'); + if (action === 'rollback') { + const result = await documentStore.rollbackDocument(input.file_path); + if (!result.ok) { + return failure( + action, + new FlowDiagramOperationError(result.error.code, result.error.message), + 'ROLLBACK_FAILED', + ); } + return success(action, fileData(projectPath, result.filePath)); + } + + const currentResult = await documentStore.readDocument(input.file_path); + if (!currentResult.ok) { + return failure( + action, + new FlowDiagramOperationError( + currentResult.error.code, + currentResult.error.message, + ), + action === 'export' ? 'EXPORT_FAILED' : 'READ_FAILED', + ); + } + const target = path.resolve(projectPath, input.file_path); + let current: ExcalidrawScene; + try { + current = parseFlowDiagramScene(currentResult.document.content); } catch (error) { - return failure(action, error, 'SOURCE_NOT_FOUND'); + return failure(action, error, 'INVALID_SCENE'); } if (action === 'get') { - try { - const current = parseFlowDiagramScene(fs.readFileSync(target)); - return success(action, { ...fileData(projectPath, target), scene: current }); - } catch (error) { - return failure(action, error, 'READ_FAILED'); - } + return success(action, { ...fileData(projectPath, target), scene: current }); } if (action === 'edit') { - const originalBytes = fs.readFileSync(target); - let original: ExcalidrawScene; - try { - original = parseFlowDiagramScene(originalBytes); - } catch (error) { - return failure(action, error, 'INVALID_SCENE'); - } let candidate: ReturnType; try { - candidate = applyOperations(original, input.operations ?? []); + candidate = applyOperations(current, input.operations ?? []); } catch (error) { return failure(action, error, 'INVALID_OPERATION'); } - const candidateBytes = serializeFlowDiagramScene(candidate.scene); - let revisionToken: string; - try { - revisionToken = await revisionStore.record(target, originalBytes, candidateBytes); - } catch (error) { + const result = await documentStore.applyAgentEdit( + input.file_path, + serializeFlowDiagramScene(candidate.scene).toString('utf-8'), + currentResult.document.version, + ); + if (!result.ok) { return failure( action, - new FlowDiagramOperationError('REVISION_FAILED', safeMessage(error)), - 'REVISION_FAILED', + new FlowDiagramOperationError(result.error.code, result.error.message), + 'WRITE_FAILED', ); } - try { - await replaceFileIfUnchanged(target, candidateBytes, originalBytes); - notify(target); - return success(action, { - ...fileData(projectPath, target), - summary: candidate.summary, - }); - } catch (error) { - try { - await replaceFileIfUnchanged(target, originalBytes, candidateBytes); - } catch { - // A concurrent source is preserved; default replacement never exposes partial bytes. - } - try { - await revisionStore.consumeLatest(target, revisionToken); - } catch (cleanupError) { - return failure( - action, - new FlowDiagramOperationError('REVISION_FAILED', safeMessage(cleanupError)), - 'REVISION_FAILED', - ); - } - return error instanceof FlowDiagramOperationError - ? failure(action, error, 'WRITE_FAILED') - : failure( - action, - new FlowDiagramOperationError('WRITE_FAILED', safeMessage(error)), - 'WRITE_FAILED', - ); - } - } - - if (action === 'rollback') { - const currentBytes = fs.existsSync(target) ? fs.readFileSync(target) : null; - let rollbackBytes: Buffer | null = null; - try { - const revision = await revisionStore.peekLatest(target); - if (!revision) { - throw new FlowDiagramOperationError( - 'NO_REVISION', - 'No applicable Agent edit revision is available for this Flow Diagram.', - ); - } - parseFlowDiagramScene(revision.sourceBytes); - if (!currentBytes || hashBytes(currentBytes) !== revision.appliedSourceHash) { - throw new FlowDiagramOperationError( - 'SOURCE_CHANGED', - 'The Flow Diagram changed after the latest Agent edit; rollback was not applied.', - ); - } - rollbackBytes = revision.sourceBytes; - await replaceFileIfUnchanged(target, revision.sourceBytes, currentBytes); - await revisionStore.consumeLatest(target, revision.token); - notify(target); - return success(action, fileData(projectPath, target)); - } catch (error) { - if (rollbackBytes) { - try { - if (currentBytes) { - await replaceFileIfUnchanged(target, currentBytes, rollbackBytes); - } else { - await removeFileAtomicallyIfUnchanged(target, rollbackBytes); - } - } catch (restoreError) { - if (!(restoreError instanceof FlowDiagramOperationError && restoreError.code === 'SOURCE_CHANGED')) { - return failure( - action, - new FlowDiagramOperationError( - 'ROLLBACK_RESTORE_FAILED', - 'Rollback failed and the previous source could not be restored.', - ), - 'ROLLBACK_RESTORE_FAILED', - ); - } - } - } - return failure(action, error, 'ROLLBACK_FAILED'); - } + return success(action, { + ...fileData(projectPath, result.filePath), + summary: candidate.summary, + }); } try { - const currentBytes = fs.readFileSync(target); - const current = parseFlowDiagramScene(currentBytes); if (input.format !== 'svg' && input.format !== 'png') { throw new FlowDiagramOperationError('FORMAT_REQUIRED', 'Export format must be png or svg.'); } - const expectedExtension = `.${input.format}`; - const requestedOutput = input.output_path - ? resolveProjectOwnedPath(projectPath, input.output_path, expectedExtension) - : collisionSafePath(resolveProjectOwnedPath( - projectPath, - path.join( - path.dirname(path.relative(projectPath, target)), - `${path.basename(target, '.excalidraw')}${expectedExtension}`, - ), - expectedExtension, - )); - if (input.output_path && fs.existsSync(requestedOutput)) { - throw new FlowDiagramOperationError( - 'FILE_EXISTS', - 'The requested export already exists; no file was overwritten.', - ); - } let artifact: FlowDiagramExportArtifact; try { artifact = await renderExport(current, input.format); @@ -608,21 +482,29 @@ export function createFlowDiagramService( 'The Flow Diagram could not be rendered for export.', ); } - await writeNewFileAtomically(requestedOutput, artifact.bytes); - notify(requestedOutput); - const relativePath = path.relative(projectPath, requestedOutput).replace(/\\/g, '/'); + const writeResult = await documentStore.createExport({ + sourceFilePath: input.file_path, + requestedOutputPath: input.output_path, + format: input.format, + bytes: artifact.bytes, + }); + if (!writeResult.ok) { + throw new FlowDiagramOperationError( + writeResult.error.code, + writeResult.error.message, + ); + } + const relativePath = path.relative(projectPath, writeResult.filePath).replace(/\\/g, '/'); return success(action, { - filePath: requestedOutput, + filePath: writeResult.filePath, relativePath, mimeType: artifact.mimeType, artifact: { kind: 'file', - path: requestedOutput, - title: path.basename(requestedOutput), + path: writeResult.filePath, + title: path.basename(writeResult.filePath), mimeType: artifact.mimeType, - displayMarkdown: input.format === 'svg' || input.format === 'png' - ? `![${path.basename(requestedOutput)}](${requestedOutput})` - : `[${path.basename(requestedOutput)}](${requestedOutput})`, + displayMarkdown: `![${path.basename(writeResult.filePath)}](${writeResult.filePath})`, }, }); } catch (error) { @@ -631,9 +513,6 @@ export function createFlowDiagramService( }; return { - execute: (input) => runProjectFileMutation( - projectPath, - () => executeUnlocked(input), - ), + execute, }; } diff --git a/src/main/flow-diagram/manage-flow-diagram-tool.ts b/src/main/flow-diagram/manage-flow-diagram-tool.ts index 7504174c..edaf79aa 100644 --- a/src/main/flow-diagram/manage-flow-diagram-tool.ts +++ b/src/main/flow-diagram/manage-flow-diagram-tool.ts @@ -2,7 +2,10 @@ import { app } from 'electron'; import path from 'path'; import { tool } from '@langchain/core/tools'; import { z } from 'zod'; -import { notifyFileChange } from '../services/file-watcher'; +import { + notifyFileChange, + notifyFlowDiagramDocumentChange, +} from '../services/file-watcher'; import { createFlowDiagramService, type FlowDiagramService, @@ -76,6 +79,7 @@ export function createManageFlowDiagramTool( projectPath, stateRoot: options.stateRoot ?? path.join(app.getPath('userData'), 'flow-diagrams'), notifyFileChange, + notifyDocumentChange: notifyFlowDiagramDocumentChange, renderExport: renderFlowDiagramExportAdapter, }); diff --git a/src/main/ipc-handlers.ts b/src/main/ipc-handlers.ts index ff8b18c4..52648fe9 100644 --- a/src/main/ipc-handlers.ts +++ b/src/main/ipc-handlers.ts @@ -19,7 +19,13 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; import { readDirectory, readFile, getFileInfo, writeFile, createFile, createDirectory, renameEntry, trashEntry, resolveProjectFile } from './services/file-system'; -import { ensureFileWatcher, notifyFileChange, watchDirectory, unwatchDirectory } from './services/file-watcher'; +import { + ensureFileWatcher, + notifyFileChange, + notifyFlowDiagramDocumentChange, + watchDirectory, + unwatchDirectory, +} from './services/file-watcher'; import { listPhysicalSkills, listResolvedSkillViews, @@ -1261,17 +1267,34 @@ export function registerIpcHandlers() { } }); - typedHandle('flow-diagram:save-document', async (_, rootPath, filePath, content, expectedContent) => { + typedHandle('flow-diagram:load-document', async (_, rootPath, filePath) => { if (!isRegisteredProjectRoot(rootPath)) { return { ok: false, error: { code: 'EACCES', message: 'rootPath is not a registered project root' } }; } - const documentStore = createFlowDiagramDocumentStore({ - projectPath: rootPath, - notifyFileChange, - }); - return documentStore.saveDocument(filePath, content, expectedContent); + const documentStore = createFlowDiagramDocumentStore({ projectPath: rootPath }); + return documentStore.readDocument(filePath); }); + typedHandle( + 'flow-diagram:save-document', + async (_, rootPath, filePath, content, expectedVersion, mutationId) => { + if (!isRegisteredProjectRoot(rootPath)) { + return { ok: false, error: { code: 'EACCES', message: 'rootPath is not a registered project root' } }; + } + const documentStore = createFlowDiagramDocumentStore({ + projectPath: rootPath, + notifyFileChange, + notifyDocumentChange: notifyFlowDiagramDocumentChange, + }); + return documentStore.saveDocument( + filePath, + content, + expectedVersion, + mutationId, + ); + }, + ); + typedHandle('fs:createFile', async (_, rootPath, filePath) => { if (!isRegisteredProjectRoot(rootPath)) { return { ok: false, error: { code: 'EACCES', message: 'rootPath is not a registered project root' } }; diff --git a/src/main/services/file-watcher.test.ts b/src/main/services/file-watcher.test.ts new file mode 100644 index 00000000..cc7b0cdf --- /dev/null +++ b/src/main/services/file-watcher.test.ts @@ -0,0 +1,45 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { send } = vi.hoisted(() => ({ send: vi.fn() })); + +vi.mock('electron', () => ({ + BrowserWindow: { + getAllWindows: () => [{ webContents: { send } }], + }, +})); + +import { notifyFileChange } from './file-watcher'; + +describe('file-watcher Flow Diagram notifications', () => { + let directory: string; + + beforeEach(() => { + vi.useFakeTimers(); + send.mockClear(); + directory = fs.mkdtempSync(path.join(os.tmpdir(), 'cdf-flow-watcher-')); + }); + + afterEach(() => { + vi.useRealTimers(); + fs.rmSync(directory, { recursive: true, force: true }); + }); + + it('publishes the current opaque version for an external diagram change', async () => { + const filePath = path.join(directory, 'diagram.excalidraw'); + fs.writeFileSync(filePath, '{"type":"excalidraw"}'); + + notifyFileChange(filePath); + await vi.advanceTimersByTimeAsync(200); + + expect(send).toHaveBeenCalledWith( + 'flow-diagram:document-change', + { + filePath, + version: expect.stringMatching(/^[a-f0-9]{64}$/), + }, + ); + }); +}); diff --git a/src/main/services/file-watcher.ts b/src/main/services/file-watcher.ts index b403ab0a..34aaccba 100644 --- a/src/main/services/file-watcher.ts +++ b/src/main/services/file-watcher.ts @@ -1,7 +1,11 @@ import { BrowserWindow } from 'electron'; +import fs from 'fs'; +import path from 'path'; import { typedSend } from '../typed-ipc'; import chokidar from 'chokidar'; import log from '../logger'; +import type { FlowDiagramDocumentChangeEvent } from '../../shared/flow-diagrams'; +import { hashBytes } from '../flow-diagram/flow-diagram-document-store'; const watchers = new Map>(); let currentRootPath: string | null = null; @@ -18,6 +22,17 @@ const IGNORED = [ let debounceTimer: ReturnType | null = null; const pendingEvents = new Map(); +export function notifyFlowDiagramDocumentChange( + event: FlowDiagramDocumentChangeEvent, +): void { + const windows = typeof BrowserWindow?.getAllWindows === 'function' + ? BrowserWindow.getAllWindows() + : []; + windows.forEach((window) => { + typedSend(window.webContents, 'flow-diagram:document-change', event); + }); +} + function flushEvents() { for (const [filePath, type] of pendingEvents) { const windows = typeof BrowserWindow?.getAllWindows === 'function' @@ -26,6 +41,15 @@ function flushEvents() { windows.forEach((w) => { typedSend(w.webContents, 'fs:directoryChange', { type, path: filePath }); }); + if (path.extname(filePath).toLowerCase() === '.excalidraw') { + let version: FlowDiagramDocumentChangeEvent['version'] = null; + try { + version = hashBytes(fs.readFileSync(filePath)); + } catch { + version = null; + } + notifyFlowDiagramDocumentChange({ filePath, version }); + } } pendingEvents.clear(); debounceTimer = null; diff --git a/src/preload/index.test.ts b/src/preload/index.test.ts index 9cc35c1f..9dc7e4a8 100644 --- a/src/preload/index.test.ts +++ b/src/preload/index.test.ts @@ -85,6 +85,7 @@ describe('preload bridge', () => { [() => api.capabilityJobs.onChanged, 'capability-jobs:changed'], [() => api.workflowRun.onProjectionEvent, 'workflow-run:projection-event'], [() => api.fs.onDirectoryChange, 'fs:directoryChange'], + [() => api.flowDiagram.onDocumentChange, 'flow-diagram:document-change'], [() => api.commands.onChanged, 'commands:changed'], [() => api.commands.onFallback, 'commands:fallback'], ]; @@ -114,6 +115,33 @@ describe('preload bridge', () => { expect(invokeMock).toHaveBeenCalledWith('workflow-run:get-tasks', 'run-1', 'stage-2'); }); + it('exposes only the versioned Flow Diagram load and save invokes', async () => { + const api = await loadApi(); + + await api.flowDiagram.loadDocument('/project', '/project/diagram.excalidraw'); + await api.flowDiagram.saveDocument( + '/project', + '/project/diagram.excalidraw', + '{"type":"excalidraw"}', + 'version-1', + 'mutation-1', + ); + + expect(invokeMock).toHaveBeenCalledWith( + 'flow-diagram:load-document', + '/project', + '/project/diagram.excalidraw', + ); + expect(invokeMock).toHaveBeenCalledWith( + 'flow-diagram:save-document', + '/project', + '/project/diagram.excalidraw', + '{"type":"excalidraw"}', + 'version-1', + 'mutation-1', + ); + }); + it('sends flow-diagram export responses through ipcRenderer.send', async () => { const api = await loadApi(); const response = { requestId: 'x', ok: true } as unknown; diff --git a/src/preload/index.ts b/src/preload/index.ts index 71467f43..2d32669a 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -33,6 +33,7 @@ import type { CapabilityJobAction } from '../shared/capability-jobs'; import { FLOW_DIAGRAM_EXPORT_REQUEST_CHANNEL, FLOW_DIAGRAM_EXPORT_RESPONSE_CHANNEL, + type FlowDiagramDocumentVersion, type FlowDiagramExportRequest, type FlowDiagramExportResponse, } from '../shared/flow-diagrams'; @@ -161,8 +162,25 @@ const api = { }, }, flowDiagram: { - saveDocument: (rootPath: string, filePath: string, content: string, expectedContent: string | null) => - typedInvoke('flow-diagram:save-document', rootPath, filePath, content, expectedContent), + loadDocument: (rootPath: string, filePath: string) => + typedInvoke('flow-diagram:load-document', rootPath, filePath), + saveDocument: ( + rootPath: string, + filePath: string, + content: string, + expectedVersion: FlowDiagramDocumentVersion, + mutationId?: string, + ) => typedInvoke( + 'flow-diagram:save-document', + rootPath, + filePath, + content, + expectedVersion, + mutationId, + ), + onDocumentChange: ( + callback: (data: IpcEventPayload<'flow-diagram:document-change'>) => void, + ) => typedOn('flow-diagram:document-change', callback), onExportRequest: (callback: (request: FlowDiagramExportRequest) => void) => { const listener = (_event: IpcRendererEvent, request: FlowDiagramExportRequest) => callback(request); ipcRenderer.on(FLOW_DIAGRAM_EXPORT_REQUEST_CHANNEL, listener); diff --git a/src/renderer/src/components/AgentLibrary/AgentEditDialog.test.tsx b/src/renderer/src/components/AgentLibrary/AgentEditDialog.test.tsx index ed39750f..729c3e6f 100644 --- a/src/renderer/src/components/AgentLibrary/AgentEditDialog.test.tsx +++ b/src/renderer/src/components/AgentLibrary/AgentEditDialog.test.tsx @@ -49,8 +49,8 @@ describe('AgentEditDialog', () => { fireEvent.click(screen.getByText('Manage Skill preload')); - expect(screen.getByRole('button', { name: /preload review/i })).toBeTruthy(); - expect(screen.queryByRole('button', { name: /preload project-review/i })).toBeNull(); + expect(screen.getByRole('checkbox', { name: /preload review/i })).toBeTruthy(); + expect(screen.queryByRole('checkbox', { name: /preload project-review/i })).toBeNull(); }); it('hydrates a new Custom Agent model after sources finish loading without resetting its draft', () => { @@ -115,7 +115,7 @@ describe('AgentEditDialog', () => { target: { value: 'Review Agent' }, }); fireEvent.click(screen.getByText('Manage Skill preload')); - fireEvent.click(screen.getByRole('button', { name: /preload review/i })); + fireEvent.click(screen.getByRole('checkbox', { name: /preload review/i })); fireEvent.click(screen.getByText('Save')); expect(createCustomAgent).toHaveBeenCalledWith(expect.objectContaining({ diff --git a/src/renderer/src/components/AgentLibrary/SkillPreloadSection.test.tsx b/src/renderer/src/components/AgentLibrary/SkillPreloadSection.test.tsx new file mode 100644 index 00000000..03896b9f --- /dev/null +++ b/src/renderer/src/components/AgentLibrary/SkillPreloadSection.test.tsx @@ -0,0 +1,130 @@ +import { useState } from 'react'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Skill } from '@shared/types'; +import i18n from '../../i18n'; +import { SkillPreloadSection } from './SkillPreloadSection'; + +const skills = [ + { + id: 'global:review', + name: 'review', + description: 'Review workflow', + scope: 'global', + sourceKind: 'user', + sourceLabel: 'Global Skill', + resourceFiles: [], + created_at: 0, + updated_at: 0, + }, + { + id: 'global:writer', + name: 'writer', + description: 'Writing workflow', + scope: 'global', + sourceKind: 'managed', + sourceLabel: 'Managed Skill', + resourceFiles: [], + created_at: 0, + updated_at: 0, + }, +] as Skill[]; + +function SkillPreloadHarness({ onToggle = vi.fn() }: { onToggle?: (skillId: string) => void }) { + const [selectedSkillIds, setSelectedSkillIds] = useState([]); + const handleToggle = (skillId: string) => { + setSelectedSkillIds((current) => ( + current.includes(skillId) + ? current.filter((id) => id !== skillId) + : [...current, skillId] + )); + onToggle(skillId); + }; + + return ( +
+ + +
+ ); +} + +beforeEach(async () => { + await i18n.changeLanguage('en-US'); +}); + +describe('SkillPreloadSection', () => { + it('opens from the keyboard and moves focus into the search field', async () => { + const user = userEvent.setup(); + render(); + const trigger = screen.getByRole('button', { name: 'Manage Skill preload' }); + trigger.focus(); + + await user.keyboard('{Enter}'); + + expect(screen.getByPlaceholderText('Search skills...')).toBe(document.activeElement); + }); + + it('selects and clears a Skill with Enter and Space while exposing checked state', async () => { + const user = userEvent.setup(); + const onToggle = vi.fn(); + render(); + await user.click(screen.getByRole('button', { name: 'Manage Skill preload' })); + const candidate = screen.getByRole('checkbox', { name: 'Preload review' }); + candidate.focus(); + + await user.keyboard('{Enter}'); + expect(screen.getByRole('checkbox', { name: 'Preload review' }).getAttribute('aria-checked')).toBe('true'); + + await user.keyboard(' '); + expect(screen.getByRole('checkbox', { name: 'Preload review' }).getAttribute('aria-checked')).toBe('false'); + expect(onToggle).toHaveBeenNthCalledWith(1, 'global:review'); + expect(onToggle).toHaveBeenNthCalledWith(2, 'global:review'); + }); + + it('closes with Escape and restores focus to the trigger', async () => { + const user = userEvent.setup(); + render(); + const trigger = screen.getByRole('button', { name: 'Manage Skill preload' }); + await user.click(trigger); + + await user.keyboard('{Escape}'); + + expect(screen.queryByPlaceholderText('Search skills...')).toBeNull(); + expect(document.activeElement).toBe(trigger); + }); + + it('closes on outside click and clears the search before reopening', async () => { + const user = userEvent.setup(); + render(); + const trigger = screen.getByRole('button', { name: 'Manage Skill preload' }); + await user.click(trigger); + await user.type(screen.getByPlaceholderText('Search skills...'), 'review'); + + await user.click(screen.getByRole('button', { name: 'Outside target' })); + expect(screen.queryByPlaceholderText('Search skills...')).toBeNull(); + + await user.click(trigger); + expect((screen.getByPlaceholderText('Search skills...') as HTMLInputElement).value).toBe(''); + }); + + it('filters candidates and reports an empty result', async () => { + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole('button', { name: 'Manage Skill preload' })); + const search = screen.getByPlaceholderText('Search skills...'); + + await user.type(search, 'writer'); + expect(screen.getByRole('checkbox', { name: 'Preload writer' })).toBeTruthy(); + expect(screen.queryByRole('checkbox', { name: 'Preload review' })).toBeNull(); + + await user.clear(search); + await user.type(search, 'missing'); + expect(screen.getByText('No matching skills found')).toBeTruthy(); + }); +}); diff --git a/src/renderer/src/components/AgentLibrary/SkillPreloadSection.tsx b/src/renderer/src/components/AgentLibrary/SkillPreloadSection.tsx index 26fd0daf..e3645c50 100644 --- a/src/renderer/src/components/AgentLibrary/SkillPreloadSection.tsx +++ b/src/renderer/src/components/AgentLibrary/SkillPreloadSection.tsx @@ -1,7 +1,8 @@ -import { useEffect, useRef, useState } from 'react'; +import { useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { Plus, Search } from 'lucide-react'; +import { Check, Plus, Search } from 'lucide-react'; import type { Skill } from '@shared/types'; +import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'; interface SkillPreloadSectionProps { skills: Skill[]; @@ -16,30 +17,19 @@ function getSkillDisplayName(skill: { name: string; qualifiedName?: string | nul /** * Skill preload field group: selected-skill chips plus a searchable dropdown of * Global Skill candidates. Owns its dropdown/search state; closing the dropdown - * (toggle or click outside) always clears the search query. + * always clears the search query. */ export function SkillPreloadSection({ skills, selectedSkillIds, onToggleSkill }: SkillPreloadSectionProps) { const { t } = useTranslation(); const [dropdownOpen, setDropdownOpenRaw] = useState(false); const [searchQuery, setSearchQuery] = useState(''); - const containerRef = useRef(null); + const searchInputRef = useRef(null); const setDropdownOpen = (open: boolean) => { setDropdownOpenRaw(open); if (!open) setSearchQuery(''); }; - useEffect(() => { - function handleClickOutside(event: MouseEvent) { - if (containerRef.current && !containerRef.current.contains(event.target as Node)) { - setDropdownOpenRaw(false); - setSearchQuery(''); - } - } - document.addEventListener('mousedown', handleClickOutside); - return () => document.removeEventListener('mousedown', handleClickOutside); - }, []); - const candidates = skills.filter(sk => { if (sk.scope !== 'global') return false; const query = searchQuery.toLowerCase(); @@ -52,7 +42,7 @@ export function SkillPreloadSection({ skills, selectedSkillIds, onToggleSkill }: skill.sourceLabel || (skill.scope === 'project' ? t('agent.skillSourceProject') : t('agent.skillSourceGlobal')); return ( -
+
- - - {dropdownOpen && ( -
+ + + + + { + event.preventDefault(); + searchInputRef.current?.focus(); + }} + className="w-[var(--radix-popover-trigger-width)] max-h-[220px] p-2 select-none flex flex-col gap-1" + >
setSearchQuery(e.target.value)} className="bg-transparent text-xs text-[var(--color-text-primary)] outline-none w-full py-0.5" - onClick={(e) => e.stopPropagation()} />
@@ -115,38 +112,50 @@ export function SkillPreloadSection({ skills, selectedSkillIds, onToggleSkill }: const sourceLabel = getSkillSourceLabel(sk); const isBound = selectedSkillIds.includes(sk.id); return ( -
onToggleSkill(sk.id)} - className={`flex items-center justify-between px-2.5 py-1.5 rounded-md text-xs cursor-pointer transition-colors ${ + onKeyDown={(event) => { + if (event.key === ' ') { + event.preventDefault(); + onToggleSkill(sk.id); + } + }} + className={`w-full flex items-center justify-between px-2.5 py-1.5 rounded-md text-xs cursor-pointer transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[var(--color-accent)] ${ isBound ? 'bg-[var(--color-success-dim)]/20 text-[var(--color-success)] font-medium' : 'text-[var(--color-text-secondary)] hover:bg-[var(--color-bg-hover)] hover:text-[var(--color-text-primary)]' }`} > -
- + + {displayName} -
+ {sourceLabel} -
+ ); })} {candidates.length === 0 && (
{t('agent.noSkillMatch')}
)}
-
- )} + +
); } diff --git a/src/renderer/src/components/FilePanel/EditorPane.tsx b/src/renderer/src/components/FilePanel/EditorPane.tsx index a55fa82a..bb1750d8 100644 --- a/src/renderer/src/components/FilePanel/EditorPane.tsx +++ b/src/renderer/src/components/FilePanel/EditorPane.tsx @@ -6,6 +6,7 @@ import { useFileStore } from '../../stores/fileStore'; import { MarkdownRenderer } from '../ChatArea/MarkdownRenderer'; import { isFlowDiagramFile } from '../../lib/flowDiagramFile'; import { FileTypeIcon } from './FileTypeIcon'; +import type { FlowDiagramDocumentVersion } from '@shared/flow-diagrams'; const MonacoEditor = lazy(() => import('@monaco-editor/react')); const FlowDiagramEditor = lazy(() => @@ -35,10 +36,17 @@ interface EditorPaneProps { filePath: string; fileName: string; content: string; + documentVersion?: FlowDiagramDocumentVersion; loadError?: 'unreadable'; } -export function EditorPane({ filePath, fileName, content, loadError }: EditorPaneProps) { +export function EditorPane({ + filePath, + fileName, + content, + documentVersion, + loadError, +}: EditorPaneProps) { const language = useMemo(() => detectLanguage(fileName), [fileName]); const isMd = language === 'markdown'; const isFlowDiagram = isFlowDiagramFile(fileName); @@ -201,6 +209,7 @@ export function EditorPane({ filePath, fileName, content, loadError }: EditorPan content={content} fileName={fileName} filePath={filePath} + documentVersion={documentVersion} loadError={loadError} /> diff --git a/src/renderer/src/components/FilePanel/FilePanel.test.tsx b/src/renderer/src/components/FilePanel/FilePanel.test.tsx index 63063f3d..76db524d 100644 --- a/src/renderer/src/components/FilePanel/FilePanel.test.tsx +++ b/src/renderer/src/components/FilePanel/FilePanel.test.tsx @@ -25,6 +25,10 @@ const DIAGRAM_CONTENT = JSON.stringify({ appState: { viewBackgroundColor: '#f8f9fa' }, files: {}, }); +type SaveSuccess = { + ok: true; + document: { content: string; version: string }; +}; const { excalidrawProps, editCount, loadFromBlob, serializeAsJSON } = vi.hoisted(() => ({ excalidrawProps: { current: null as Record | null }, @@ -106,8 +110,10 @@ vi.mock('@excalidraw/excalidraw', () => ({ const readFile = vi.fn(); const writeFile = vi.fn(); +const fsReadFile = vi.fn(); +const fsWriteFile = vi.fn(); const directoryChangeListeners: Array<( - data: { type: string; path: string }, + data: { filePath: string; version: string | null; mutationId?: string }, ) => void> = []; beforeAll(() => { @@ -137,26 +143,41 @@ beforeEach(() => { serializeAsJSON.mockClear(); readFile.mockReset().mockResolvedValue({ ok: true, - data: { content: DIAGRAM_CONTENT }, + document: { content: DIAGRAM_CONTENT, version: 'version-1' }, }); - writeFile.mockReset().mockResolvedValue({ ok: true, data: undefined }); + let savedVersion = 1; + writeFile.mockReset().mockImplementation(async ( + _rootPath: string, + _filePath: string, + savedContent: string, + ) => ({ + ok: true, + document: { + content: savedContent, + version: `version-${++savedVersion}`, + }, + })); + fsReadFile.mockReset(); + fsWriteFile.mockReset(); directoryChangeListeners.length = 0; (window as unknown as { electronAPI: unknown }).electronAPI = { store: { get: vi.fn().mockResolvedValue(false) }, - // 流程图 autosave 走文档存储通道 (#200)；参数形状与 fs.writeFile 的 - // CAS 版本一致（rootPath, filePath, content, expectedContent），共用断言。 - flowDiagram: { saveDocument: writeFile }, + flowDiagram: { + loadDocument: readFile, + saveDocument: writeFile, + onDocumentChange: vi.fn((callback) => { + directoryChangeListeners.push(callback); + return vi.fn(); + }), + }, fs: { readDirectory: vi.fn().mockResolvedValue({ ok: true, data: [] }), - readFile, - writeFile, + readFile: fsReadFile, + writeFile: fsWriteFile, watchDirectory: vi.fn().mockResolvedValue({ ok: true, data: undefined }), unwatchDirectory: vi.fn().mockResolvedValue({ ok: true, data: undefined }), - onDirectoryChange: vi.fn((callback) => { - directoryChangeListeners.push(callback); - return vi.fn(); - }), + onDirectoryChange: vi.fn(() => vi.fn()), }, }; @@ -218,6 +239,7 @@ describe('Editable Flow Diagram workspace', () => { await screen.findByTestId('official-excalidraw'); expect(readFile).toHaveBeenCalledTimes(1); + expect(fsReadFile).not.toHaveBeenCalled(); expect(useFileStore.getState().openTabs).toHaveLength(1); expect(useFileStore.getState().previewFile?.path).toBe(DIAGRAM_PATH); await waitFor(() => expect(excalidrawProps.current?.initialData).toMatchObject({ @@ -254,17 +276,20 @@ describe('Editable Flow Diagram workspace', () => { loadFromBlob.mockResolvedValue(agentDiagram); readFile.mockResolvedValue({ ok: true, - data: { content: JSON.stringify({ - type: 'excalidraw', - version: 2, - source: 'https://cdf.local', - ...agentDiagram, - }) }, + document: { + content: JSON.stringify({ + type: 'excalidraw', + version: 2, + source: 'https://cdf.local', + ...agentDiagram, + }), + version: 'version-agent-1', + }, }); await act(async () => { for (const listener of directoryChangeListeners) { - listener({ type: 'change', path: DIAGRAM_PATH }); + listener({ filePath: DIAGRAM_PATH, version: 'version-agent-1' }); } }); @@ -284,6 +309,7 @@ describe('Editable Flow Diagram workspace', () => { path: `${PROJECT_PATH}/diagrams/second.excalidraw`, name: 'second.excalidraw', content: DIAGRAM_CONTENT, + documentVersion: 'version-second' as never, }); useFileStore.getState().setActiveTab(0); }); @@ -298,17 +324,20 @@ describe('Editable Flow Diagram workspace', () => { loadFromBlob.mockResolvedValue(agentDiagram); readFile.mockResolvedValue({ ok: true, - data: { content: JSON.stringify({ - type: 'excalidraw', - version: 2, - source: 'https://cdf.local', - ...agentDiagram, - }) }, + document: { + content: JSON.stringify({ + type: 'excalidraw', + version: 2, + source: 'https://cdf.local', + ...agentDiagram, + }), + version: 'version-agent-1', + }, }); await act(async () => { for (const listener of directoryChangeListeners) { - listener({ type: 'change', path: DIAGRAM_PATH }); + listener({ filePath: DIAGRAM_PATH, version: 'version-agent-1' }); } await Promise.resolve(); }); @@ -380,7 +409,8 @@ describe('Editable Flow Diagram workspace', () => { PROJECT_PATH, DIAGRAM_PATH, expect.any(String), - DIAGRAM_CONTENT, + 'version-1', + expect.stringMatching(/^flow-diagram-/), ); const savedDocument = JSON.parse(writeFile.mock.calls[0][2]); expect(useFileStore.getState().previewFile?.content).toBe(writeFile.mock.calls[0][2]); @@ -404,8 +434,134 @@ describe('Editable Flow Diagram workspace', () => { expect(useFileStore.getState().dirtyTabs[DIAGRAM_PATH]).toBe(false); }); + it('uses a save conflict snapshot directly and restores against its latest version', async () => { + writeFile.mockResolvedValueOnce({ + ok: false, + error: { + code: 'SOURCE_CHANGED', + message: 'changed', + currentContent: DIAGRAM_CONTENT, + currentVersion: 'version-conflict', + }, + }); + render(
); + fireEvent.click(screen.getByRole('button', { name: 'release.excalidraw' })); + await screen.findByTestId('official-excalidraw'); + vi.useFakeTimers(); + + fireEvent.click(screen.getByRole('button', { name: 'Draw in diagram' })); + await act(async () => { + await vi.advanceTimersByTimeAsync(1000); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(screen.getByRole('button', { + name: /恢复我的编辑|Restore my edits/, + })).toBeTruthy(); + expect(readFile).toHaveBeenCalledTimes(1); + expect(fsReadFile).not.toHaveBeenCalled(); + + fireEvent.click(screen.getByRole('button', { + name: /恢复我的编辑|Restore my edits/, + })); + await act(async () => { + await vi.advanceTimersByTimeAsync(1000); + }); + + expect(writeFile).toHaveBeenCalledTimes(2); + expect(writeFile.mock.calls[1][3]).toBe('version-conflict'); + expect(useFileStore.getState().dirtyTabs[DIAGRAM_PATH]).toBe(false); + }); + + it('ignores the versioned notification produced by its own active save', async () => { + writeFile.mockImplementationOnce(async ( + _rootPath: string, + _filePath: string, + savedContent: string, + _expectedVersion: string, + mutationId: string, + ) => { + for (const listener of directoryChangeListeners) { + listener({ + filePath: DIAGRAM_PATH, + version: 'version-2', + mutationId, + }); + } + return { + ok: true, + document: { content: savedContent, version: 'version-2' }, + }; + }); + render(
); + fireEvent.click(screen.getByRole('button', { name: 'release.excalidraw' })); + await screen.findByTestId('official-excalidraw'); + vi.useFakeTimers(); + + fireEvent.click(screen.getByRole('button', { name: 'Draw in diagram' })); + await act(async () => { + await vi.advanceTimersByTimeAsync(1000); + }); + + expect(readFile).toHaveBeenCalledTimes(1); + expect(screen.queryByRole('button', { + name: /恢复我的编辑|Restore my edits/, + })).toBeNull(); + expect(useFileStore.getState().dirtyTabs[DIAGRAM_PATH]).toBe(false); + }); + + it('does not regress when versioned external notifications resolve out of order', async () => { + let resolveOlder: ((value: unknown) => void) | undefined; + let resolveLatest: ((value: unknown) => void) | undefined; + render(
); + fireEvent.click(screen.getByRole('button', { name: 'release.excalidraw' })); + await screen.findByTestId('official-excalidraw'); + + readFile + .mockImplementationOnce(() => new Promise((resolve) => { + resolveOlder = resolve; + })) + .mockImplementationOnce(() => new Promise((resolve) => { + resolveLatest = resolve; + })); + const latestContent = DIAGRAM_CONTENT.replace('"elements":[]', '"elements":[{"id":"latest"}]'); + const olderContent = DIAGRAM_CONTENT.replace('"elements":[]', '"elements":[{"id":"older"}]'); + + act(() => { + for (const listener of directoryChangeListeners) { + listener({ filePath: DIAGRAM_PATH, version: 'version-older' }); + listener({ filePath: DIAGRAM_PATH, version: 'version-latest' }); + } + }); + await act(async () => { + resolveLatest?.({ + ok: true, + document: { content: latestContent, version: 'version-latest' }, + }); + await Promise.resolve(); + await Promise.resolve(); + }); + await act(async () => { + resolveOlder?.({ + ok: true, + document: { content: olderContent, version: 'version-older' }, + }); + await Promise.resolve(); + await Promise.resolve(); + }); + + await waitFor(() => { + expect(useFileStore.getState().previewFile).toMatchObject({ + content: latestContent, + documentVersion: 'version-latest', + }); + }); + expect(useFileStore.getState().previewFile?.content).not.toBe(olderContent); + }); + it('serializes overlapping saves so an older write cannot replace the latest edit', async () => { - let resolveFirstWrite: ((value: { ok: true; data: undefined }) => void) | undefined; + let resolveFirstWrite: ((value: SaveSuccess) => void) | undefined; writeFile.mockImplementationOnce(() => new Promise((resolve) => { resolveFirstWrite = resolve; })); @@ -429,7 +585,10 @@ describe('Editable Flow Diagram workspace', () => { expect(writeFile).toHaveBeenCalledTimes(1); await act(async () => { - resolveFirstWrite?.({ ok: true, data: undefined }); + resolveFirstWrite?.({ + ok: true, + document: { content: writeFile.mock.calls[0][2], version: 'version-2' }, + }); await Promise.resolve(); await Promise.resolve(); }); @@ -476,8 +635,8 @@ describe('Editable Flow Diagram workspace', () => { }); it('waits for edits made while close-time flush is still in progress', async () => { - let resolveFirstWrite: ((value: { ok: true; data: undefined }) => void) | undefined; - let resolveSecondWrite: ((value: { ok: true; data: undefined }) => void) | undefined; + let resolveFirstWrite: ((value: SaveSuccess) => void) | undefined; + let resolveSecondWrite: ((value: SaveSuccess) => void) | undefined; writeFile .mockImplementationOnce(() => new Promise((resolve) => { resolveFirstWrite = resolve; @@ -500,7 +659,10 @@ describe('Editable Flow Diagram workspace', () => { fireEvent.click(drawButton); await act(async () => { - resolveFirstWrite?.({ ok: true, data: undefined }); + resolveFirstWrite?.({ + ok: true, + document: { content: writeFile.mock.calls[0][2], version: 'version-2' }, + }); await Promise.resolve(); await Promise.resolve(); }); @@ -508,7 +670,10 @@ describe('Editable Flow Diagram workspace', () => { expect(useFileStore.getState().openTabs).toHaveLength(1); await act(async () => { - resolveSecondWrite?.({ ok: true, data: undefined }); + resolveSecondWrite?.({ + ok: true, + document: { content: writeFile.mock.calls[1][2], version: 'version-3' }, + }); await Promise.resolve(); await Promise.resolve(); }); @@ -537,7 +702,10 @@ describe('Editable Flow Diagram workspace', () => { it('shows a read-only error without overwriting invalid or unreadable diagrams', async () => { loadFromBlob.mockRejectedValueOnce(new Error('invalid document')); - readFile.mockResolvedValueOnce({ ok: true, data: { content: '{not-json' } }); + readFile.mockResolvedValueOnce({ + ok: true, + document: { content: '{not-json', version: 'version-invalid' }, + }); const { unmount } = render(
); fireEvent.click(screen.getByRole('button', { name: 'release.excalidraw' })); diff --git a/src/renderer/src/components/FilePanel/FilePanel.tsx b/src/renderer/src/components/FilePanel/FilePanel.tsx index 72bf9250..5b35a90f 100644 --- a/src/renderer/src/components/FilePanel/FilePanel.tsx +++ b/src/renderer/src/components/FilePanel/FilePanel.tsx @@ -293,6 +293,7 @@ export function FilePanel() { filePath={previewFile.path} fileName={previewFile.name} content={previewFile.content} + documentVersion={previewFile.documentVersion} loadError={previewFile.loadError} />
state.theme); const rootPath = useFileStore((state) => state.rootPath); @@ -49,17 +70,14 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl const [diagramRevision, setDiagramRevision] = useState(0); const [conflictedContent, setConflictedContent] = useState(null); const conflictedContentRef = useRef(null); - const editGenerationRef = useRef(0); const saveTimerRef = useRef | null>(null); - const lastSavedContentRef = useRef(null); - const lastDiskContentRef = useRef(null); + const baselineContentRef = useRef(null); + const documentVersionRef = useRef(null); const pendingContentRef = useRef(null); - const saveQueueRef = useRef>(Promise.resolve(true)); - const lastQueuedContentRef = useRef(null); - const queuedDiskContentRef = useRef(null); - const externalReloadVersionRef = useRef(0); - const externalReloadPromiseRef = useRef | null>(null); - const externalPendingPreservationRef = useRef(null); + const saveOperationRef = useRef | null>(null); + const savingContentRef = useRef(null); + const activeMutationIdRef = useRef(null); + const latestNotifiedVersionRef = useRef(null); useEffect(() => { const mediaQuery = window.matchMedia('(prefers-color-scheme: dark)'); @@ -68,72 +86,144 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl return () => mediaQuery.removeEventListener('change', handleChange); }, []); - const persist = useCallback((contentToSave: string): Promise => { - if (!rootPath) return Promise.resolve(false); - if (lastQueuedContentRef.current === contentToSave) { - return saveQueueRef.current; + const installAuthoritativeSnapshot = useCallback(async ( + snapshot: FlowDiagramDocumentSnapshot, + preservedLocalContent: string | null, + notificationGuard?: FlowDiagramDocumentVersion | null, + ): Promise => { + try { + const diagram = await restoreFlowDiagram(snapshot.content); + if ( + notificationGuard !== undefined + && latestNotifiedVersionRef.current !== notificationGuard + ) { + return true; + } + const restoredContent = serializeFlowDiagram( + diagram.elements, + diagram.appState, + diagram.files, + ); + if (saveTimerRef.current) { + clearTimeout(saveTimerRef.current); + saveTimerRef.current = null; + } + pendingContentRef.current = null; + baselineContentRef.current = restoredContent; + documentVersionRef.current = snapshot.version; + + const hasConflict = Boolean( + preservedLocalContent && preservedLocalContent !== restoredContent, + ); + const nextConflict = hasConflict ? preservedLocalContent : null; + conflictedContentRef.current = nextConflict; + setConflictedContent(nextConflict); + setTabContent(filePath, snapshot.content, snapshot.version); + setTabDirty(filePath, hasConflict); + setSaveState(hasConflict ? 'dirty' : 'saved'); + setDiagramRevision((current) => current + 1); + setLoadState({ status: 'ready', diagram }); + return !hasConflict; + } catch { + if ( + notificationGuard !== undefined + && latestNotifiedVersionRef.current !== notificationGuard + ) { + return true; + } + if (preservedLocalContent && !conflictedContentRef.current) { + conflictedContentRef.current = preservedLocalContent; + setConflictedContent(preservedLocalContent); + } + setTabDirty(filePath, conflictedContentRef.current !== null); + setSaveState(conflictedContentRef.current ? 'dirty' : 'error'); + setLoadState({ status: 'invalid', reason: 'invalid' }); + return false; } + }, [filePath, setTabContent, setTabDirty]); - const expectedDiskContent = queuedDiskContentRef.current ?? lastDiskContentRef.current ?? null; - lastQueuedContentRef.current = contentToSave; - queuedDiskContentRef.current = contentToSave; - const operation = saveQueueRef.current.then(async () => { - setSaveState('saving'); - try { - const result = await window.electronAPI.flowDiagram.saveDocument( - rootPath, - filePath, - contentToSave, - expectedDiskContent, - ); - if (!result.ok) { - if (result.error.code === FLOW_DIAGRAM_SOURCE_CHANGED) { - // The document changed externally: the store returns the current - // on-disk content so we can relink our CAS baseline without a - // second read, then surface the unsaved attempt as a conflict. - const currentContent = result.error.currentContent ?? null; - if (currentContent != null) lastDiskContentRef.current = currentContent; - if (!conflictedContentRef.current) { - conflictedContentRef.current = contentToSave; - setConflictedContent(contentToSave); + const persistPending = useCallback((): Promise => { + if (!rootPath) return Promise.resolve(false); + if (saveOperationRef.current) return saveOperationRef.current; + + const operation = (async () => { + while (pendingContentRef.current && !conflictedContentRef.current) { + const contentToSave = pendingContentRef.current; + const expectedVersion = documentVersionRef.current; + if (!expectedVersion) { + setSaveState('error'); + return false; + } + + const mutationId = nextMutationId(); + savingContentRef.current = contentToSave; + activeMutationIdRef.current = mutationId; + setSaveState('saving'); + + try { + const result = await window.electronAPI.flowDiagram.saveDocument( + rootPath, + filePath, + contentToSave, + expectedVersion, + mutationId, + ); + if (!result.ok) { + if (result.error.code === FLOW_DIAGRAM_SOURCE_CHANGED) { + const preserved = pendingContentRef.current ?? contentToSave; + pendingContentRef.current = null; + if (result.error.currentContent != null && result.error.currentVersion) { + await installAuthoritativeSnapshot( + { + content: result.error.currentContent, + version: result.error.currentVersion, + }, + preserved, + ); + } else { + conflictedContentRef.current = preserved; + setConflictedContent(preserved); + setTabDirty(filePath, true); + setSaveState('dirty'); + } + return false; } - setTabDirty(filePath, true); - setSaveState('dirty'); + console.error('[FlowDiagramEditor] Save failed:', result.error.message); + setSaveState('error'); return false; } - console.error('[FlowDiagramEditor] Save failed:', result.error.message); + + documentVersionRef.current = result.document.version; + baselineContentRef.current = contentToSave; + setTabContent( + filePath, + result.document.content, + result.document.version, + ); + if (pendingContentRef.current === contentToSave) { + pendingContentRef.current = null; + } + const isDirty = pendingContentRef.current !== null; + setTabDirty(filePath, isDirty); + setSaveState(isDirty ? 'dirty' : 'saved'); + } catch (error) { + console.error('[FlowDiagramEditor] Save error:', error); setSaveState('error'); return false; + } finally { + savingContentRef.current = null; + activeMutationIdRef.current = null; } - - lastSavedContentRef.current = contentToSave; - lastDiskContentRef.current = contentToSave; - setTabContent(filePath, contentToSave); - if (pendingContentRef.current === contentToSave) { - pendingContentRef.current = null; - const hasConflict = conflictedContentRef.current !== null; - setTabDirty(filePath, hasConflict); - setSaveState(hasConflict ? 'dirty' : 'saved'); - } else { - setSaveState('dirty'); - } - return true; - } catch (error) { - console.error('[FlowDiagramEditor] Save error:', error); - setSaveState('error'); - return false; } - }); + return conflictedContentRef.current === null; + })(); - saveQueueRef.current = operation; + saveOperationRef.current = operation; void operation.finally(() => { - if (lastQueuedContentRef.current === contentToSave) { - lastQueuedContentRef.current = null; - queuedDiskContentRef.current = null; - } + if (saveOperationRef.current === operation) saveOperationRef.current = null; }); return operation; - }, [filePath, rootPath, setTabContent, setTabDirty]); + }, [filePath, installAuthoritativeSnapshot, rootPath, setTabContent, setTabDirty]); const scheduleSave = useCallback((serialized: string) => { pendingContentRef.current = serialized; @@ -142,28 +232,21 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl if (saveTimerRef.current) clearTimeout(saveTimerRef.current); saveTimerRef.current = setTimeout(() => { saveTimerRef.current = null; - void persist(serialized); + void persistPending(); }, AUTOSAVE_DELAY_MS); - }, [filePath, persist, setTabDirty]); + }, [filePath, persistPending, setTabDirty]); const flushPendingSave = useCallback(async (): Promise => { - while (externalReloadPromiseRef.current) { - const reload = externalReloadPromiseRef.current; - if (!await reload) return false; - if (externalReloadPromiseRef.current === reload) externalReloadPromiseRef.current = null; + if (saveTimerRef.current) { + clearTimeout(saveTimerRef.current); + saveTimerRef.current = null; } if (conflictedContentRef.current) return false; - while (true) { - if (saveTimerRef.current) { - clearTimeout(saveTimerRef.current); - saveTimerRef.current = null; - } - const pending = pendingContentRef.current; - const saved = await (pending ? persist(pending) : saveQueueRef.current); - if (!saved) return false; - if (!pendingContentRef.current) return true; + while (pendingContentRef.current || saveOperationRef.current) { + if (!await persistPending()) return false; } - }, [persist]); + return true; + }, [persistPending]); const handleChange = useCallback(( elements: FlowDiagramElements, @@ -171,29 +254,35 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl files: FlowDiagramFiles, ) => { const serialized = serializeFlowDiagram(elements, appState, files); + if (serialized === pendingContentRef.current) return; if ( - serialized === lastSavedContentRef.current - || serialized === pendingContentRef.current + serialized === baselineContentRef.current + && savingContentRef.current === null ) { + if (saveTimerRef.current) { + clearTimeout(saveTimerRef.current); + saveTimerRef.current = null; + } + pendingContentRef.current = null; + setTabDirty(filePath, false); + setSaveState('saved'); return; } - editGenerationRef.current += 1; scheduleSave(serialized); - }, [scheduleSave]); + }, [filePath, scheduleSave, setTabDirty]); useEffect(() => { let cancelled = false; - if (lastSavedContentRef.current !== null && lastDiskContentRef.current === content) { + if (loadError || !documentVersion) { + setLoadState({ status: 'invalid', reason: 'unreadable' }); return () => { cancelled = true; }; } - lastSavedContentRef.current = null; - lastDiskContentRef.current = content; - pendingContentRef.current = null; - - if (loadError) { - setLoadState({ status: 'invalid', reason: 'unreadable' }); + if ( + baselineContentRef.current !== null + && documentVersionRef.current === documentVersion + ) { return () => { cancelled = true; }; @@ -203,11 +292,14 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl restoreFlowDiagram(content) .then((diagram) => { if (cancelled) return; - lastSavedContentRef.current = serializeFlowDiagram( + baselineContentRef.current = serializeFlowDiagram( diagram.elements, diagram.appState, diagram.files, ); + documentVersionRef.current = documentVersion; + pendingContentRef.current = null; + setTabDirty(filePath, false); setSaveState('saved'); setDiagramRevision((current) => current + 1); setLoadState({ status: 'ready', diagram }); @@ -219,103 +311,64 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl return () => { cancelled = true; }; - }, [content, loadError]); + }, [content, documentVersion, filePath, loadError, setTabDirty]); useEffect(() => { if (!rootPath) return; - const unsubscribe = window.electronAPI.fs.onDirectoryChange((data) => { - if (data.path.replace(/\\/g, '/') !== filePath.replace(/\\/g, '/')) return; - const version = ++externalReloadVersionRef.current; - const previousReload = externalReloadPromiseRef.current ?? Promise.resolve(true); - const reload = previousReload.then(async () => { - if (externalReloadVersionRef.current !== version) return true; - const generationAtNotification = editGenerationRef.current; - const pendingAtNotification = pendingContentRef.current; - if (pendingAtNotification) { - externalPendingPreservationRef.current ??= pendingAtNotification; - } - if (saveTimerRef.current) { - clearTimeout(saveTimerRef.current); - saveTimerRef.current = null; - } - pendingContentRef.current = null; - await saveQueueRef.current; - const result = await window.electronAPI.fs.readFile(rootPath, filePath); - if (externalReloadVersionRef.current !== version) return true; - if (!result.ok || 'binary' in result.data) { - const preserved = pendingContentRef.current ?? externalPendingPreservationRef.current; - externalPendingPreservationRef.current = null; - pendingContentRef.current = null; + const unsubscribe = window.electronAPI.flowDiagram.onDocumentChange((event) => { + if (normalizedPath(event.filePath) !== normalizedPath(filePath)) return; + if (event.mutationId && event.mutationId === activeMutationIdRef.current) return; + if (event.version && event.version === documentVersionRef.current) return; + + latestNotifiedVersionRef.current = event.version; + const notificationVersion = event.version; + void (async () => { + const result = await window.electronAPI.flowDiagram.loadDocument(rootPath, filePath); + if (latestNotifiedVersionRef.current !== notificationVersion) return; + if (!result.ok) { + const preserved = ( + conflictedContentRef.current + ?? pendingContentRef.current + ?? savingContentRef.current + ); if (preserved && !conflictedContentRef.current) { conflictedContentRef.current = preserved; setConflictedContent(preserved); } + pendingContentRef.current = null; setTabDirty(filePath, conflictedContentRef.current !== null); setSaveState(conflictedContentRef.current ? 'dirty' : 'error'); - return false; + setLoadState({ status: 'invalid', reason: 'unreadable' }); + return; } - try { - const diagram = await restoreFlowDiagram(result.data.content); - const restoredContent = serializeFlowDiagram( - diagram.elements, - diagram.appState, - diagram.files, - ); - const pendingAfterWait = pendingContentRef.current; - const editedWhileWaiting = editGenerationRef.current !== generationAtNotification; - if (restoredContent === lastSavedContentRef.current) { - const preserved = pendingAfterWait ?? externalPendingPreservationRef.current; - externalPendingPreservationRef.current = null; - if (preserved && preserved !== restoredContent) scheduleSave(preserved); - return true; - } + if (result.document.version === documentVersionRef.current) return; - const preservedContent = editedWhileWaiting - ? (pendingAfterWait ?? externalPendingPreservationRef.current) - : (pendingAtNotification ?? externalPendingPreservationRef.current); - if (saveTimerRef.current) { - clearTimeout(saveTimerRef.current); - saveTimerRef.current = null; - } - pendingContentRef.current = null; - externalPendingPreservationRef.current = null; - if (preservedContent && preservedContent !== restoredContent && !conflictedContentRef.current) { - conflictedContentRef.current = preservedContent; - setConflictedContent(preservedContent); - } - - setLoadState({ status: 'loading' }); - lastSavedContentRef.current = restoredContent; - lastDiskContentRef.current = result.data.content; - setSaveState(conflictedContentRef.current ? 'dirty' : 'saved'); - setTabDirty(filePath, conflictedContentRef.current !== null); - setTabContent(filePath, result.data.content); - setDiagramRevision((current) => current + 1); - setLoadState({ status: 'ready', diagram }); - return conflictedContentRef.current === null; - } catch { - const preserved = pendingContentRef.current ?? externalPendingPreservationRef.current; - externalPendingPreservationRef.current = null; - pendingContentRef.current = null; - if (preserved && !conflictedContentRef.current) { - conflictedContentRef.current = preserved; - setConflictedContent(preserved); - } - setTabDirty(filePath, conflictedContentRef.current !== null); - setLoadState({ status: 'invalid', reason: 'invalid' }); - return false; + const savingContent = savingContentRef.current; + if (savingContent && result.document.content === savingContent) { + documentVersionRef.current = result.document.version; + baselineContentRef.current = savingContent; + setTabContent( + filePath, + result.document.content, + result.document.version, + ); + return; } - }); - externalReloadPromiseRef.current = reload; - void reload.finally(() => { - if (externalReloadPromiseRef.current === reload) externalReloadPromiseRef.current = null; - }); + + const preserved = ( + conflictedContentRef.current + ?? pendingContentRef.current + ?? savingContent + ); + await installAuthoritativeSnapshot( + result.document, + preserved, + notificationVersion, + ); + })(); }); - return () => { - externalReloadVersionRef.current += 1; - unsubscribe(); - }; - }, [filePath, rootPath, scheduleSave, setTabContent, setTabDirty]); + return unsubscribe; + }, [filePath, installAuthoritativeSnapshot, rootPath, setTabContent, setTabDirty]); useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { @@ -347,7 +400,6 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl setConflictedContent(null); setDiagramRevision((current) => current + 1); setLoadState({ status: 'ready', diagram }); - editGenerationRef.current += 1; scheduleSave(conflictedContent); } catch { setLoadState({ status: 'invalid', reason: 'invalid' }); @@ -365,26 +417,18 @@ export function FlowDiagramEditor({ content, fileName, filePath, loadError }: Fl if (!rootPath) return; setLoadState({ status: 'loading' }); const result = await reloadProjectFile(rootPath, filePath, fileName); - if (!result.ok || result.file.loadError) { + if (!result.ok || result.file.loadError || !result.file.documentVersion) { setLoadState({ status: 'invalid', reason: 'unreadable' }); return; } - - try { - const diagram = await restoreFlowDiagram(result.file.content); - lastSavedContentRef.current = serializeFlowDiagram( - diagram.elements, - diagram.appState, - diagram.files, - ); - lastDiskContentRef.current = result.file.content; - setSaveState('saved'); - setDiagramRevision((current) => current + 1); - setLoadState({ status: 'ready', diagram }); - } catch { - setLoadState({ status: 'invalid', reason: 'invalid' }); - } - }, [fileName, filePath, rootPath]); + await installAuthoritativeSnapshot( + { + content: result.file.content, + version: result.file.documentVersion, + }, + null, + ); + }, [fileName, filePath, installAuthoritativeSnapshot, rootPath]); if (loadState.status === 'loading') { return ( diff --git a/src/renderer/src/lib/openProjectFile.ts b/src/renderer/src/lib/openProjectFile.ts index 5bdd5b1d..b27ea893 100644 --- a/src/renderer/src/lib/openProjectFile.ts +++ b/src/renderer/src/lib/openProjectFile.ts @@ -60,16 +60,25 @@ async function openProjectFileFromDisk( const isFlowDiagram = isFlowDiagramFile(fileName); try { + if (isFlowDiagram) { + const result = await window.electronAPI.flowDiagram.loadDocument(rootPath, filePath); + if (!result.ok) return openUnreadableDiagram(); + const file = { + path: filePath, + name: fileName, + content: result.document.content, + documentVersion: result.document.version, + }; + useFileStore.getState().openPreview(file); + return { ok: true, file, reused: false }; + } + const result = await window.electronAPI.fs.readFile(rootPath, filePath); if (!result.ok) { - return isFlowDiagram - ? openUnreadableDiagram() - : { ok: false, message: result.error.message }; + return { ok: false, message: result.error.message }; } if ('binary' in result.data) { - return isFlowDiagram - ? openUnreadableDiagram() - : { ok: false, message: 'Binary files cannot be opened in the editor.' }; + return { ok: false, message: 'Binary files cannot be opened in the editor.' }; } const file = { diff --git a/src/renderer/src/stores/fileStore.ts b/src/renderer/src/stores/fileStore.ts index bca95de2..d0079a0d 100644 --- a/src/renderer/src/stores/fileStore.ts +++ b/src/renderer/src/stores/fileStore.ts @@ -1,10 +1,12 @@ import { create } from 'zustand'; import type { DirectoryEntry } from '@shared/types'; +import type { FlowDiagramDocumentVersion } from '@shared/flow-diagrams'; export interface PreviewFile { path: string; name: string; content: string; + documentVersion?: FlowDiagramDocumentVersion; loadError?: 'unreadable'; } @@ -25,7 +27,11 @@ interface FileState { fileTreeCollapsed: boolean; dirtyTabs: Record; setTabDirty: (path: string, dirty: boolean) => void; - setTabContent: (path: string, content: string) => void; + setTabContent: ( + path: string, + content: string, + documentVersion?: FlowDiagramDocumentVersion, + ) => void; setFilePanelOpen: (open: boolean) => void; toggleFilePanel: () => void; @@ -71,12 +77,14 @@ export const useFileStore = create((set) => ({ setFilePanelWidth: (width) => set({ filePanelWidth: width }), setRootPath: (path) => set({ rootPath: path, expandedDirs: {}, dirContents: {}, dirErrors: {}, filterQuery: '', openTabs: [], activeTabIndex: -1, previewFile: null, filePanelMode: 'tree', filePanelWidth: 280, selectedPath: null, fileTreeCollapsed: false, dirtyTabs: {} }), setTabDirty: (path, dirty) => set((s) => ({ dirtyTabs: { ...s.dirtyTabs, [path]: dirty } })), - setTabContent: (path, content) => set((s) => { + setTabContent: (path, content, documentVersion) => set((s) => { const openTabs = s.openTabs.map((tab) => ( - tab.path === path ? { ...tab, content, loadError: undefined } : tab + tab.path === path + ? { ...tab, content, documentVersion, loadError: undefined } + : tab )); const previewFile = s.previewFile?.path === path - ? { ...s.previewFile, content, loadError: undefined } + ? { ...s.previewFile, content, documentVersion, loadError: undefined } : s.previewFile; return { openTabs, previewFile }; }), diff --git a/src/shared/flow-diagrams.ts b/src/shared/flow-diagrams.ts index 526aa9a7..709b8e9c 100644 --- a/src/shared/flow-diagrams.ts +++ b/src/shared/flow-diagrams.ts @@ -22,16 +22,46 @@ export type FlowDiagramExportResponse = export const FLOW_DIAGRAM_EXPORT_REQUEST_CHANNEL = 'flow-diagram:export-request'; export const FLOW_DIAGRAM_EXPORT_RESPONSE_CHANNEL = 'flow-diagram:export-response'; +declare const flowDiagramDocumentVersionBrand: unique symbol; + +/** + * 精确文档字节的 opaque identity。调用方只能比较相等性，不能推断顺序。 + */ +export type FlowDiagramDocumentVersion = string & { + readonly [flowDiagramDocumentVersionBrand]: 'FlowDiagramDocumentVersion'; +}; + +export interface FlowDiagramDocumentSnapshot { + content: string; + version: FlowDiagramDocumentVersion; +} + +export interface FlowDiagramDocumentChangeEvent { + filePath: string; + version: FlowDiagramDocumentVersion | null; + /** Present only for the CDF renderer mutation that directly caused this publication. */ + mutationId?: string; +} + +export type FlowDiagramDocumentReadResult = + | { ok: true; document: FlowDiagramDocumentSnapshot } + | { ok: false; error: { code: string; message: string } }; + /** 文档在保存基线之后被外部改写时的冲突码（编辑器与文档存储共享）。 */ export const FLOW_DIAGRAM_SOURCE_CHANGED = 'SOURCE_CHANGED' as const; /** - * Flow Diagram 文档存储的保存结果。SOURCE_CHANGED 冲突附带当前磁盘内容， - * 编辑器用它重定位下一次保存的 CAS 基线，无需二次读取。 + * Flow Diagram 文档存储的保存结果。成功与 SOURCE_CHANGED 都附带权威版本， + * 编辑器无需通过通用文件系统 API 二次读取或自行维护磁盘内容镜像。 */ export type FlowDiagramDocumentSaveResult = - | { ok: true } + | { ok: true; document: FlowDiagramDocumentSnapshot } | { ok: false; - error: { code: string; message: string; currentContent?: string | null }; + error: { + code: string; + message: string; + currentContent?: string | null; + currentVersion?: FlowDiagramDocumentVersion | null; + }; }; diff --git a/src/shared/ipc-contract.ts b/src/shared/ipc-contract.ts index 232d9fa8..a2a43db6 100644 --- a/src/shared/ipc-contract.ts +++ b/src/shared/ipc-contract.ts @@ -32,7 +32,12 @@ import type { } from './conversations'; import type { ContextAggregate } from './context'; import type { BinaryFileInfo, DirectoryEntry, FileContent, FileError, FileInfo } from './filesystem'; -import type { FlowDiagramDocumentSaveResult } from './flow-diagrams'; +import type { + FlowDiagramDocumentChangeEvent, + FlowDiagramDocumentReadResult, + FlowDiagramDocumentSaveResult, + FlowDiagramDocumentVersion, +} from './flow-diagrams'; import type { KnowledgeEntryCreateInput, KnowledgeEntrySearchOptions, @@ -261,9 +266,19 @@ export interface IpcInvokeContract { args: [rootPath: string, filePath: string, content: string]; result: FsAck; }; - // Flow Diagram 文档写路径走文档存储：原子 CAS + 场景校验 + 冲突结构化返回。 + // Flow Diagram 一致性路径只暴露版本化文档 API，不经通用 fs read/write。 + 'flow-diagram:load-document': { + args: [rootPath: string, filePath: string]; + result: FlowDiagramDocumentReadResult; + }; 'flow-diagram:save-document': { - args: [rootPath: string, filePath: string, content: string, expectedContent: string | null]; + args: [ + rootPath: string, + filePath: string, + content: string, + expectedVersion: FlowDiagramDocumentVersion, + mutationId?: string, + ]; result: FlowDiagramDocumentSaveResult; }; 'fs:createFile': { args: [rootPath: string, filePath: string]; result: FsAck }; @@ -448,6 +463,7 @@ export const IPC_INVOKE_CHANNELS = [ 'fs:readFile', 'fs:getFileInfo', 'fs:writeFile', + 'flow-diagram:load-document', 'flow-diagram:save-document', 'fs:createFile', 'fs:createDirectory', @@ -490,6 +506,7 @@ export interface IpcEventContract { 'conversation:messages-changed': { sessionId: string }; 'conversation:run-event': ConversationRunStreamEnvelope; 'fs:directoryChange': { type: string; path: string }; + 'flow-diagram:document-change': FlowDiagramDocumentChangeEvent; 'commands:changed': { source: string }; 'commands:fallback': { scope: 'system' | 'project'; dir: string; error: string }; 'capability-jobs:changed': CapabilityJobEvent; From 9d7b90f40a25f11a608f9e979ae625c1323451e9 Mon Sep 17 00:00:00 2001 From: suntianc Date: Wed, 29 Jul 2026 08:28:47 -0700 Subject: [PATCH 8/8] =?UTF-8?q?test(ci):=20=E6=94=BE=E5=AE=BD=20Windows=20?= =?UTF-8?q?=E6=96=87=E4=BB=B6=E7=B3=BB=E7=BB=9F=E7=94=A8=E4=BE=8B=E8=B6=85?= =?UTF-8?q?=E6=97=B6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/academic-style-revision-skill.test.ts | 2 +- src/main/at-mention/candidate-lister.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/academic-style-revision-skill.test.ts b/src/main/academic-style-revision-skill.test.ts index 4f9abc02..83dff753 100644 --- a/src/main/academic-style-revision-skill.test.ts +++ b/src/main/academic-style-revision-skill.test.ts @@ -76,7 +76,7 @@ describe('Academic Style Revision Skill', () => { expect(fs.readFileSync(path.join(skillDir as string, 'SKILL.md'), 'utf-8')).toBe(markdown); expect(fs.existsSync(path.join(skillDir as string, 'scripts'))).toBe(false); expect(fs.readFileSync(path.join(skillDir as string, 'PROVENANCE.md'), 'utf-8')).toContain('MIT'); - }); + }, 15_000); it('publishes scope, fidelity, coverage, and report safety contracts without executable capabilities', () => { const markdown = getAcademicStyleRevisionSkillMarkdown(); diff --git a/src/main/at-mention/candidate-lister.test.ts b/src/main/at-mention/candidate-lister.test.ts index 738aaed7..7c330b4c 100644 --- a/src/main/at-mention/candidate-lister.test.ts +++ b/src/main/at-mention/candidate-lister.test.ts @@ -121,7 +121,7 @@ describe('candidate-lister', () => { const result = listCandidates(tempDir); expect(result.candidates.length).toBe(5000); expect(result.truncated).toBe(true); - }, 15_000); + }, 30_000); // Phase 08.3 fix #6: symlink-traversal guard. it('drops symlink paths that resolve outside the project root', () => {