Skip to content

Commit 39a6d70

Browse files
committed
Track file scope at the owning commit across renames
1 parent 30729a2 commit 39a6d70

3 files changed

Lines changed: 71 additions & 32 deletions

File tree

‎core/linking.ts‎

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ export interface Segment {
1818
oldLine: number | null; newLine: number | null; operation: '+' | '-' | null;
1919
content: string; context: string; hunk: number; sharesHunkWith: string[];
2020
}
21-
interface TrackedLine { text: string; owners: (string | null)[]; origins: string[]; movedOwners: (string | null)[] }
22-
interface TrackedFile { lines: TrackedLine[]; metadataOwners: (string | null)[] }
21+
interface Evidence { owners: (string | null)[]; outOfScope: string[] }
22+
interface TrackedLine { text: string; evidence: Evidence; origins: string[]; moved: Evidence }
23+
interface TrackedFile { lines: TrackedLine[]; metadata: Evidence }
2324
const lines = (text: string | null | undefined): string[] => text?.match(/[^\n]*\n|[^\n]+$/g) ?? [];
2425
const unique = <T>(values: T[]): T[] => [...new Set(values)];
2526
const origin = (path: string, i: number) => `${path}\0${i}`;
@@ -30,21 +31,25 @@ function metadataChange(delta: FileDelta): boolean {
3031
return delta.oldPath !== delta.newPath || delta.before?.mode !== delta.after?.mode ||
3132
!textFile(delta.before) || !textFile(delta.after);
3233
}
33-
function classify(plan: Plan, owners: (string | null)[], paths: string[]): Pick<Segment, 'row' | 'scope'> {
34+
const empty = (): Evidence => ({ owners: [], outOfScope: [] });
35+
const combine = (...evidence: Evidence[]): Evidence => ({
36+
owners: unique(evidence.flatMap(e => e.owners)),
37+
outOfScope: unique(evidence.flatMap(e => e.outOfScope)),
38+
});
39+
function classify(evidence: Evidence): Pick<Segment, 'row' | 'scope'> {
40+
const { owners, outOfScope } = evidence;
3441
if (!owners.length || owners.includes(null)) return { row: 'Unplanned', scope: 'unplanned' };
3542
if (owners.length > 1) return { row: 'Ambiguous', scope: 'ambiguous' };
36-
const item = plan.items.find(item => item.id === owners[0]);
37-
if (!item) throw new Error(`Ledger references unknown plan item ${owners[0]}.`);
38-
const declared = new Set(item.files.flatMap(file => [file.path, ...(file.renamed_from ? [file.renamed_from] : [])]));
39-
return { row: item.id, scope: paths.every(path => declared.has(path)) ? 'in-scope' : 'out-of-scope' };
43+
const owner = owners[0]!;
44+
return { row: owner, scope: outOfScope.includes(owner) ? 'out-of-scope' : 'in-scope' };
4045
}
4146

4247
/** Replays a linear history. Commit messages and Plan-Item trailers are never trusted. */
4348
export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap<string, string>): Segment[] {
4449
const files = new Map<string, TrackedFile>();
45-
const removed = new Map<string, (string | null)[]>();
50+
const removed = new Map<string, Evidence>();
4651
// Deletions retain metadata even after the file leaves the tree.
47-
const metadata = new Map<string, (string | null)[]>();
52+
const metadata = new Map<string, Evidence>();
4853
let parent = history.base;
4954
for (const commit of history.commits) {
5055
if (commit.parent !== parent) throw new Error('Linking requires a contiguous linear history.');
@@ -53,11 +58,14 @@ export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap<st
5358
if (owner !== null && !plan.items.some(item => item.id === owner)) throw new Error(`Unknown ledger item: ${owner}`);
5459
for (const delta of commit.files) {
5560
const oldPath = delta.oldPath;
56-
const path = delta.newPath ?? oldPath!;
61+
const item = plan.items.find(item => item.id === owner);
62+
const declared = new Set(item?.files.flatMap(file => [file.path, ...(file.renamed_from ? [file.renamed_from] : [])]));
63+
const touched = [delta.oldPath, delta.newPath].filter((path): path is string => path !== null);
64+
const current: Evidence = { owners: [owner], outOfScope: owner !== null && touched.some(path => !declared.has(path)) ? [owner] : [] };
5765
let previous = oldPath ? files.get(oldPath) : undefined;
5866
if (!previous) previous = {
59-
lines: lines(textFile(delta.before) ? delta.before!.text : '').map((text, i) => ({ text, owners: [], origins: [origin(oldPath!, i)], movedOwners: [] })),
60-
metadataOwners: [],
67+
lines: lines(textFile(delta.before) ? delta.before!.text : '').map((text, i) => ({ text, evidence: empty(), origins: [origin(oldPath!, i)], moved: empty() })),
68+
metadata: empty(),
6169
};
6270
const next: TrackedLine[] = [];
6371
const changes = diffArrays(previous.lines.map(line => line.text), lines(textFile(delta.after) ? delta.after!.text : ''), { timeout: 2000 });
@@ -68,22 +76,22 @@ export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap<st
6876
if (!change.added && !change.removed) { next.push(...previous.lines.slice(cursor, cursor + change.value.length)); cursor += change.value.length; continue; }
6977
const deleted = change.removed ? previous.lines.slice(cursor, cursor + change.value.length) : [];
7078
if (change.removed) cursor += change.value.length;
71-
const owners = unique([...deleted.flatMap(line => line.owners), owner]);
79+
const evidence = combine(...deleted.map(line => line.evidence), current);
7280
const origins = unique(deleted.flatMap(line => line.origins));
73-
for (const id of origins) removed.set(id, owners);
81+
for (const id of origins) removed.set(id, evidence);
7482
const added = change.added ? change : changes[n + 1]?.added ? changes[++n]! : undefined;
75-
if (added) for (const text of added.value) next.push({ text, owners, origins, movedOwners: unique(deleted.flatMap(line => line.movedOwners)) });
83+
if (added) for (const text of added.value) next.push({ text, evidence, origins, moved: combine(...deleted.map(line => line.moved)) });
7684
}
7785
if (oldPath && delta.newPath && oldPath !== delta.newPath) {
7886
for (let i = 0; i < next.length; i++) {
7987
const line = next[i]!;
80-
next[i] = { ...line, movedOwners: unique([...line.movedOwners, owner]) };
81-
for (const id of line.origins) if (!removed.has(id)) removed.set(id, unique([...line.owners, owner]));
88+
next[i] = { ...line, moved: combine(line.moved, current) };
89+
for (const id of line.origins) if (!removed.has(id)) removed.set(id, combine(line.evidence, current));
8290
}
8391
}
84-
const metadataOwners = metadataChange(delta) ? unique([...previous.metadataOwners, owner]) : previous.metadataOwners;
85-
if (oldPath) { files.delete(oldPath); metadata.set(oldPath, metadataOwners); }
86-
if (delta.newPath) { files.set(delta.newPath, { lines: next, metadataOwners }); metadata.set(delta.newPath, metadataOwners); }
92+
const metadataEvidence = metadataChange(delta) ? combine(previous.metadata, current) : previous.metadata;
93+
if (oldPath) { files.delete(oldPath); metadata.set(oldPath, metadataEvidence); }
94+
if (delta.newPath) { files.set(delta.newPath, { lines: next, metadata: metadataEvidence }); metadata.set(delta.newPath, metadataEvidence); }
8795
}
8896
}
8997
if (parent !== history.head) throw new Error('History does not end at the requested head.');
@@ -96,17 +104,17 @@ export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap<st
96104
let oldIndex = 0, newIndex = 0, hunk = 0;
97105
const fileSegments: Segment[] = [];
98106
const affectedPaths = unique([delta.oldPath, delta.newPath].filter((p): p is string => p !== null));
99-
const push = (part: Omit<Segment, 'row' | 'scope' | 'sharesHunkWith'>) => fileSegments.push({
100-
...part, ...classify(plan, part.owners, affectedPaths), sharesHunkWith: [],
107+
const push = (part: Omit<Segment, 'row' | 'scope' | 'sharesHunkWith' | 'owners'>, evidence: Evidence) => fileSegments.push({
108+
...part, owners: evidence.owners, ...classify(evidence), sharesHunkWith: [],
101109
});
102110
if (metadataChange(delta)) {
103-
const owners = unique(affectedPaths.flatMap(path => metadata.get(path) ?? []));
104-
push({ path, oldPath: delta.oldPath, kind: 'file', owners, oldLine: null, newLine: null,
111+
const evidence = combine(...affectedPaths.map(path => metadata.get(path) ?? empty()));
112+
push({ path, oldPath: delta.oldPath, kind: 'file', oldLine: null, newLine: null,
105113
operation: null, context: '', hunk: -1,
106114
content: JSON.stringify({ oldPath: delta.oldPath, newPath: delta.newPath,
107115
oldMode: delta.before?.mode ?? null, newMode: delta.after?.mode ?? null,
108116
oldOid: delta.before?.oid ?? null, newOid: delta.after?.oid ?? null }),
109-
});
117+
}, evidence);
110118
}
111119
const finalChanges = diffArrays(oldLines, newLines, { timeout: 2000 });
112120
if (!finalChanges) throw new Error('Final diff exceeded the time budget.');
@@ -118,16 +126,16 @@ export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap<st
118126
}
119127
for (const text of change.value) {
120128
const trackedLine = tracked?.lines[newIndex];
121-
const owners = change.added
122-
? trackedLine?.owners.length ? trackedLine.owners : trackedLine?.movedOwners ?? []
123-
: removed.get(origin(delta.oldPath!, oldIndex)) ?? [];
129+
const evidence = change.added
130+
? trackedLine?.evidence.owners.length ? trackedLine.evidence : trackedLine?.moved ?? empty()
131+
: removed.get(origin(delta.oldPath!, oldIndex)) ?? empty();
124132
const context = delta.contexts.find(range => change.added
125133
? newIndex + 1 >= range.newStart && newIndex + 1 < range.newStart + range.newCount
126134
: oldIndex + 1 >= range.oldStart && oldIndex + 1 < range.oldStart + range.oldCount)?.name ?? '';
127-
push({ path, oldPath: delta.oldPath, kind: 'text', owners, content: text, context, hunk,
135+
push({ path, oldPath: delta.oldPath, kind: 'text', content: text, context, hunk,
128136
oldLine: change.removed ? oldIndex + 1 : null, newLine: change.added ? newIndex + 1 : null,
129137
operation: change.added ? '+' : '-',
130-
});
138+
}, evidence);
131139
if (change.added) newIndex++; else oldIndex++;
132140
}
133141
}
@@ -136,7 +144,7 @@ export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap<st
136144
for (const part of fileSegments) {
137145
const last = grouped.at(-1);
138146
if (last && part.kind === 'text' && last.kind === 'text' && last.hunk === part.hunk &&
139-
last.operation === part.operation && last.context === part.context &&
147+
last.operation === part.operation && last.context === part.context && last.scope === part.scope &&
140148
(part.operation === '+' ? last.newLine! + lines(last.content).length === part.newLine : last.oldLine! + lines(last.content).length === part.oldLine) &&
141149
JSON.stringify(last.owners) === JSON.stringify(part.owners)) last.content += part.content;
142150
else grouped.push({ ...part });

‎docs/implementation/build-step-1.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ A focused independent review found BOM-only changes could disappear because the
3131

3232
The rename case first failed (moved lines became Unplanned), then passed after the provenance fix. `npm run typecheck` checks all source and tests. The public examples and shared schema definitions are checked on every test run.
3333

34+
## Review round 1
35+
36+
Copilot reported two findings. Fixed scope after rename: scope now travels with each owning change at its actual path, instead of comparing all segments against both ends of the final rename. Real rename/edit and rename/delete cases failed before the fix and pass after it; declaring only the historical name does not authorize the destination.
37+
38+
Declined the claim that repository aliases can override `rev-parse`, `rev-list`, `diff`, or `cat-file`: these are built-in commands, and Git ignores aliases that shadow them. A scratch probe and a permanent adapter regression confirmed the shell alias never ran. See [Git's alias documentation](https://git-scm.com/docs/git-config/2.54.0).
39+
3440
## Remaining gates
3541

3642
This is a working foundation, not a completed application or a claim that all implementation tasks are done. T18's pure validation/edit core is present; its agent adapters, import UI, and persistence are pending. Ledger storage, rebase mappings, and the read-only review screen remain next. The already-fixed GitHub check belongs to the later GitHub/runner integration.

‎test/history.test.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, chmodSync, symlinkSync, renameSync } from 'node:fs';
1+
import { mkdtempSync, mkdirSync, writeFileSync, existsSync, rmSync, chmodSync, symlinkSync, renameSync } from 'node:fs';
22
import { tmpdir } from 'node:os';
33
import { join, dirname } from 'node:path';
44
import { execFileSync } from 'node:child_process';
@@ -136,3 +136,28 @@ it('reads the whole repository even when called from a subdirectory with relativ
136136
const parts = linkHistory(f.plan, readHistory(join(f.dir, 'sub'), f.base), f.ledger);
137137
expect(new Set(parts.map(s => s.path))).toEqual(new Set(['a.txt', 'sub/b.txt']));
138138
});
139+
it('checks scope at each owning commit, not against both ends of a final rename', () => {
140+
const f = fixture();
141+
f.plan.items[0]!.files = [{ path: 'b.txt', kind: 'rename', renamed_from: 'a.txt', change: 'Move' }];
142+
f.plan.items[1]!.files = [{ path: 'b.txt', kind: 'edit', renamed_from: null, change: 'Edit new name' }];
143+
renameSync(join(f.dir, 'a.txt'), join(f.dir, 'b.txt')); f.commit('P1');
144+
f.write('b.txt', 'ONE\ntwo\nthree\n'); f.commit('P2');
145+
expect(f.segments().filter(s => s.row === 'P2').every(s => s.scope === 'in-scope')).toBe(true);
146+
// Declaring only the old name must not authorize edits to the new one.
147+
f.plan.items[1]!.files[0]!.path = 'a.txt';
148+
expect(f.segments().filter(s => s.row === 'P2').every(s => s.scope === 'out-of-scope')).toBe(true);
149+
});
150+
it('checks scope of deletion after a rename using the deleted current path', () => {
151+
const f = fixture(); f.plan.items[0]!.files = [{ path: 'b.txt', kind: 'rename', renamed_from: 'a.txt', change: 'Move' }];
152+
f.plan.items[1]!.files = [{ path: 'b.txt', kind: 'delete', renamed_from: null, change: 'Delete' }];
153+
renameSync(join(f.dir, 'a.txt'), join(f.dir, 'b.txt')); f.commit('P1');
154+
rmSync(join(f.dir, 'b.txt')); f.commit('P2');
155+
expect(f.segments().filter(s => s.row === 'P2').every(s => s.scope === 'in-scope')).toBe(true);
156+
});
157+
158+
it('Git built-ins cannot be overridden by repository shell aliases', () => {
159+
const f = fixture(); f.write('a.txt', 'changed\n'); f.commit('P1');
160+
for (const name of ['rev-parse', 'rev-list', 'diff', 'cat-file']) f.git('config', `alias.${name}`, '!touch alias-executed');
161+
expect(f.segments().length).toBeGreaterThan(0);
162+
expect(existsSync(join(f.dir, 'alias-executed'))).toBe(false);
163+
});

0 commit comments

Comments
 (0)