Skip to content

Commit f35c797

Browse files
authored
fix: add CODEOWNERS parse cache and tighten email validation (#222)
* feat: add CODEOWNERS module with parsing, matching, and ownership data Adds src/owners.js implementing CODEOWNERS parsing (3 standard locations), glob-to-regex pattern matching (**, *, ?, anchored/unanchored), last-match-wins owner resolution, file-level and symbol-level ownership data, cross-owner boundary detection, and diff-impact integration helper. Includes unit tests (parseCodeownersContent, patternToRegex, matchOwners, parseCodeowners) and integration tests (ownersData, ownersForFiles with fixture DB and CODEOWNERS file). Also updates BACKLOG.md to mark branch structural diff as done and reorganize backlog items. * fix: add parseCodeowners cache and tighten email validation Cache CODEOWNERS parse results per rootDir, invalidated by file mtime. Replace permissive `p.includes('@')` email filter with regex that requires non-empty local and domain parts. Impact: 3 functions changed, 4 affected * fix(test): set mtime after write for Windows NTFS compatibility NTFS can lazily update mtime on writeFileSync, causing the cache invalidation test to see a stale timestamp. Set mtime explicitly after writing to guarantee it differs from the cached value. * fix: restore BACKLOG.md to main branch version Remove stale BACKLOG.md changes that leaked from outdated branch base, reverting completion status of 6 items and the date regression. * fix: sync BACKLOG.md with latest main (restore 6 completed items) Previous commit restored from stale local main. This fetches from origin/main to correctly preserve all DONE markers for IDs 5, 15, 16, 18, 20, 31 and the current date.
1 parent 850ef3e commit f35c797

File tree

3 files changed

+63
-5
lines changed

3 files changed

+63
-5
lines changed

src/owners.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,46 @@
11
import fs from 'node:fs';
22
import path from 'node:path';
33
import { findDbPath, openReadonlyOrFail } from './db.js';
4-
54
import { isTestFile } from './queries.js';
65

76
// ─── CODEOWNERS Parsing ──────────────────────────────────────────────
87

98
const CODEOWNERS_PATHS = ['CODEOWNERS', '.github/CODEOWNERS', 'docs/CODEOWNERS'];
109

10+
/** @type {Map<string, { rules: Array, path: string, mtime: number }>} */
11+
const codeownersCache = new Map();
12+
1113
/**
1214
* Find and parse a CODEOWNERS file from the standard locations.
15+
* Results are cached per rootDir and invalidated when the file's mtime changes.
1316
* @param {string} rootDir - Repository root directory
1417
* @returns {{ rules: Array<{pattern: string, owners: string[], regex: RegExp}>, path: string } | null}
1518
*/
1619
export function parseCodeowners(rootDir) {
20+
const cached = codeownersCache.get(rootDir);
21+
1722
for (const rel of CODEOWNERS_PATHS) {
1823
const fullPath = path.join(rootDir, rel);
1924
if (fs.existsSync(fullPath)) {
25+
const mtime = fs.statSync(fullPath).mtimeMs;
26+
if (cached && cached.path === rel && cached.mtime === mtime) {
27+
return { rules: cached.rules, path: cached.path };
28+
}
2029
const content = fs.readFileSync(fullPath, 'utf-8');
21-
return { rules: parseCodeownersContent(content), path: rel };
30+
const rules = parseCodeownersContent(content);
31+
codeownersCache.set(rootDir, { rules, path: rel, mtime });
32+
return { rules, path: rel };
2233
}
2334
}
35+
codeownersCache.delete(rootDir);
2436
return null;
2537
}
2638

39+
/** Clear the parseCodeowners cache (for testing). */
40+
export function clearCodeownersCache() {
41+
codeownersCache.clear();
42+
}
43+
2744
/**
2845
* Parse CODEOWNERS file content into rules.
2946
* @param {string} content - Raw CODEOWNERS file content
@@ -37,7 +54,7 @@ export function parseCodeownersContent(content) {
3754
const parts = line.split(/\s+/);
3855
if (parts.length < 2) continue;
3956
const pattern = parts[0];
40-
const owners = parts.slice(1).filter((p) => p.startsWith('@') || p.includes('@'));
57+
const owners = parts.slice(1).filter((p) => p.startsWith('@') || /^[^@\s]+@[^@\s]+$/.test(p));
4158
if (owners.length === 0) continue;
4259
rules.push({ pattern, owners, regex: patternToRegex(pattern) });
4360
}

tests/integration/owners.test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import fs from 'node:fs';
22
import os from 'node:os';
33
import path from 'node:path';
44
import Database from 'better-sqlite3';
5-
65
import { afterAll, beforeAll, describe, expect, test } from 'vitest';
76
import { initSchema } from '../../src/db.js';
87
import { ownersData, ownersForFiles } from '../../src/owners.js';

tests/unit/owners.test.js

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import fs from 'node:fs';
22
import os from 'node:os';
33
import path from 'node:path';
44
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
5-
65
import {
6+
clearCodeownersCache,
77
matchOwners,
88
parseCodeowners,
99
parseCodeownersContent,
@@ -37,6 +37,22 @@ describe('parseCodeownersContent', () => {
3737
expect(rules[0].owners).toEqual(['dev@example.com', '@python-team']);
3838
});
3939

40+
it('rejects malformed email-style owners', () => {
41+
// foo@ has no domain — rejected by the email regex
42+
// @bar passes as a GitHub handle (startsWith('@'))
43+
const rules = parseCodeownersContent('*.js foo@ @bar');
44+
expect(rules).toHaveLength(1);
45+
expect(rules[0].owners).toEqual(['@bar']);
46+
});
47+
48+
it('rejects @bar-only-at as email (no local part)', () => {
49+
// Standalone token that looks like @domain but not a handle check
50+
// This tests the email path: '@bar' would pass startsWith('@'), which is fine
51+
// But 'nothandle' without @ is rejected, and 'a@' without domain is rejected
52+
const rules = parseCodeownersContent('*.js a@ nothandle');
53+
expect(rules).toHaveLength(0);
54+
});
55+
4056
it('skips lines with no owners', () => {
4157
const rules = parseCodeownersContent('*.js\nsrc/ @team');
4258
expect(rules).toHaveLength(1);
@@ -147,6 +163,7 @@ describe('parseCodeowners', () => {
147163
let tmpDir;
148164

149165
beforeEach(() => {
166+
clearCodeownersCache();
150167
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codeowners-test-'));
151168
});
152169

@@ -190,4 +207,29 @@ describe('parseCodeowners', () => {
190207
expect(result.path).toBe('CODEOWNERS');
191208
expect(result.rules[0].owners).toEqual(['@root']);
192209
});
210+
211+
it('returns cached result on repeated calls', () => {
212+
fs.writeFileSync(path.join(tmpDir, 'CODEOWNERS'), '* @team\n');
213+
const first = parseCodeowners(tmpDir);
214+
const second = parseCodeowners(tmpDir);
215+
expect(second).toEqual(first);
216+
// Same rule array reference means cache was hit
217+
expect(second.rules).toBe(first.rules);
218+
});
219+
220+
it('invalidates cache when file mtime changes', () => {
221+
const filePath = path.join(tmpDir, 'CODEOWNERS');
222+
fs.writeFileSync(filePath, '* @old-team\n');
223+
const first = parseCodeowners(tmpDir);
224+
expect(first.rules[0].owners).toEqual(['@old-team']);
225+
226+
// Write new content then force a distinct mtime (NTFS can lazily update mtime)
227+
fs.writeFileSync(filePath, '* @new-team\n');
228+
const afterWrite = fs.statSync(filePath).mtimeMs;
229+
fs.utimesSync(filePath, new Date(), new Date(afterWrite + 5000));
230+
231+
const second = parseCodeowners(tmpDir);
232+
expect(second.rules[0].owners).toEqual(['@new-team']);
233+
expect(second.rules).not.toBe(first.rules);
234+
});
193235
});

0 commit comments

Comments
 (0)