fix(coding): preserve conflict and unicode search results

This commit is contained in:
2026-08-23 18:31:53 +08:00
parent 9520872d2f
commit 365c2b0f76
6 changed files with 152 additions and 19 deletions

View File

@@ -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.

View File

@@ -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;

View File

@@ -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<string[]> {
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,

View File

@@ -5,6 +5,7 @@ export type CodingProjectFileStatus =
| 'modified'
| 'deleted'
| 'renamed'
| 'conflicted'
| 'untracked';
export interface CodingProjectFileEntry {

View File

@@ -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<void> {
await exec('git', ['-C', root, ...args], { windowsHide: true });
}
async function gitOutput(root: string, ...args: string[]): Promise<string> {
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);

View File

@@ -25,6 +25,10 @@ async function git(root: string, ...args: string[]): Promise<void> {
await exec('git', ['-C', root, ...args], { windowsHide: true });
}
async function gitOutput(root: string, ...args: string[]): Promise<string> {
return (await exec('git', ['-C', root, ...args], { windowsHide: true })).stdout.trim();
}
async function initializeRepository(root: string): Promise<void> {
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 },
],
}));
});
});