From 365c2b0f76f1e13f576b2c2b59508371e06c8a64 Mon Sep 17 00:00:00 2001 From: brother7 <7brother7@gmail.com> Date: Sun, 23 Aug 2026 18:31:53 +0800 Subject: [PATCH] fix(coding): preserve conflict and unicode search results --- .../20260823-pi-host-cutover-c4e8a1d7.md | 9 ++- .../conversation-change-tracker.ts | 14 ++++- electron/coding-projects/project-files.ts | 60 +++++++++++++++---- shared/coding-product-tools.ts | 1 + tests/unit/coding-product-services.test.ts | 45 ++++++++++++++ tests/unit/coding-project-files.test.ts | 42 ++++++++++++- 6 files changed, 152 insertions(+), 19 deletions(-) diff --git a/.project-docs/30-worklog/tasks/20260823-pi-host-cutover-c4e8a1d7.md b/.project-docs/30-worklog/tasks/20260823-pi-host-cutover-c4e8a1d7.md index c71b83d..61f37ef 100644 --- a/.project-docs/30-worklog/tasks/20260823-pi-host-cutover-c4e8a1d7.md +++ b/.project-docs/30-worklog/tasks/20260823-pi-host-cutover-c4e8a1d7.md @@ -86,15 +86,20 @@ - Added focused unit and Windows Electron coverage for Git/non-Git files, Unicode truncation, path rejection/redaction, shared tracker identity, catalog filtering, seven route DTOs, Renderer encoding, and Main dispatcher. +- Architecture review reproduced two reachable parity gaps in the first + implementation commit. The follow-up recognizes porcelain-v2 unmerged + records as `conflicted` in both file status and target-run changes, and maps + length-changing Unicode case-fold indices back to the original line before + returning search submatches. Both failures now have real regression tests. ## Verification -- `corepack pnpm exec vitest run tests/unit/coding-project-files.test.ts tests/unit/coding-product-services.test.ts tests/unit/coding-files-routes.test.ts tests/unit/coding-product-tools-facade.test.ts` — Pass, 4 files / 10 tests. +- `corepack pnpm exec vitest run tests/unit/coding-project-files.test.ts tests/unit/coding-product-services.test.ts tests/unit/coding-files-routes.test.ts tests/unit/coding-product-tools-facade.test.ts` — Pass, 4 files / 12 tests after review fixes. - `corepack pnpm exec vitest run --config vitest.electron.config.ts tests/electron-runtime/coding-files-host.test.ts` — Pass, 1 file / 1 test (focused seam). - `corepack pnpm run typecheck` — Pass. - `corepack pnpm run lint:check` — Pass with 6 pre-existing warnings and no errors. - `corepack pnpm run build:vite` — Pass; existing dynamic-import/chunk-size warnings remain. -- `corepack pnpm test` — Pass on clean rerun, 207 files / 2237 passed / 2 skipped. The first run had one Windows temporary-file `EPERM` and one async AI-hardware loading timeout; both passed when isolated and the complete suite then passed. +- `corepack pnpm test` — Pass after review fixes, 207 files / 2239 passed / 2 skipped. The first pre-review run had one Windows temporary-file `EPERM` and one async AI-hardware loading timeout; both passed when isolated and complete-suite reruns passed. - `corepack pnpm run test:electron:windows` — Pass, 3 files / 7 tests. A direct Vitest invocation was intentionally discarded because it bypassed the repository's local-Electron wrapper. - Real Provider verification — **Explicitly Waived / Accepted Risk**; `realTurnVerified=false`, not Pass. diff --git a/electron/coding-projects/conversation-change-tracker.ts b/electron/coding-projects/conversation-change-tracker.ts index 1c8e88c..af94572 100644 --- a/electron/coding-projects/conversation-change-tracker.ts +++ b/electron/coding-projects/conversation-change-tracker.ts @@ -101,8 +101,13 @@ function normalizeRelativePath(value: string): string { return normalized; } -function statusFromSignature(signature: string, renamed: boolean): ConversationChangedFileStatus { +function statusFromSignature( + signature: string, + renamed: boolean, + conflicted = false, +): ConversationChangedFileStatus { if (signature === '??') return 'untracked'; + if (conflicted) return 'conflicted'; if (renamed || signature.includes('R')) return 'renamed'; if (signature.includes('D')) return 'deleted'; if (signature.includes('A')) return 'added'; @@ -121,7 +126,10 @@ function parsePorcelainV2(value: string): StatusEntry[] { continue; } const renamed = record.startsWith('2 '); - const match = renamed + const conflicted = record.startsWith('u '); + const match = conflicted + ? record.match(/^u ([^ ]+) [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ (.*)$/s) + : renamed ? record.match(/^2 ([^ ]+) [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ (.*)$/s) : record.match(/^1 ([^ ]+) [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ (.*)$/s); if (!match) continue; @@ -129,7 +137,7 @@ function parsePorcelainV2(value: string): StatusEntry[] { const filePath = normalizeRelativePath(match[2]); entries.push({ path: filePath, - status: statusFromSignature(signature, renamed), + status: statusFromSignature(signature, renamed, conflicted), signature, }); if (renamed) index += 1; diff --git a/electron/coding-projects/project-files.ts b/electron/coding-projects/project-files.ts index c372246..f9c6af6 100644 --- a/electron/coding-projects/project-files.ts +++ b/electron/coding-projects/project-files.ts @@ -66,8 +66,13 @@ async function containedExistingTarget(projectPath: string, relativePath: string return target; } -function statusFromSignature(signature: string, renamed: boolean): CodingProjectFileStatus { +function statusFromSignature( + signature: string, + renamed: boolean, + conflicted = false, +): CodingProjectFileStatus { if (signature === '??') return 'untracked'; + if (conflicted) return 'conflicted'; if (renamed || signature.includes('R')) return 'renamed'; if (signature.includes('D')) return 'deleted'; if (signature.includes('A')) return 'added'; @@ -87,12 +92,15 @@ function parseStatus(value: string): CodingProjectFileEntry[] { status = 'untracked'; } else { const renamed = record.startsWith('2 '); - const match = renamed + const conflicted = record.startsWith('u '); + const match = conflicted + ? record.match(/^u ([^ ]+) [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ (.*)$/s) + : renamed ? record.match(/^2 ([^ ]+) [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ (.*)$/s) : record.match(/^1 ([^ ]+) [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ [^ ]+ (.*)$/s); if (!match) continue; filePath = match[2]; - status = statusFromSignature(match[1], renamed); + status = statusFromSignature(match[1], renamed, conflicted); if (renamed) index += 1; } const normalized = normalizeRelativePath(filePath); @@ -141,6 +149,30 @@ function decodeText(data: Buffer, allowIncompleteSuffix = false): string { } } +function foldedTextWithOffsets(value: string): { + text: string; + starts: number[]; + ends: number[]; +} { + let text = ''; + const starts: number[] = []; + const ends: number[] = []; + for (let index = 0; index < value.length;) { + const codePoint = value.codePointAt(index); + if (codePoint === undefined) break; + const character = String.fromCodePoint(codePoint); + const end = index + character.length; + const folded = character.toLowerCase(); + text += folded; + for (let foldedIndex = 0; foldedIndex < folded.length; foldedIndex += 1) { + starts.push(index); + ends.push(end); + } + index = end; + } + return { text, starts, ends }; +} + async function fallbackFileList(projectPath: string): Promise { const root = path.resolve(projectPath); const files: string[] = []; @@ -243,7 +275,7 @@ export class CodingProjectFileService { const needle = pattern.trim(); if (!needle) throw new Error('Search pattern is required'); if (needle.length > 512) throw new Error('Search pattern is too long'); - const foldedNeedle = needle.toLocaleLowerCase(); + const foldedNeedle = foldedTextWithOffsets(needle).text; const matches: CodingTextSearchResult[] = []; for (const filePath of await this.listFiles(projectPath)) { if (matches.length >= MAX_SEARCH_RESULTS) break; @@ -258,20 +290,24 @@ export class CodingProjectFileService { const lines = content.split(/\r?\n/); for (let lineIndex = 0; lineIndex < lines.length && matches.length < MAX_SEARCH_RESULTS; lineIndex += 1) { const line = lines[lineIndex]; - const foldedLine = line.toLocaleLowerCase(); - const first = foldedLine.indexOf(foldedNeedle); + const foldedLine = foldedTextWithOffsets(line); + const first = foldedLine.text.indexOf(foldedNeedle); if (first < 0) continue; - const windowStart = Math.max(0, first - 256); + const firstOriginalIndex = foldedLine.starts[first] ?? 0; + const windowStart = Math.max(0, firstOriginalIndex - 256); const lineText = line.slice(windowStart, windowStart + MAX_SEARCH_LINE_CHARS); - const foldedText = lineText.toLocaleLowerCase(); + const foldedText = foldedTextWithOffsets(lineText); const submatches = []; let offset = 0; while (submatches.length < 20) { - const start = foldedText.indexOf(foldedNeedle, offset); - if (start < 0) break; - const end = start + needle.length; + const foldedStart = foldedText.text.indexOf(foldedNeedle, offset); + if (foldedStart < 0) break; + const foldedEnd = foldedStart + foldedNeedle.length; + const start = foldedText.starts[foldedStart]; + const end = foldedText.ends[foldedEnd - 1]; + if (start === undefined || end === undefined) break; submatches.push({ text: lineText.slice(start, end), start, end }); - offset = Math.max(end, start + 1); + offset = Math.max(foldedEnd, foldedStart + 1); } matches.push({ path: filePath, diff --git a/shared/coding-product-tools.ts b/shared/coding-product-tools.ts index 76eb292..2332409 100644 --- a/shared/coding-product-tools.ts +++ b/shared/coding-product-tools.ts @@ -5,6 +5,7 @@ export type CodingProjectFileStatus = | 'modified' | 'deleted' | 'renamed' + | 'conflicted' | 'untracked'; export interface CodingProjectFileEntry { diff --git a/tests/unit/coding-product-services.test.ts b/tests/unit/coding-product-services.test.ts index cbe448b..1a44074 100644 --- a/tests/unit/coding-product-services.test.ts +++ b/tests/unit/coding-product-services.test.ts @@ -1,8 +1,10 @@ // @vitest-environment node +import { execFile } from 'node:child_process'; import { mkdtemp, rm, 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 { @@ -19,6 +21,7 @@ import { PiProductTools } from '../../electron/coding-runtime/pi/product-tools'; const roots: string[] = []; const conversationId = '11111111-1111-4111-8111-111111111111'; +const exec = promisify(execFile); afterEach(async () => { await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); @@ -62,6 +65,14 @@ function productTools(root: string): PiProductTools { }); } +async function git(root: string, ...args: string[]): Promise { + await exec('git', ['-C', root, ...args], { windowsHide: true }); +} + +async function gitOutput(root: string, ...args: string[]): Promise { + return (await exec('git', ['-C', root, ...args], { windowsHide: true })).stdout.trim(); +} + describe('PI-105 product Host composition', () => { it('projects managed skills and a stable command catalog without raw Pi fields', async () => { const root = await configuredProject(); @@ -116,6 +127,40 @@ describe('PI-105 product Host composition', () => { expect(JSON.stringify(await host.getChanges(conversationId))).not.toContain(root); }); + it('keeps a merge conflict created during the target run in its changes snapshot', async () => { + const root = await configuredProject(); + await git(root, 'init'); + await git(root, 'config', 'user.email', 'pi-products@example.invalid'); + await git(root, 'config', 'user.name', 'PI Products'); + await writeFile(path.join(root, 'conflict.txt'), 'baseline\n', 'utf8'); + await git(root, 'add', '.'); + await git(root, 'commit', '-m', 'baseline'); + const baseBranch = await gitOutput(root, 'branch', '--show-current'); + await git(root, 'switch', '-c', 'conflicting-change'); + await writeFile(path.join(root, 'conflict.txt'), 'branch change\n', 'utf8'); + await git(root, 'commit', '-am', 'branch change'); + await git(root, 'switch', baseBranch); + await writeFile(path.join(root, 'conflict.txt'), 'base change\n', 'utf8'); + await git(root, 'commit', '-am', 'base change'); + + const tools = productTools(root); + const host = createCodingProductHost({ + getActiveProject: async () => ({ id: 'project-a', path: root }), + productTools: tools, + }); + await tools.beginRun({ conversationId, runId: 'run-conflict', projectPath: root }); + await expect(exec('git', ['-C', root, 'merge', 'conflicting-change'], { windowsHide: true })) + .rejects.toThrow(); + await tools.markBash(conversationId, 'run-conflict'); + await tools.settleRun(conversationId, 'run-conflict'); + + expect(await host.getChanges(conversationId)).toMatchObject({ + conversationId, + runId: 'run-conflict', + files: [expect.objectContaining({ path: 'conflict.txt', status: 'conflicted' })], + }); + }); + it('returns typed project, Agent, and Conversation errors', async () => { const root = await configuredProject(); const tools = productTools(root); diff --git a/tests/unit/coding-project-files.test.ts b/tests/unit/coding-project-files.test.ts index 5eec174..55c8d76 100644 --- a/tests/unit/coding-project-files.test.ts +++ b/tests/unit/coding-project-files.test.ts @@ -25,6 +25,10 @@ async function git(root: string, ...args: string[]): Promise { await exec('git', ['-C', root, ...args], { windowsHide: true }); } +async function gitOutput(root: string, ...args: string[]): Promise { + return (await exec('git', ['-C', root, ...args], { windowsHide: true })).stdout.trim(); +} + async function initializeRepository(root: string): Promise { await git(root, 'init'); await git(root, 'config', 'user.email', 'pi-files@example.invalid'); @@ -79,11 +83,33 @@ describe('PI-105 project file service', () => { await expect(service.content(root, 'binary.bin')).rejects.toThrow('Binary project files'); }); + it('keeps reachable Git merge conflicts visible in file status', async () => { + const root = await temporaryRoot('makelore-pi-files-conflict-'); + await initializeRepository(root); + const baseBranch = await gitOutput(root, 'branch', '--show-current'); + await writeFile(path.join(root, 'conflict.txt'), 'baseline\n', 'utf8'); + await git(root, 'add', 'conflict.txt'); + await git(root, 'commit', '-m', 'conflict baseline'); + await git(root, 'switch', '-c', 'conflicting-change'); + await writeFile(path.join(root, 'conflict.txt'), 'branch change\n', 'utf8'); + await git(root, 'commit', '-am', 'branch change'); + await git(root, 'switch', baseBranch); + await writeFile(path.join(root, 'conflict.txt'), 'base change\n', 'utf8'); + await git(root, 'commit', '-am', 'base change'); + await expect(exec('git', ['-C', root, 'merge', 'conflicting-change'], { windowsHide: true })) + .rejects.toThrow(); + + expect(await new CodingProjectFileService().status(root)).toContainEqual({ + path: 'conflict.txt', name: 'conflict.txt', type: 'file', status: 'conflicted', + }); + }); + it('searches literal text with bounded relative matches and falls back outside Git', async () => { const root = await temporaryRoot('makelore-pi-files-search-'); await mkdir(path.join(root, 'docs'), { recursive: true }); await mkdir(path.join(root, 'node_modules', 'private-package'), { recursive: true }); await writeFile(path.join(root, 'docs', 'guide.md'), 'First line\nNeedle and needle again\n', 'utf8'); + await writeFile(path.join(root, 'docs', 'unicode.md'), 'İx needle and NEEDLE\n', 'utf8'); await writeFile(path.join(root, 'node_modules', 'private-package', 'secret.txt'), 'needle\n', 'utf8'); const service = new CodingProjectFileService({ async run() { @@ -95,7 +121,7 @@ describe('PI-105 project file service', () => { 'docs/guide.md', ]); const matches = await service.search(root, 'needle'); - expect(matches).toEqual([{ + expect(matches).toContainEqual({ path: 'docs/guide.md', name: 'guide.md', lineNumber: 2, @@ -104,8 +130,20 @@ describe('PI-105 project file service', () => { { text: 'Needle', start: 0, end: 6 }, { text: 'needle', start: 11, end: 17 }, ], - }]); + }); expect(JSON.stringify(matches)).not.toContain(root); expect(JSON.stringify(matches)).not.toContain('private-package'); + + expect(await service.search(root, 'i')).toContainEqual(expect.objectContaining({ + path: 'docs/unicode.md', + submatches: [{ text: 'İ', start: 0, end: 1 }], + })); + expect(await service.search(root, 'needle')).toContainEqual(expect.objectContaining({ + path: 'docs/unicode.md', + submatches: [ + { text: 'needle', start: 3, end: 9 }, + { text: 'NEEDLE', start: 14, end: 20 }, + ], + })); }); });