Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions .github/scripts/change-scope.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ const SPEC_RECORD_PATTERNS = [

const CHANGESET_PATTERN = /^\.changeset\/(?!README\.md$)[^/]+\.md$/;

const THEME_DOC_CANDIDATE = /^docs\/themes\/(?!README\.md$)[^/]+\.md$/;
const THEME_DOC_PREFIX = 'docs/themes/';
const THEME_DOC_GUIDANCE = `${THEME_DOC_PREFIX}README.md`;
const THEME_PACKAGE_CANDIDATE =
/^packages\/themes\/[^/]+\/(?:.*\/)?[^/]+\.spec\.md$/;

const KNOWLEDGE_RECORD_PATTERNS = [
...SPEC_RECORD_PATTERNS,
THEME_DOC_CANDIDATE,
THEME_PACKAGE_CANDIDATE,
/^docs\/architecture\/(?!README\.md$)[^/]+\.md$/,
/^docs\/design\/assets\//,
Expand Down Expand Up @@ -59,8 +59,13 @@ function isPackageReleasePath(filePath) {
}

function isKnowledgeRecordPath(filePath) {
const isThemeDocCandidate =
filePath.startsWith(THEME_DOC_PREFIX) &&
filePath.endsWith('.md') &&
filePath !== THEME_DOC_GUIDANCE;
return (
isSpecRecordPath(filePath) ||
isThemeDocCandidate ||
KNOWLEDGE_RECORD_PATTERNS.some(pattern => pattern.test(filePath))
);
}
Expand Down
29 changes: 29 additions & 0 deletions .github/scripts/change-scope.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,37 @@ describe('spec-only change scope', () => {
expect(classifyChanges([{filename}]).specOnly).toBe(false);
});

it('classifies allowed, canonical, and nested theme paths distinctly', () => {
const guidance = classifyChanges([{filename: 'docs/themes/README.md'}]);
expect(guidance.touchesKnowledgeRecords).toBe(false);
expect(guidance.specOnly).toBe(false);

const canonical = classifyChanges([
{filename: 'packages/themes/neutral/neutral.spec.md'},
]);
expect(canonical.touchesKnowledgeRecords).toBe(true);
expect(canonical.specOnly).toBe(true);

const nestedCandidate = classifyChanges([
{filename: 'docs/themes/subdir/neutral.md'},
]);
expect(nestedCandidate.touchesKnowledgeRecords).toBe(true);
expect(nestedCandidate.specOnly).toBe(false);

const minimalCandidate = classifyChanges([{filename: 'docs/themes/.md'}]);
expect(minimalCandidate.touchesKnowledgeRecords).toBe(true);
expect(minimalCandidate.specOnly).toBe(false);

const nonMarkdown = classifyChanges([
{filename: 'docs/themes/subdir/neutral.md.txt'},
]);
expect(nonMarkdown.touchesKnowledgeRecords).toBe(false);
expect(nonMarkdown.specOnly).toBe(false);
});

it.each([
'docs/themes/neutral.md',
'docs/themes/subdir/neutral.md',
'packages/themes/neutral/Theme.spec.md',
'packages/themes/neutral/subdir/neutral.spec.md',
])('treats misplaced theme candidate %s as unsafe knowledge', filename => {
Expand Down
25 changes: 25 additions & 0 deletions .github/scripts/spec-owner-reconcile.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,33 @@ describe('spec owner workflow reconciliation', () => {
expect(harness.state.pr.auto_merge).toBe(null);
});

it('treats the allowed theme guidance index as no knowledge change', async () => {
const harness = createHarness({
changedFile: {filename: 'docs/themes/README.md', status: 'modified'},
headContent: '# Theme guidance\n',
baseContent: '# Theme guidance\n',
});

await run(
harness,
context({
runId: 100n,
action: 'synchronize',
actor: 'cixzhang',
}),
);

expect(latestGateStatus(harness.state)).toMatchObject({
state: 'success',
description: 'No knowledge records changed.',
});
expect(harness.state.calls).not.toContain('enable-auto-merge');
});

it.each([
'docs/themes/neutral.md',
'docs/themes/.md',
'docs/themes/subdir/neutral.md',
'packages/themes/neutral/Theme.spec.md',
'packages/themes/neutral/subdir/neutral.spec.md',
])(
Expand Down
37 changes: 30 additions & 7 deletions scripts/check-knowledge.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function matchingFiles(directory, predicate) {
function matchingFilesRecursively(
directory,
predicate,
{skipDirectory = () => false} = {},
{skipDirectory = () => false, includeSymlinks = false} = {},
) {
if (!fs.existsSync(directory)) return [];
const matches = [];
Expand All @@ -134,7 +134,10 @@ function matchingFilesRecursively(
continue;
}
pending.push(candidate);
} else if (entry.isFile() && predicate(entry.name, candidate)) {
} else if (
(entry.isFile() || (includeSymlinks && entry.isSymbolicLink())) &&
predicate(entry.name, candidate)
) {
matches.push(candidate);
}
}
Expand All @@ -146,11 +149,31 @@ export function discoverThemeRecordCandidates(root = DEFAULT_ROOT) {
const records = [];
const problems = [];

for (const filePath of matchingFiles(
path.join(root, 'docs/themes'),
name => name.endsWith('.md') && name !== 'README.md',
)) {
records.push(filePath);
const themeDocsDirectory = path.join(root, 'docs/themes');
const allowedThemeDocs = new Set([
path.join(themeDocsDirectory, 'README.md'),
]);
let themeDocsIsDirectory = false;
try {
themeDocsIsDirectory = fs.lstatSync(themeDocsDirectory).isDirectory();
if (!themeDocsIsDirectory) {
problems.push(
'docs/themes: theme guidance root must be a directory and may not be a symlink.',
);
}
} catch (error) {
if (error.code !== 'ENOENT') throw error;
}
for (const filePath of themeDocsIsDirectory
? matchingFilesRecursively(
themeDocsDirectory,
name => name.endsWith('.md'),
{includeSymlinks: true},
)
: []) {
const isRegularFile = fs.lstatSync(filePath).isFile();
if (isRegularFile && allowedThemeDocs.has(filePath)) continue;
if (isRegularFile) records.push(filePath);
problems.push(
`${path.relative(root, filePath)}: theme records must be placed at packages/themes/<theme>/<theme>.spec.md; docs/themes contains guidance only.`,
);
Expand Down
71 changes: 71 additions & 0 deletions scripts/check-knowledge.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1175,12 +1175,83 @@ describe('knowledge validation', () => {
expect(await validateKnowledgeRoot(root)).toEqual([]);
});

it('does not follow symlink directories while scanning theme guidance', async () => {
const root = fixtureRoot();
const outside = path.join(root, 'outside-theme-docs');
fs.mkdirSync(outside);
fs.writeFileSync(path.join(outside, 'neutral.md'), themeRecord());
fs.symlinkSync(outside, path.join(root, 'docs/themes/linked'), 'dir');

const discovered = discoverKnowledgeRecords(root).map(filePath =>
path.relative(root, filePath),
);
expect(discovered).not.toContain('docs/themes/linked/neutral.md');
expect(await validateKnowledgeRoot(root)).toEqual([]);
});

it('rejects a symlinked theme guidance root without following it', async () => {
const root = fixtureRoot();
const outside = path.join(root, 'outside-theme-docs');
fs.mkdirSync(outside);
fs.writeFileSync(
path.join(outside, 'README.md'),
'# Not repository guidance\n',
);
fs.rmSync(path.join(root, 'docs/themes'), {recursive: true});
fs.symlinkSync(outside, path.join(root, 'docs/themes'), 'dir');

const problems = (await validateKnowledgeRoot(root)).join('\n');
expect(problems).toMatch(
/docs\/themes: theme guidance root must be a directory and may not be a symlink/,
);
});

it('rejects Markdown symlinks without following their targets', async () => {
const root = fixtureRoot();
const outside = path.join(root, 'outside-theme.md');
fs.writeFileSync(outside, themeRecord());
fs.symlinkSync(outside, path.join(root, 'docs/themes/linked.md'));

const discovered = discoverKnowledgeRecords(root).map(filePath =>
path.relative(root, filePath),
);
expect(discovered).not.toContain('docs/themes/linked.md');
const problems = (await validateKnowledgeRoot(root)).join('\n');
expect(problems).toMatch(
/docs\/themes\/linked\.md: theme records must be placed/,
);
expect(problems).not.toMatch(/outside-theme\.md/);
});

it('ignores nested non-Markdown files', async () => {
const root = fixtureRoot();
const nested = path.join(root, 'docs/themes/subdir/neutral.md.txt');
fs.mkdirSync(path.dirname(nested), {recursive: true});
fs.writeFileSync(nested, themeRecord());

const discovered = discoverKnowledgeRecords(root).map(filePath =>
path.relative(root, filePath),
);
expect(discovered).not.toContain('docs/themes/subdir/neutral.md.txt');
expect(await validateKnowledgeRoot(root)).toEqual([]);
});

it.each([
[
'docs guidance directory',
'docs/themes/neutral.md',
/theme records must be placed at packages\/themes\/<theme>\/<theme>\.spec\.md/,
],
[
'minimal Markdown filename',
'docs/themes/.md',
/theme records must be placed at packages\/themes\/<theme>\/<theme>\.spec\.md/,
],
[
'nested docs guidance path',
'docs/themes/subdir/neutral.md',
/theme records must be placed at packages\/themes\/<theme>\/<theme>\.spec\.md/,
],
[
'wrong package filename',
'packages/themes/neutral/Theme.spec.md',
Expand Down
Loading