From 7e024093b1f82b5974a1d20330677b9cccb2506f Mon Sep 17 00:00:00 2001 From: brother7 <7brother7@gmail.com> Date: Sun, 23 Aug 2026 15:58:32 +0800 Subject: [PATCH] fix: close PI-090 review findings --- .../20260823-pi-product-tools-a9c4e7d2.md | 34 ++++- .../conversation-change-tracker.ts | 138 +++++++++++++----- electron/coding-runtime/pi/extension-host.ts | 34 ++++- .../pi/extensions/changed-file.ts | 2 +- .../pi/extensions/makelore-runtime.ts | 5 +- electron/coding-runtime/pi/product-tools.ts | 4 + electron/coding-runtime/pi/runtime.ts | 12 +- scripts/run-pi-subagent-packaged-smoke.mjs | 23 +++ tests/unit/pi-extension-bundle.test.ts | 77 +++++++++- tests/unit/pi-product-tools.test.ts | 57 +++++++- vitest.electron.config.ts | 5 +- 11 files changed, 325 insertions(+), 66 deletions(-) diff --git a/.project-docs/30-worklog/tasks/20260823-pi-product-tools-a9c4e7d2.md b/.project-docs/30-worklog/tasks/20260823-pi-product-tools-a9c4e7d2.md index 0683904..480bcd7 100644 --- a/.project-docs/30-worklog/tasks/20260823-pi-product-tools-a9c4e7d2.md +++ b/.project-docs/30-worklog/tasks/20260823-pi-product-tools-a9c4e7d2.md @@ -91,6 +91,17 @@ exposed; project/user auto-discovery remains disabled. - Extended the real Pi and staged production-closure smoke to require all parent product tools while retaining the exact read-only child tool set. +- Closed the planner's first review findings: dirty baselines now use one Git + status plus lightweight file metadata instead of per-file Git diffs before + prompt acceptance; a missing Git executable degrades to no-Git tracking; + write/edit hooks from parent and coding child use a dedicated internal + `changes.touched` ACK bridge; full diff/preview data stays Main-only; and + untracked reads plus previews are both limited to 8 KiB while size/mtime + detect equal-length changes beyond that window. +- The packaged smoke now has two halves: a staged production-closure Pi + process check and a controlled Electron-Node authenticated bridge check that + actually executes browser status/screenshot attachment and game asset + browse/review, rather than checking registrations only. - No Renderer or `/api/coding` route was added; PI-105/PI-130 remain the owners of Host API and UI consumption. `README.md` therefore still describes the unchanged current product cutover state and did not require an update here. @@ -101,17 +112,24 @@ - `corepack pnpm run lint:check`: passed with the same six repository warnings and no errors (`ExecutionGraphCard.tsx`, `Home/index.tsx`, and `Makelore/index.tsx`; none are in this task's changed scope). -- Focused PI-090/Pi projection/bridge/game suite: 10 files, 61 tests passed - before the final boundary additions; subsequent focused runs covered 27 and - 9 tests respectively and passed. -- `corepack pnpm test`: 203 files passed; 2224 tests passed, 2 skipped. +- Final focused PI-090/Pi projection/bridge/game/resource suite: 12 files, + 71 tests passed. The planner's five minimal failure shapes are also covered + by the 4-file 19-test tracker/bridge/runtime subset. +- Final serial full suite + (`corepack pnpm vitest run --maxWorkers=1 --no-file-parallelism`): 203 files, + 2226 tests passed, 2 skipped. Two preceding parallel full-suite runs each + hit the repository's known Windows temporary-file `rename EPERM` in the + unchanged Conversation store atomic write; the same file passed 1/1 in + isolation before the serial full suite passed. - `corepack pnpm run build:vite`: passed for Renderer, Electron Main, Preload, and release utility outputs; existing chunk-size/dynamic-import warnings remain non-blocking. -- `corepack pnpm run test:electron:windows`: 1 file, 3 tests passed. -- `corepack pnpm run test:pi-subagent:packaged`: 1 file, 4 tests passed, - including staged parent product-tool registration and the unchanged staged - read-only child boundary. +- `corepack pnpm run test:electron:windows`: 2 files, 6 tests passed, + including authenticated product-tool execution under Electron Node. +- `corepack pnpm run test:pi-subagent:packaged`: staged runtime 4/4 plus + controlled Electron product-tool bridge 3/3 passed. It verifies browser + status/screenshot attachment persistence, game browse/review execution, + parent tool registration and the unchanged staged read-only child boundary. - Real external Provider turns remain **Explicitly Waived / Accepted Risk**; `realTurnVerified=false`. macOS x64/arm64 validation remains deferred to the mandatory PI-150 gate. Neither item is recorded as passed. diff --git a/electron/coding-projects/conversation-change-tracker.ts b/electron/coding-projects/conversation-change-tracker.ts index 635f3a0..0a22ee1 100644 --- a/electron/coding-projects/conversation-change-tracker.ts +++ b/electron/coding-projects/conversation-change-tracker.ts @@ -1,5 +1,5 @@ import { spawn } from 'node:child_process'; -import { readFile, stat } from 'node:fs/promises'; +import { open, stat } from 'node:fs/promises'; import path from 'node:path'; const MAX_CHANGED_FILES = 200; @@ -72,7 +72,7 @@ interface StatusEntry { } interface FileState extends StatusEntry { - content: string; + identity: string; preview?: string; truncated?: boolean; } @@ -152,7 +152,7 @@ function boundedText(value: string, maxBytes: number): { text: string; truncated async function untrackedPreview( projectPath: string, relativePath: string, -): Promise<{ preview?: string; content: string; truncated?: boolean }> { +): Promise<{ preview?: string; identity: string; truncated?: boolean }> { const target = path.resolve(projectPath, ...relativePath.split('/')); const relative = path.relative(path.resolve(projectPath), target); if (!relative || relative.startsWith('..') || path.isAbsolute(relative)) { @@ -160,19 +160,44 @@ async function untrackedPreview( } try { const file = await stat(target); - if (!file.isFile()) return { content: 'not-file' }; - const data = await readFile(target); - const bounded = data.subarray(0, MAX_UNTRACKED_PREVIEW_BYTES); - if (bounded.includes(0)) return { content: `binary:${data.byteLength}` }; + if (!file.isFile()) return { identity: `not-file:${file.mtimeMs}` }; + const byteLength = Math.min(file.size, MAX_UNTRACKED_PREVIEW_BYTES); + const data = Buffer.alloc(byteLength); + const handle = await open(target, 'r'); + let bytesRead = 0; + try { + ({ bytesRead } = await handle.read(data, 0, byteLength, 0)); + } finally { + await handle.close(); + } + const bounded = data.subarray(0, bytesRead); + const identity = `${file.size}:${file.mtimeMs}`; + if (bounded.includes(0)) return { identity: `binary:${identity}` }; const preview = bounded.toString('utf8'); return { preview, - content: `text:${data.byteLength}:${preview}`, - ...(data.byteLength > bounded.byteLength ? { truncated: true } : {}), + identity: `text:${identity}`, + ...(file.size > bounded.byteLength ? { truncated: true } : {}), }; } catch (error) { if (error && typeof error === 'object' && 'code' in error && error.code === 'ENOENT') { - return { content: 'missing' }; + return { identity: 'missing' }; + } + throw error; + } +} + +async function trackedIdentity(projectPath: string, entry: StatusEntry): Promise { + const target = path.resolve(projectPath, ...entry.path.split('/')); + try { + const file = await stat(target); + return { + ...entry, + identity: `${file.isFile() ? 'file' : 'not-file'}:${file.size}:${file.mtimeMs}`, + }; + } catch (error) { + if (error && typeof error === 'object' && 'code' in error && error.code === 'ENOENT') { + return { ...entry, identity: 'missing' }; } throw error; } @@ -246,18 +271,29 @@ export class ConversationChangeTracker { } private async capture(projectPath: string): Promise { - const repository = await this.git.run(projectPath, ['rev-parse', '--is-inside-work-tree']); + let repository: GitCommandResult; + try { + repository = await this.git.run(projectPath, ['rev-parse', '--is-inside-work-tree']); + } catch { + return { git: false, head: null, files: new Map() }; + } if (repository.code !== 0 || repository.stdout.trim() !== 'true') { return { git: false, head: null, files: new Map() }; } - const [headResult, statusResult] = await Promise.all([ - this.git.run(projectPath, ['rev-parse', 'HEAD']), - this.git.run(projectPath, ['status', '--porcelain=v2', '-z', '--untracked-files=all', '--', '.']), - ]); + let headResult: GitCommandResult; + let statusResult: GitCommandResult; + try { + [headResult, statusResult] = await Promise.all([ + this.git.run(projectPath, ['rev-parse', 'HEAD']), + this.git.run(projectPath, ['status', '--porcelain=v2', '-z', '--untracked-files=all', '--', '.']), + ]); + } catch { + return { git: false, head: null, files: new Map() }; + } if (statusResult.code !== 0) return { git: false, head: null, files: new Map() }; const entries = parsePorcelainV2(statusResult.stdout); - const files = new Map(); - for (const entry of entries) files.set(entry.path, await this.readState(projectPath, entry)); + const states = await Promise.all(entries.map((entry) => this.readState(projectPath, entry))); + const files = new Map(states.map((state) => [state.path, state])); return { git: true, head: headResult.code === 0 ? headResult.stdout.trim() || null : null, @@ -265,31 +301,37 @@ export class ConversationChangeTracker { }; } - private async currentStates(projectPath: string): Promise> { - const status = await this.git.run( - projectPath, - ['status', '--porcelain=v2', '-z', '--untracked-files=all', '--', '.'], - ); - if (status.code !== 0) return new Map(); - const result = new Map(); - for (const entry of parsePorcelainV2(status.stdout)) { - result.set(entry.path, await this.readState(projectPath, entry)); + private async currentEntries(projectPath: string): Promise | null> { + try { + const status = await this.git.run( + projectPath, + ['status', '--porcelain=v2', '-z', '--untracked-files=all', '--', '.'], + ); + if (status.code !== 0) return null; + return new Map(parsePorcelainV2(status.stdout).map((entry) => [entry.path, entry])); + } catch { + return null; } - return result; } private async readState(projectPath: string, entry: StatusEntry): Promise { if (entry.status === 'untracked') { return { ...entry, ...await untrackedPreview(projectPath, entry.path) }; } + return await trackedIdentity(projectPath, entry); + } + + private async readDiff(projectPath: string, relativePath: string): Promise<{ + diff?: string; + truncated?: boolean; + }> { const [working, staged] = await Promise.all([ - this.git.run(projectPath, ['diff', '--no-ext-diff', '--no-color', '--relative', '--', entry.path]), - this.git.run(projectPath, ['diff', '--cached', '--no-ext-diff', '--no-color', '--relative', '--', entry.path]), + this.git.run(projectPath, ['diff', '--no-ext-diff', '--no-color', '--relative', '--', relativePath]), + this.git.run(projectPath, ['diff', '--cached', '--no-ext-diff', '--no-color', '--relative', '--', relativePath]), ]); const bounded = boundedText(`${staged.stdout}${working.stdout}`, MAX_DIFF_BYTES); return { - ...entry, - content: bounded.text, + ...(bounded.text ? { diff: bounded.text } : {}), ...(bounded.truncated ? { truncated: true } : {}), }; } @@ -301,20 +343,37 @@ export class ConversationChangeTracker { const current = await untrackedPreview(record.projectPath, filePath); files.push({ path: filePath, - status: current.content === 'missing' ? 'deleted' : 'modified', + status: current.identity === 'missing' ? 'deleted' : 'modified', ...(current.preview !== undefined ? { preview: current.preview } : {}), ...(current.truncated ? { truncated: true } : {}), }); } return { ...record.snapshot, files }; } - const current = await this.currentStates(record.projectPath); + const current = await this.currentEntries(record.projectPath); + if (!current) { + const files: ConversationChangedFile[] = []; + for (const filePath of [...record.touchedPaths].sort().slice(0, MAX_CHANGED_FILES)) { + const state = await untrackedPreview(record.projectPath, filePath); + files.push({ + path: filePath, + status: state.identity === 'missing' ? 'deleted' : 'modified', + ...(state.preview !== undefined ? { preview: state.preview } : {}), + ...(state.truncated ? { truncated: true } : {}), + }); + } + return { ...record.snapshot, git: false, files }; + } const candidates = projectWide ? [...current.keys(), ...record.baseline.files.keys()] : [...record.touchedPaths]; const files: ConversationChangedFile[] = []; - for (const filePath of [...new Set(candidates)].sort().slice(0, MAX_CHANGED_FILES)) { - const next = current.get(filePath); + const candidatePaths = [...new Set(candidates)].sort().slice(0, MAX_CHANGED_FILES); + const states = await Promise.all(candidatePaths.map(async (filePath) => { + const entry = current.get(filePath); + return { filePath, next: entry ? await this.readState(record.projectPath, entry) : undefined }; + })); + for (const { filePath, next } of states) { const baseline = record.baseline.files.get(filePath); if (!next) { if (baseline) { @@ -325,13 +384,16 @@ export class ConversationChangeTracker { } continue; } - if (baseline?.signature === next.signature && baseline.content === next.content) continue; + if (baseline?.signature === next.signature && baseline.identity === next.identity) continue; + const diff = next.status === 'untracked' + ? {} + : await this.readDiff(record.projectPath, next.path); files.push({ path: next.path, status: next.status, ...(next.status === 'untracked' && next.preview !== undefined ? { preview: next.preview } : {}), - ...(next.status !== 'untracked' && next.content ? { diff: next.content } : {}), - ...(next.truncated ? { truncated: true } : {}), + ...diff, + ...(next.truncated || diff.truncated ? { truncated: true } : {}), }); } return { ...record.snapshot, files }; diff --git a/electron/coding-runtime/pi/extension-host.ts b/electron/coding-runtime/pi/extension-host.ts index 4110df5..0c2f303 100644 --- a/electron/coding-runtime/pi/extension-host.ts +++ b/electron/coding-runtime/pi/extension-host.ts @@ -88,7 +88,20 @@ interface ChangeRefreshBridgeRequest { resourceId: string; } -type BridgeRequest = LeaseBridgeRequest | SubagentBridgeRequest | ProductToolBridgeRequest | ChangeRefreshBridgeRequest; +interface ChangeTouchedBridgeRequest { + action: 'changes.touched'; + conversationId: string; + workerGeneration: number; + runId: string; + resourceId: string; + paths: string[]; +} + +type BridgeRequest = LeaseBridgeRequest + | SubagentBridgeRequest + | ProductToolBridgeRequest + | ChangeRefreshBridgeRequest + | ChangeTouchedBridgeRequest; export interface PiExtensionSubagentBridge { scheduler: PiSubagentScheduler; @@ -111,6 +124,12 @@ function bridgeRequest(value: unknown): value is BridgeRequest { return typeof value.toolName === 'string' && 'input' in value; } if (value.action === 'changes.bash') return true; + if (value.action === 'changes.touched') { + return Array.isArray(value.paths) + && value.paths.length > 0 + && value.paths.length <= 200 + && value.paths.every((filePath) => typeof filePath === 'string'); + } return (value.action === 'lease.acquire' || value.action === 'lease.release') && (value.leaseId === undefined || typeof value.leaseId === 'string'); } @@ -304,6 +323,19 @@ export class PiManagedExtensionHost { this.respond(response, 200, { marked: true }); return; } + if (value.action === 'changes.touched') { + if (!this.productTools) { + this.respond(response, 503, { error: 'Conversation change tracker is unavailable' }); + return; + } + await this.productTools.recordTouchedPaths( + record.conversationId, + value.runId, + value.paths, + ); + this.respond(response, 200, { recorded: true }); + return; + } if (value.action === 'product.invoke') { if (record.role !== 'parent') { this.respond(response, 403, { error: 'Child workers cannot invoke parent product tools' }); diff --git a/electron/coding-runtime/pi/extensions/changed-file.ts b/electron/coding-runtime/pi/extensions/changed-file.ts index 7c536eb..a05dd23 100644 --- a/electron/coding-runtime/pi/extensions/changed-file.ts +++ b/electron/coding-runtime/pi/extensions/changed-file.ts @@ -29,7 +29,7 @@ export async function reportChangedFiles( const paths = snapshot.files.map((file) => file.path); const details: ChangedFileDetailsV1 = { schema: 'changed-file.v1', paths }; return { - content: [{ type: 'text' as const, text: JSON.stringify({ paths, changes: snapshot.files }) }], + content: [{ type: 'text' as const, text: `${paths.length} changed path(s) recorded` }], details, }; } diff --git a/electron/coding-runtime/pi/extensions/makelore-runtime.ts b/electron/coding-runtime/pi/extensions/makelore-runtime.ts index 3857185..97ced02 100644 --- a/electron/coding-runtime/pi/extensions/makelore-runtime.ts +++ b/electron/coding-runtime/pi/extensions/makelore-runtime.ts @@ -306,7 +306,10 @@ export default function makeloreRuntime(pi) { const paths = touchedPaths.get(event.toolCallId); touchedPaths.delete(event.toolCallId); if (paths) { - await invokeProduct(event.toolCallId, 'changed_file', { paths, refresh: true }).catch(() => undefined); + await bridge('changes.touched', { + resourceId: event.toolCallId, + paths, + }).catch(() => undefined); } }); pi.on('agent_end', releaseAll); diff --git a/electron/coding-runtime/pi/product-tools.ts b/electron/coding-runtime/pi/product-tools.ts index 4691554..bbee574 100644 --- a/electron/coding-runtime/pi/product-tools.ts +++ b/electron/coding-runtime/pi/product-tools.ts @@ -69,6 +69,10 @@ export class PiProductTools { await this.changeTracker.markProjectRefresh(conversationId, runId); } + recordTouchedPaths(conversationId: string, runId: string, paths: readonly string[]) { + return this.changeTracker.recordTouchedPaths(conversationId, runId, paths); + } + async execute( toolName: PiProductToolName, context: PiProductToolContext, diff --git a/electron/coding-runtime/pi/runtime.ts b/electron/coding-runtime/pi/runtime.ts index 8f8d1cd..6c435df 100644 --- a/electron/coding-runtime/pi/runtime.ts +++ b/electron/coding-runtime/pi/runtime.ts @@ -598,11 +598,11 @@ export class PiConversationRuntime implements CodingConversationRuntime { }; const generation = this.pool.getState(input.conversationId)?.generation; if (generation) this.extensionUi.beginRun(input.conversationId, generation, runId); - if (this.extensionHost && generation) { - await this.extensionHost.bindRun(input.conversationId, generation, runId); - } let ticket; try { + if (this.extensionHost && generation) { + await this.extensionHost.bindRun(input.conversationId, generation, runId); + } ticket = this.pool.startTopLevel({ conversationId: input.conversationId, runId, @@ -744,11 +744,11 @@ export class PiConversationRuntime implements CodingConversationRuntime { const runId = this.id('run'); const generation = this.pool.getState(conversationId)?.generation; if (generation) this.extensionUi.beginRun(conversationId, generation, runId); - if (this.extensionHost && generation) { - await this.extensionHost.bindRun(conversationId, generation, runId); - } let ticket; try { + if (this.extensionHost && generation) { + await this.extensionHost.bindRun(conversationId, generation, runId); + } ticket = this.pool.startTopLevel({ conversationId, runId, diff --git a/scripts/run-pi-subagent-packaged-smoke.mjs b/scripts/run-pi-subagent-packaged-smoke.mjs index 2fcd0b4..cc227fb 100644 --- a/scripts/run-pi-subagent-packaged-smoke.mjs +++ b/scripts/run-pi-subagent-packaged-smoke.mjs @@ -30,6 +30,28 @@ function runVitest(runtimeRoot) { }); } +function runElectronProductTools() { + return new Promise((resolvePromise, reject) => { + const child = spawn(process.execPath, [ + resolve('scripts/run-electron-vitest.mjs'), + 'tests/unit/pi-extension-bundle.test.ts', + ], { + cwd: process.cwd(), + stdio: 'inherit', + windowsHide: true, + }); + child.once('error', reject); + child.once('exit', (code, signal) => { + if (code === 0) resolvePromise(); + else { + reject(new Error( + `Packaged product-tools Electron smoke failed with code ${code ?? 'null'} signal ${signal ?? 'none'}`, + )); + } + }); + }); +} + const outputRoot = await mkdtemp(join(tmpdir(), 'makelore-pi-subagent-package-')); try { const [bundle] = await bundlePiRuntime({ @@ -38,6 +60,7 @@ try { }); if (!bundle) throw new Error('Pi runtime bundler returned no staged runtime'); await runVitest(bundle.destination); + await runElectronProductTools(); } finally { await rm(outputRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 200 }); } diff --git a/tests/unit/pi-extension-bundle.test.ts b/tests/unit/pi-extension-bundle.test.ts index 5cfb9c3..dbc68ac 100644 --- a/tests/unit/pi-extension-bundle.test.ts +++ b/tests/unit/pi-extension-bundle.test.ts @@ -31,6 +31,11 @@ describe('Makelore Pi extension bundle', () => { const root = await mkdtemp(path.join(tmpdir(), 'makelore-pi-product-bundle-')); roots.push(root); await writeFile(path.join(root, 'notes.txt'), 'changed\n', 'utf8'); + await writeFile(path.join(root, 'ASSET_PLAN.md'), [ + '```json', + JSON.stringify({ assets: [{ id: 'hero', name: 'Hero', category: 'visual', status: 'candidate' }] }), + '```', + ].join('\n'), 'utf8'); const browser = { async getSnapshot() { return { @@ -39,11 +44,17 @@ describe('Makelore Pi extension bundle', () => { visible: false, bounds: null, canGoBack: false, canGoForward: false, eventCursor: 0, }; }, + async sendCdp() { + return { kind: 'inline', value: { data: Buffer.from('packaged-png').toString('base64') } }; + }, } as unknown as AgentBrowserModule; + const attachments = new CodingAttachmentStore(path.join(root, 'attachments'), { + createId: () => 'packaged-attachment-a', + }); const host = new PiManagedExtensionHost(); const productTools = new PiProductTools({ browser, - attachments: new CodingAttachmentStore(path.join(root, 'attachments')), + attachments, bundledSkillsDir: path.resolve('resources/coding-skills'), }); host.configureProductTools(productTools); @@ -82,7 +93,10 @@ describe('Makelore Pi extension bundle', () => { )).resolves.toMatchObject({ details: { schema: 'task-state.v1' } }); await expect(tools.get('changed_file')?.execute?.( 'changed-a', { paths: ['notes.txt'] }, new AbortController().signal, - )).resolves.toMatchObject({ details: { schema: 'changed-file.v1', paths: ['notes.txt'] } }); + )).resolves.toMatchObject({ + content: [{ type: 'text', text: '1 changed path(s) recorded' }], + details: { schema: 'changed-file.v1', paths: ['notes.txt'] }, + }); await expect(tools.get('runtime_context')?.execute?.( 'context-a', {}, new AbortController().signal, )).resolves.toMatchObject({ @@ -96,6 +110,34 @@ describe('Makelore Pi extension bundle', () => { ); expect(browserResult).toMatchObject({ details: { schema: 'agent-browser.v1', action: 'status' } }); expect(JSON.stringify(browserResult)).not.toContain(root); + const screenshotResult = await tools.get('agent_browser')?.execute?.( + 'browser-screenshot-a', + { action: 'send_cdp', method: 'Page.captureScreenshot', params: { format: 'png' } }, + new AbortController().signal, + ); + expect(screenshotResult).toMatchObject({ + details: { + schema: 'agent-browser.v1', action: 'send_cdp', + attachmentId: 'packaged-attachment-a', mime: 'image/png', + }, + }); + expect(JSON.stringify(screenshotResult)).not.toContain( + Buffer.from('packaged-png').toString('base64'), + ); + expect((await attachments.read('packaged-attachment-a')).data.toString()).toBe('packaged-png'); + const gameBrowse = await tools.get('game_asset_browser')?.execute?.( + 'game-browse-a', {}, new AbortController().signal, + ); + expect(gameBrowse).toMatchObject({ + details: { schema: 'game-assets.v1', candidateIds: ['hero'], status: 'pending' }, + }); + const gameReview = await tools.get('game_asset_review')?.execute?.( + 'game-review-a', { candidateIds: ['hero'] }, new AbortController().signal, + ); + expect(gameReview).toMatchObject({ + details: { schema: 'game-assets.v1', candidateIds: ['hero'], status: 'pending' }, + }); + expect(JSON.stringify({ gameBrowse, gameReview })).not.toContain(root); await writeFile(path.join(root, 'notes.txt'), 'changed by write tool\n', 'utf8'); await handlers.get('tool_call')?.({ toolName: 'write', toolCallId: 'write-a', input: { path: path.join(root, 'notes.txt') }, @@ -234,11 +276,23 @@ describe('Makelore Pi extension bundle', () => { it('does not expose parent-only tools from a child process', async () => { const root = await mkdtemp(path.join(tmpdir(), 'makelore-pi-extension-child-bundle-')); roots.push(root); + await writeFile(path.join(root, 'child.txt'), 'before\n', 'utf8'); const host = new PiManagedExtensionHost(); + const productTools = new PiProductTools({ + browser: {} as AgentBrowserModule, + attachments: new CodingAttachmentStore(path.join(root, 'attachments')), + bundledSkillsDir: path.resolve('resources/coding-skills'), + }); + host.configureProductTools(productTools); hosts.push(host); + await host.registerWorker({ + conversationId: 'conversation-child', generation: 1, projectId: 'project-a', + projectPath: root, extensionsDir: root, + }); + await host.bindRun('conversation-child', 1, 'run-parent'); const child = await host.registerWorker({ conversationId: 'conversation-child', generation: 1, projectId: 'project-a', - extensionsDir: root, role: 'child', runId: 'run-parent', + projectPath: root, extensionsDir: root, role: 'child', runId: 'run-parent', }); const previous = { bridge: process.env.MAKELORE_PI_BRIDGE_URL, @@ -257,8 +311,23 @@ describe('Makelore Pi extension bundle', () => { }): void; }; const tools: string[] = []; - module.default({ registerTool: (tool) => tools.push(tool.name), on: () => undefined }); + const handlers = new Map(); + module.default({ + registerTool: (tool) => tools.push(tool.name), + on: (event, handler) => handlers.set(event, handler), + }); expect(tools).toEqual([]); + await writeFile(path.join(root, 'child.txt'), 'after\n', 'utf8'); + await handlers.get('tool_call')?.({ + toolName: 'write', toolCallId: 'child-write', input: { path: 'child.txt' }, + }, { + signal: new AbortController().signal, + ui: { setStatus: () => undefined }, + }); + await handlers.get('tool_result')?.({ toolName: 'write', toolCallId: 'child-write' }); + expect(productTools.getChanges('conversation-child')?.files).toEqual([ + expect.objectContaining({ path: 'child.txt', preview: 'after\n' }), + ]); } finally { for (const [key, value] of Object.entries(previous)) { const environmentKey = key === 'bridge' ? 'MAKELORE_PI_BRIDGE_URL' diff --git a/tests/unit/pi-product-tools.test.ts b/tests/unit/pi-product-tools.test.ts index c9a11cc..c6645c0 100644 --- a/tests/unit/pi-product-tools.test.ts +++ b/tests/unit/pi-product-tools.test.ts @@ -1,14 +1,17 @@ // @vitest-environment node import { execFile } from 'node:child_process'; -import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; +import { mkdtemp, mkdir, rm, utimes, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { promisify } from 'node:util'; import { afterEach, describe, expect, it } from 'vitest'; import type { AgentBrowserModule } from '../../electron/agent-browser'; import { CodingAttachmentStore } from '../../electron/coding-projects/attachment-store'; -import { ConversationChangeTracker } from '../../electron/coding-projects/conversation-change-tracker'; +import { + ConversationChangeTracker, + type ConversationGitAdapter, +} from '../../electron/coding-projects/conversation-change-tracker'; import { buildProductCodingCommandCatalog, listProductCodingSkills, @@ -80,7 +83,7 @@ describe('PI-090 product tools', () => { expect(settled?.files.some((file) => file.path === 'existing.txt')).toBe(false); }); - it('bounds untracked previews without hiding append-only changes beyond the preview', async () => { + it('bounds untracked reads and detects equal-length changes beyond the preview', async () => { const root = await temporaryRoot('makelore-pi-bounded-preview-'); await initializeRepository(root); const target = path.join(root, 'large.txt'); @@ -89,15 +92,57 @@ describe('PI-090 product tools', () => { await tracker.beginRun({ conversationId: 'conversation-a', runId: 'run-a', projectPath: root, }); - await writeFile(target, `${'a'.repeat(9 * 1024)}tail`, 'utf8'); - await tracker.markProjectRefresh('conversation-a', 'run-a'); - const snapshot = await tracker.settleRun('conversation-a', 'run-a'); + await writeFile(target, `${'a'.repeat((9 * 1024) - 1)}b`, 'utf8'); + const changedAt = new Date(Date.now() + 5_000); + await utimes(target, changedAt, changedAt); + const snapshot = await tracker.recordTouchedPaths('conversation-a', 'run-a', ['large.txt']); const file = snapshot?.files.find(({ path: filePath }) => filePath === 'large.txt'); expect(file).toMatchObject({ status: 'untracked', truncated: true }); expect(Buffer.byteLength(file?.preview ?? '', 'utf8')).toBeLessThanOrEqual(8 * 1024); expect(JSON.stringify(snapshot)).not.toContain(root); }); + it('captures a dirty baseline without per-file Git diffs', async () => { + const root = await temporaryRoot('makelore-pi-dirty-baseline-'); + const calls: string[][] = []; + const status = Array.from({ length: 100 }, (_, index) => ( + `1 .M N... 100644 100644 100644 abc abc dirty-${index}.txt\0` + )).join(''); + const adapter: ConversationGitAdapter = { + async run(_projectPath, args) { + calls.push([...args]); + if (args[0] === 'rev-parse' && args[1] === '--is-inside-work-tree') { + return { code: 0, stdout: 'true\n' }; + } + if (args[0] === 'rev-parse') return { code: 0, stdout: `${'a'.repeat(40)}\n` }; + if (args[0] === 'status') return { code: 0, stdout: status }; + throw new Error(`Unexpected Git call: ${args.join(' ')}`); + }, + }; + const tracker = new ConversationChangeTracker(adapter); + expect((await tracker.beginRun({ + conversationId: 'conversation-a', runId: 'run-a', projectPath: root, + })).git).toBe(true); + expect(calls).toHaveLength(3); + expect(calls.some(([command]) => command === 'diff')).toBe(false); + }); + + it('degrades to no-Git tracking when the Git executable is unavailable', async () => { + const root = await temporaryRoot('makelore-pi-git-unavailable-'); + await writeFile(path.join(root, 'notes.txt'), 'local notes\n', 'utf8'); + const tracker = new ConversationChangeTracker({ + async run() { + throw Object.assign(new Error('spawn git ENOENT'), { code: 'ENOENT' }); + }, + }); + expect((await tracker.beginRun({ + conversationId: 'conversation-a', runId: 'run-a', projectPath: root, + })).git).toBe(false); + expect((await tracker.recordTouchedPaths( + 'conversation-a', 'run-a', ['notes.txt'], + )).files).toEqual([expect.objectContaining({ path: 'notes.txt', preview: 'local notes\n' })]); + }); + it('supports no-git projects and rejects paths outside the project', async () => { const root = await temporaryRoot('makelore-pi-no-git-'); await writeFile(path.join(root, 'notes.txt'), 'local notes\n', 'utf8'); diff --git a/vitest.electron.config.ts b/vitest.electron.config.ts index 90547e1..2626a80 100644 --- a/vitest.electron.config.ts +++ b/vitest.electron.config.ts @@ -4,7 +4,10 @@ import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { environment: 'node', - include: ['tests/electron-runtime/**/*.test.ts'], + include: [ + 'tests/electron-runtime/**/*.test.ts', + 'tests/unit/pi-extension-bundle.test.ts', + ], fileParallelism: false, maxWorkers: 1, pool: 'threads',