Skip to content

Commit 67ae3e4

Browse files
tyaginidhiclaude
andcommitted
Add regression tests + doc for the legacy-mirror symlink guards
Addresses the two non-blocking gaps raised in review of #206. Test coverage gap: CI only ran validate-legacy-compatibility.js against the real (always-clean) repo, exercising the happy path but never the symlink-rejection / drift-detection logic — a future refactor could drop assertRegularFile and CI would stay green. Add a LEGACY_COMPAT_ROOT seam so the script can validate a fixture tree, and scripts/tests/validate-legacy-compatibility.test.js (node:test) which plants each regression — symlinked manifest, symlinked CLAUDE.md, dangling CLAUDE.md symlink, content drift — and asserts the validator exits non-zero. Wired into CI as a new `node --test scripts/tests/` step in validate-repository-metadata. Symlink-creation cases self-skip where the platform forbids symlinks; the ubuntu CI runner runs them all. Doc gap: the Legacy Marketplace Compatibility section documented the manifest-mirror sync rule but not the new CLAUDE.md one. Add a paragraph stating each CLAUDE.md is a committed real-file mirror of its sibling AGENTS.md (never a symlink) that must be updated in the same change, enforced by the validator + its tests. Re-synced root CLAUDE.md to match the edited AGENTS.md. Verified: 7/7 tests pass; validator still green on the real repo; plugin-names and skill-descriptions checks unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9879a03 commit 67ae3e4

5 files changed

Lines changed: 210 additions & 1 deletion

File tree

.github/workflows/validate-repository-metadata.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,11 @@ jobs:
2525
- name: validate-legacy-compatibility
2626
run: node scripts/validate-legacy-compatibility.js
2727

28+
# Runs the script against the real repo above; this exercises its
29+
# symlink-rejection / mirror-drift logic against fixture trees so the guard
30+
# can't silently rot. ubuntu runner creates symlinks, so no cases are skipped.
31+
- name: validate-legacy-compatibility-tests
32+
run: node --test scripts/tests/
33+
2834
- name: validate-skill-descriptions
2935
run: node scripts/validate-skill-descriptions.js

AGENTS.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ reinstall. Because mirrors are committed files (not symlinks), update both sourc
106106
and legacy copies together, then run
107107
`node scripts/validate-legacy-compatibility.js` after metadata changes.
108108

109+
Each `CLAUDE.md` is likewise a committed **real-file mirror** of its sibling
110+
`AGENTS.md` (root and per-plugin), for the same reason: a symlink materializes as a
111+
broken text file on Windows clones without Developer Mode. So when you edit an
112+
`AGENTS.md`, copy it over the sibling `CLAUDE.md` in the same change — neither file
113+
may be a symlink. `validate-legacy-compatibility.js` enforces both rules (regular
114+
file + identical content) and runs in CI via `validate-repository-metadata`; its
115+
fixture-based regression tests live at `scripts/tests/` (`node --test scripts/tests/`).
116+
109117
## Code Conventions
110118

111119
**DRY (Don't Repeat Yourself):** Never duplicate logic across files. Each plugin has shared utilities (e.g., `scripts/lib/`) and shared reference docs (e.g., `references/`). Always check for and reuse existing helpers before writing new code. When adding shared logic, put it in the plugin's shared modules — not in individual skill directories.

CLAUDE.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ reinstall. Because mirrors are committed files (not symlinks), update both sourc
106106
and legacy copies together, then run
107107
`node scripts/validate-legacy-compatibility.js` after metadata changes.
108108

109+
Each `CLAUDE.md` is likewise a committed **real-file mirror** of its sibling
110+
`AGENTS.md` (root and per-plugin), for the same reason: a symlink materializes as a
111+
broken text file on Windows clones without Developer Mode. So when you edit an
112+
`AGENTS.md`, copy it over the sibling `CLAUDE.md` in the same change — neither file
113+
may be a symlink. `validate-legacy-compatibility.js` enforces both rules (regular
114+
file + identical content) and runs in CI via `validate-repository-metadata`; its
115+
fixture-based regression tests live at `scripts/tests/` (`node --test scripts/tests/`).
116+
109117
## Code Conventions
110118

111119
**DRY (Don't Repeat Yourself):** Never duplicate logic across files. Each plugin has shared utilities (e.g., `scripts/lib/`) and shared reference docs (e.g., `references/`). Always check for and reuse existing helpers before writing new code. When adding shared logic, put it in the plugin's shared modules — not in individual skill directories.
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
const test = require('node:test');
2+
const assert = require('node:assert/strict');
3+
const fs = require('fs');
4+
const os = require('os');
5+
const path = require('path');
6+
const { spawnSync } = require('child_process');
7+
8+
// These tests guard the guard: scripts/validate-legacy-compatibility.js exists to keep
9+
// the legacy .claude-plugin manifests (and CLAUDE.md) committed as real, parseable files
10+
// rather than symlinks — the issue #201 Windows failure mode. CI only ever runs that
11+
// script against the real (always-clean) repo, so the happy path is covered but the
12+
// rejection logic is not; a future refactor could drop assertRegularFile and CI would
13+
// stay green. Here we point the validator at throwaway fixture trees (via
14+
// LEGACY_COMPAT_ROOT) and assert it actually FAILS on each planted regression.
15+
// See: https://github.com/microsoft/power-platform-skills/issues/201
16+
17+
const VALIDATOR = path.join(__dirname, '..', 'validate-legacy-compatibility.js');
18+
19+
function writeJson(filePath, value) {
20+
fs.mkdirSync(path.dirname(filePath), { recursive: true });
21+
// Pretty-print + trailing newline so JSON mirrors are byte-identical the way the real
22+
// committed files are (deepEqual ignores formatting, but keeping it realistic).
23+
fs.writeFileSync(filePath, `${JSON.stringify(value, null, 2)}\n`, 'utf8');
24+
}
25+
26+
// fs.symlinkSync needs SeCreateSymbolicLink on Windows (Developer Mode / admin); CI is
27+
// Linux where it always works. If a dev box can't make symlinks, skip that case rather
28+
// than fail — the Linux CI run is the authoritative one.
29+
function trySymlink(target, linkPath) {
30+
try {
31+
if (fs.existsSync(linkPath) || fs.lstatSync(linkPath)) {
32+
fs.rmSync(linkPath, { force: true });
33+
}
34+
} catch {
35+
/* nothing to remove */
36+
}
37+
try {
38+
fs.symlinkSync(target, linkPath);
39+
return true;
40+
} catch (error) {
41+
if (error.code === 'EPERM' || error.code === 'ENOSYS') {
42+
return false;
43+
}
44+
throw error;
45+
}
46+
}
47+
48+
// Build a minimal marketplace tree that PASSES every check, so each test can then mutate
49+
// exactly one thing and attribute the resulting failure to that mutation.
50+
function buildFixture(t) {
51+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'legacy-compat-'));
52+
t.after(() => fs.rmSync(root, { recursive: true, force: true }));
53+
54+
const marketplace = {
55+
name: 'fixture-marketplace',
56+
metadata: { pluginRoot: '.' },
57+
plugins: [
58+
{
59+
name: 'foo',
60+
source: './plugins/foo',
61+
description: 'Foo plugin',
62+
category: 'development',
63+
version: '1.0.0',
64+
tags: ['x', 'y'],
65+
keywords: ['x', 'y'],
66+
},
67+
],
68+
};
69+
// Legacy marketplace mirror is a byte-for-byte copy of the Open Plugins marketplace.
70+
writeJson(path.join(root, 'marketplace.json'), marketplace);
71+
writeJson(path.join(root, '.claude-plugin', 'marketplace.json'), marketplace);
72+
73+
const pluginManifest = {
74+
name: 'foo',
75+
version: '1.0.0',
76+
description: 'Foo plugin',
77+
keywords: ['x', 'y'],
78+
};
79+
const pluginDir = path.join(root, 'plugins', 'foo');
80+
writeJson(path.join(pluginDir, '.plugin', 'plugin.json'), pluginManifest);
81+
writeJson(path.join(pluginDir, '.claude-plugin', 'plugin.json'), pluginManifest);
82+
83+
// CLAUDE.md is a real copy of the sibling AGENTS.md, at root and in the plugin dir,
84+
// so both checkClaudeMirror(ROOT) and checkClaudeMirror(pluginDir) are exercised.
85+
for (const dir of [root, pluginDir]) {
86+
const agents = `# AGENTS for ${path.basename(dir)}\n\nGuidance.\n`;
87+
fs.writeFileSync(path.join(dir, 'AGENTS.md'), agents, 'utf8');
88+
fs.writeFileSync(path.join(dir, 'CLAUDE.md'), agents, 'utf8');
89+
}
90+
91+
return root;
92+
}
93+
94+
function runValidator(root) {
95+
const result = spawnSync(process.execPath, [VALIDATOR], {
96+
env: { ...process.env, LEGACY_COMPAT_ROOT: root },
97+
encoding: 'utf8',
98+
});
99+
return { status: result.status, output: `${result.stdout || ''}${result.stderr || ''}` };
100+
}
101+
102+
test('passes on a clean mirror tree', (t) => {
103+
const root = buildFixture(t);
104+
const { status, output } = runValidator(root);
105+
assert.equal(status, 0, output);
106+
assert.match(output, /in sync/);
107+
});
108+
109+
test('fails when the legacy marketplace manifest is a symlink', (t) => {
110+
const root = buildFixture(t);
111+
const legacy = path.join(root, '.claude-plugin', 'marketplace.json');
112+
fs.rmSync(legacy);
113+
if (!trySymlink('../marketplace.json', legacy)) {
114+
t.skip('symlink creation not permitted on this platform');
115+
return;
116+
}
117+
const { status, output } = runValidator(root);
118+
assert.equal(status, 1, output);
119+
assert.match(output, /regular file/);
120+
});
121+
122+
test('fails when a legacy plugin manifest is a symlink', (t) => {
123+
const root = buildFixture(t);
124+
const legacy = path.join(root, 'plugins', 'foo', '.claude-plugin', 'plugin.json');
125+
fs.rmSync(legacy);
126+
if (!trySymlink(path.join('..', '.plugin', 'plugin.json'), legacy)) {
127+
t.skip('symlink creation not permitted on this platform');
128+
return;
129+
}
130+
const { status, output } = runValidator(root);
131+
assert.equal(status, 1, output);
132+
assert.match(output, /regular file/);
133+
});
134+
135+
test('fails when CLAUDE.md is a (resolving) symlink', (t) => {
136+
const root = buildFixture(t);
137+
const claude = path.join(root, 'CLAUDE.md');
138+
fs.rmSync(claude);
139+
if (!trySymlink('AGENTS.md', claude)) {
140+
t.skip('symlink creation not permitted on this platform');
141+
return;
142+
}
143+
const { status, output } = runValidator(root);
144+
assert.equal(status, 1, output);
145+
assert.match(output, /regular file/);
146+
});
147+
148+
test('fails when CLAUDE.md is a DANGLING symlink (would slip past existsSync)', (t) => {
149+
const root = buildFixture(t);
150+
const claude = path.join(root, 'CLAUDE.md');
151+
fs.rmSync(claude);
152+
// Target intentionally missing — fs.existsSync(claude) is false, so a presence test
153+
// based on existsSync would skip this file entirely. The lstat-based check must catch it.
154+
if (!trySymlink('AGENTS.MISSING.md', claude)) {
155+
t.skip('symlink creation not permitted on this platform');
156+
return;
157+
}
158+
assert.equal(fs.existsSync(claude), false, 'dangling symlink should look absent to existsSync');
159+
const { status, output } = runValidator(root);
160+
assert.equal(status, 1, output);
161+
assert.match(output, /regular file/);
162+
});
163+
164+
test('fails when CLAUDE.md content drifts from AGENTS.md', (t) => {
165+
const root = buildFixture(t);
166+
fs.appendFileSync(path.join(root, 'CLAUDE.md'), 'drifted line\n', 'utf8');
167+
const { status, output } = runValidator(root);
168+
assert.equal(status, 1, output);
169+
assert.match(output, /issues/);
170+
});
171+
172+
test('fails when a legacy manifest mirror content drifts', (t) => {
173+
const root = buildFixture(t);
174+
const legacy = path.join(root, '.claude-plugin', 'marketplace.json');
175+
const drifted = JSON.parse(fs.readFileSync(legacy, 'utf8'));
176+
drifted.name = 'drifted-name';
177+
writeJson(legacy, drifted);
178+
const { status, output } = runValidator(root);
179+
assert.equal(status, 1, output);
180+
assert.match(output, /issues/);
181+
});

scripts/validate-legacy-compatibility.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,13 @@ const fs = require('fs');
2525
const path = require('path');
2626
const assert = require('assert/strict');
2727

28-
const ROOT = path.resolve(__dirname, '..');
28+
// Normally the repo root. Tests set LEGACY_COMPAT_ROOT to point the exact same checks
29+
// at a throwaway fixture tree — without this seam the script can only ever validate the
30+
// real repo (always clean), so the guard logic itself would be untestable and could rot
31+
// silently. Unset in CI/production; it only ever widens where the script looks.
32+
const ROOT = process.env.LEGACY_COMPAT_ROOT
33+
? path.resolve(process.env.LEGACY_COMPAT_ROOT)
34+
: path.resolve(__dirname, '..');
2935
const OPEN_MARKETPLACE_PATH = path.join(ROOT, 'marketplace.json');
3036
const LEGACY_MARKETPLACE_PATH = path.join(ROOT, '.claude-plugin', 'marketplace.json');
3137

0 commit comments

Comments
 (0)