Skip to content

Commit b1eccd5

Browse files
committed
feat(agent): add legacy artifact migration on install --force
- Detect legacy own-file skill artifacts and AGENTS.md managed sections during install - Back up legacy files and directories as *.bak before migrating or removing them - Convert legacy skill folders (e.g., claude) to symlink landings pointing to canonical skills - Remove managed section from AGENTS.md and back it up during migration - Implement dry-run mode that plans migration without file changes - Refuse plain installs when legacy artifacts block new symlink target paths - Surface a status advisory listing legacy targets and how to migrate them - Introduce findManagedSectionBounds utility to locate legacy sentinel blocks - Update install action aggregation to consider migrated steps as updated - Add comprehensive tests covering migration scenarios, error cases, and idempotency - Extend filesystem mocks with readdir support for migration operations - Update CLI help text for --force to mention legacy artifact migration and backups
1 parent 68f01d5 commit b1eccd5

7 files changed

Lines changed: 927 additions & 6 deletions

File tree

src/commands/agent.test.ts

Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,18 @@ import path from 'node:path';
22
import { describe, expect, it } from 'vitest';
33
import {
44
DEFAULT_SKILLS,
5+
LEGACY_MANAGED_SECTION_BEGIN,
6+
LEGACY_MANAGED_SECTION_END,
7+
LEGACY_OWN_FILE_TARGETS,
58
SKILLS,
69
TARGETS,
710
type AgentTarget,
11+
type LegacyOwnFileSpec,
812
buildSkillMarker,
13+
canonicalSkillDir,
914
canonicalSkillFile,
15+
findManagedSectionBounds,
16+
legacyOwnFilePath,
1017
loadSkillFull,
1118
pathFor,
1219
renderCanonicalWithMarker,
@@ -95,6 +102,18 @@ function makeMemFs() {
95102
if (d === p || d.startsWith(p + path.sep)) dirs.delete(d);
96103
}
97104
},
105+
async readdir(p) {
106+
const names = new Set<string>();
107+
const prefix = p + path.sep;
108+
const direct = (k: string) => {
109+
if (!k.startsWith(prefix)) return;
110+
const rest = k.slice(prefix.length);
111+
if (!rest.includes(path.sep)) names.add(rest);
112+
};
113+
for (const k of files.keys()) direct(k);
114+
for (const d of dirs) direct(d);
115+
return [...names];
116+
},
98117
};
99118

100119
const seedFile = (p: string, content: string) => {
@@ -887,3 +906,269 @@ describe('shipped skills', () => {
887906
expect(Object.keys(SKILLS).sort()).toEqual(['testsprite-onboard', 'testsprite-verify']);
888907
});
889908
});
909+
910+
// ---------------------------------------------------------------------------
911+
// runInstall --force — legacy migration (issue #270 gate #2)
912+
// ---------------------------------------------------------------------------
913+
914+
const legacyMarker = (skill: string) => buildSkillMarker(skill, loadSkillFull(skill));
915+
const claudeSpec = LEGACY_OWN_FILE_TARGETS.find(s => s.legacyTarget === 'claude')!;
916+
const cursorSpec = LEGACY_OWN_FILE_TARGETS.find(s => s.legacyTarget === 'cursor')!;
917+
918+
function seedLegacyOwnFile(
919+
fs: ReturnType<typeof makeMemFs>,
920+
spec: LegacyOwnFileSpec,
921+
skill: string,
922+
): string {
923+
// Mimic the old wrap: frontmatter + provenance marker + body. Only the marker
924+
// matters for detection; the body/shape is irrelevant to migration.
925+
const content = `---\nname: ${skill}\ndescription: legacy\n---\n${legacyMarker(skill)}\n# ${skill}\nlegacy body\n`;
926+
const rel = legacyOwnFilePath(spec, skill);
927+
fs.seedFile(path.resolve(ROOT, rel), content);
928+
return rel;
929+
}
930+
931+
function seedLegacyCodexSection(
932+
fs: ReturnType<typeof makeMemFs>,
933+
opts: { before?: string; body?: string; after?: string } = {},
934+
): string {
935+
const before = opts.before ?? '# My project\n\nWelcome.\n';
936+
const after = opts.after ?? '\n## Notes\n\nHand-written notes.\n';
937+
const body =
938+
opts.body ?? `${legacyMarker('testsprite-verify')}\nRun testsprite tests after changes.`;
939+
const content = `${before}${LEGACY_MANAGED_SECTION_BEGIN}\n${body}\n${LEGACY_MANAGED_SECTION_END}\n${after}`;
940+
fs.seedFile(path.resolve(ROOT, 'AGENTS.md'), content);
941+
return content;
942+
}
943+
944+
/** Run `agent install --force` (default target: codex) capturing stdout/stderr. */
945+
async function runInstallForceCapture(
946+
fs: ReturnType<typeof makeMemFs>,
947+
opts: { dryRun?: boolean; target?: string[]; output?: 'json' | 'text' } = {},
948+
) {
949+
const stdout: string[] = [];
950+
const stderr: string[] = [];
951+
await runInstall(
952+
{
953+
...COMMON,
954+
output: opts.output ?? 'json',
955+
target: opts.target ?? ['codex'],
956+
force: true,
957+
dryRun: opts.dryRun ?? false,
958+
},
959+
{ ...deps(fs), stdout: (l: string) => stdout.push(l), stderr: (l: string) => stderr.push(l) },
960+
);
961+
return { stdout: stdout.join(''), stderr: stderr.join('\n') };
962+
}
963+
964+
describe('runInstall --force — migrates legacy own-file artifacts (scoped to --target)', () => {
965+
it('claude: backs up the legacy folder to a sibling .bak/ and REPLACES it with a symlink', async () => {
966+
const fs = makeMemFs();
967+
const rel = seedLegacyOwnFile(fs, claudeSpec, 'testsprite-verify');
968+
969+
const { stderr } = await runInstallForceCapture(fs, { target: ['claude-code'] });
970+
971+
// The legacy folder is gone; a sibling <folder>.bak/ preserves the old SKILL.md.
972+
expect(fs.files.has(path.resolve(ROOT, rel))).toBe(false);
973+
expect(
974+
fs.files.get(path.resolve(ROOT, '.claude/skills/testsprite-verify.bak/SKILL.md'))?.content,
975+
).toContain('legacy body');
976+
// The agent's read path is now a SYMLINK → canonical (created by the install phase).
977+
const link = fs.files.get(landingPath('claude-code', 'testsprite-verify'))!;
978+
expect(link?.kind).toBe('symlink');
979+
expect(link?.target).toBe(
980+
path.relative(
981+
path.dirname(landingPath('claude-code', 'testsprite-verify')),
982+
path.resolve(ROOT, canonicalSkillDir('testsprite-verify')),
983+
),
984+
);
985+
// The SKILL.md reachable through the link is canonical, not the old body.
986+
expect(fs.files.get(canonicalPath('testsprite-verify'))?.content).not.toContain('legacy body');
987+
expect(stderr).toMatch(
988+
/converted claude skill at .claude\/skills\/testsprite-verify\/SKILL.md/,
989+
);
990+
expect(stderr).toMatch(/find . -name "\*\.bak" -prune -exec rm -rf/);
991+
});
992+
993+
it('cursor (universal): backs up the obsolete file and removes it; no symlink (reads canonical)', async () => {
994+
const fs = makeMemFs();
995+
const rel = seedLegacyOwnFile(fs, cursorSpec, 'testsprite-verify');
996+
await runInstallForceCapture(fs, { target: ['cursor'] });
997+
expect(fs.files.has(path.resolve(ROOT, rel))).toBe(false);
998+
expect(fs.files.get(path.resolve(ROOT, `${rel}.bak`))?.content).toContain('legacy body');
999+
// cursor is universal — canonical is the destination, no symlink landing.
1000+
expect(fs.files.has(canonicalPath('testsprite-verify'))).toBe(true);
1001+
});
1002+
1003+
it('does not touch a user-authored file at a legacy path (no provenance marker)', async () => {
1004+
const fs = makeMemFs();
1005+
const rel = '.cursor/rules/testsprite-verify.mdc';
1006+
fs.seedFile(path.resolve(ROOT, rel), '---\ndescription: mine\n---\n# my own rule\n');
1007+
await runInstallForceCapture(fs, { target: ['cursor'] });
1008+
expect(fs.files.has(path.resolve(ROOT, rel))).toBe(true);
1009+
});
1010+
1011+
it('SCOPED: --target codex does NOT migrate a legacy claude folder', async () => {
1012+
const fs = makeMemFs();
1013+
seedLegacyOwnFile(fs, claudeSpec, 'testsprite-verify');
1014+
const { stderr } = await runInstallForceCapture(fs, { target: ['codex'] });
1015+
// claude's legacy folder is untouched (codex wasn't asked to migrate it).
1016+
expect(fs.files.has(path.resolve(ROOT, '.claude/skills/testsprite-verify/SKILL.md'))).toBe(
1017+
true,
1018+
);
1019+
expect(fs.dirs.has(path.resolve(ROOT, '.claude/skills/testsprite-verify.bak'))).toBe(false);
1020+
expect(stderr).not.toMatch(/migrated claude/);
1021+
});
1022+
1023+
it('does not migrate a new-format symlink landing (claude-code)', async () => {
1024+
const fs = makeMemFs();
1025+
await runInstallJson(fs, { target: ['claude-code'] });
1026+
await runInstallForceCapture(fs, { target: ['claude-code'] });
1027+
// The symlink is intact; no .bak folder was created next to it.
1028+
expect(fs.files.get(landingPath('claude-code', 'testsprite-verify'))?.kind).toBe('symlink');
1029+
expect(fs.dirs.has(path.resolve(ROOT, '.claude/skills/testsprite-verify.bak'))).toBe(false);
1030+
});
1031+
1032+
it('refuses (exit 5) a legacy folder with unknown nested content', async () => {
1033+
const fs = makeMemFs();
1034+
seedLegacyOwnFile(fs, claudeSpec, 'testsprite-verify');
1035+
// Plant a nested subdirectory the old install never wrote → refuse rather than destroy.
1036+
fs.seedFile(path.resolve(ROOT, '.claude/skills/testsprite-verify/notes/x.md'), 'user data');
1037+
await expect(runInstallForceCapture(fs, { target: ['claude-code'] })).rejects.toMatchObject({
1038+
exitCode: 5,
1039+
});
1040+
expect(
1041+
fs.files.get(path.resolve(ROOT, '.claude/skills/testsprite-verify/notes/x.md'))?.content,
1042+
).toBe('user data');
1043+
});
1044+
});
1045+
1046+
describe('runInstall --force — codex AGENTS.md managed section', () => {
1047+
it('backs up AGENTS.md, removes only the section, preserves surrounding content', async () => {
1048+
const fs = makeMemFs();
1049+
const original = seedLegacyCodexSection(fs, {
1050+
before: '# Project Alpha\n\nIntro paragraph.\n',
1051+
after: '\n## Footer\n\nKeep me.\n',
1052+
});
1053+
1054+
await runInstallForceCapture(fs);
1055+
1056+
// Backup holds the entire original file.
1057+
expect(fs.files.get(path.resolve(ROOT, 'AGENTS.md.bak'))?.content).toBe(original);
1058+
// The sentinel block is gone; user content survives.
1059+
const next = fs.files.get(path.resolve(ROOT, 'AGENTS.md'))?.content ?? '';
1060+
expect(next).not.toContain(LEGACY_MANAGED_SECTION_BEGIN);
1061+
expect(next).not.toContain(LEGACY_MANAGED_SECTION_END);
1062+
expect(next).toContain('# Project Alpha');
1063+
expect(next).toContain('Intro paragraph.');
1064+
expect(next).toContain('## Footer');
1065+
expect(next).toContain('Keep me.');
1066+
});
1067+
1068+
it('refuses (exit 5) a corrupt sentinel block without writing anything', async () => {
1069+
const fs = makeMemFs();
1070+
const malformed = `# head\n\n${LEGACY_MANAGED_SECTION_BEGIN}\nbody with no end sentinel\n`;
1071+
fs.seedFile(path.resolve(ROOT, 'AGENTS.md'), malformed);
1072+
await expect(runInstallForceCapture(fs)).rejects.toMatchObject({ exitCode: 5 });
1073+
expect(fs.files.get(path.resolve(ROOT, 'AGENTS.md'))?.content).toBe(malformed);
1074+
expect(fs.files.has(path.resolve(ROOT, 'AGENTS.md.bak'))).toBe(false);
1075+
});
1076+
});
1077+
1078+
describe('runInstall --force — idempotency, dry-run, and the cleanup tip', () => {
1079+
it('is idempotent: a second --force install reports no migration', async () => {
1080+
const fs = makeMemFs();
1081+
seedLegacyCodexSection(fs);
1082+
await runInstallForceCapture(fs);
1083+
const second = await runInstallForceCapture(fs);
1084+
expect(second.stderr).not.toMatch(/migrated /);
1085+
});
1086+
1087+
it('--dry-run --force plans the migration but writes/removes nothing', async () => {
1088+
const fs = makeMemFs();
1089+
const rel = seedLegacyOwnFile(fs, cursorSpec, 'testsprite-verify');
1090+
const { stderr } = await runInstallForceCapture(fs, { target: ['cursor'], dryRun: true });
1091+
expect(stderr).toMatch(/migrated cursor skill/);
1092+
expect(fs.files.has(path.resolve(ROOT, rel))).toBe(true);
1093+
expect(fs.files.has(path.resolve(ROOT, `${rel}.bak`))).toBe(false);
1094+
expect(fs.files.has(canonicalPath('testsprite-verify'))).toBe(false);
1095+
});
1096+
1097+
it('a clean repo: --force install does no migration and prints no tip', async () => {
1098+
const fs = makeMemFs();
1099+
const { stderr } = await runInstallForceCapture(fs);
1100+
expect(stderr).not.toMatch(/migrated /);
1101+
expect(stderr).not.toMatch(/prune -exec rm -rf/);
1102+
});
1103+
});
1104+
1105+
describe('runInstall (plain, no --force) — refuses a legacy dir collision', () => {
1106+
it('exit 6 with a migration hint when a legacy claude folder blocks the symlink', async () => {
1107+
const fs = makeMemFs();
1108+
seedLegacyOwnFile(fs, claudeSpec, 'testsprite-verify');
1109+
await expect(
1110+
runInstall({ ...COMMON, target: ['claude-code'], force: false }, deps(fs)),
1111+
).rejects.toMatchObject({ exitCode: 6 });
1112+
// Plain install never migrates: the legacy folder is untouched.
1113+
expect(fs.files.has(path.resolve(ROOT, '.claude/skills/testsprite-verify/SKILL.md'))).toBe(
1114+
true,
1115+
);
1116+
expect(fs.dirs.has(path.resolve(ROOT, '.claude/skills/testsprite-verify.bak'))).toBe(false);
1117+
});
1118+
});
1119+
1120+
describe('runStatus — legacy-artifact nudge', () => {
1121+
it('names the scoped targets and the exact --force --target command to migrate', async () => {
1122+
const fs = makeMemFs();
1123+
seedLegacyOwnFile(fs, claudeSpec, 'testsprite-verify');
1124+
seedLegacyCodexSection(fs);
1125+
const stderr: string[] = [];
1126+
await expect(
1127+
runStatus(
1128+
{ ...COMMON, output: 'json' },
1129+
{ ...deps(fs), stderr: (l: string) => stderr.push(l) },
1130+
),
1131+
).rejects.toMatchObject({ exitCode: 1 });
1132+
const out = stderr.join('\n');
1133+
expect(out).toMatch(/legacy installs for:.*claude-code/);
1134+
expect(out).toMatch(/codex/);
1135+
expect(out).toMatch(/testsprite agent install --force --target \S+/);
1136+
});
1137+
1138+
it('does not emit the nudge on a clean project', async () => {
1139+
const fs = makeMemFs();
1140+
const stderr: string[] = [];
1141+
await runStatus(
1142+
{ ...COMMON, output: 'json' },
1143+
{ ...deps(fs), stderr: (l: string) => stderr.push(l) },
1144+
);
1145+
expect(stderr.join('\n')).not.toMatch(/legacy installs for:/);
1146+
});
1147+
});
1148+
1149+
// Pure bounds-finder (data-only; no filesystem).
1150+
describe('findManagedSectionBounds', () => {
1151+
it('absent when no sentinels', () => {
1152+
expect(findManagedSectionBounds('no sentinels here\n')).toEqual({ state: 'absent' });
1153+
});
1154+
it('present with a balanced pair', () => {
1155+
const c = `a\n${LEGACY_MANAGED_SECTION_BEGIN}\nx\n${LEGACY_MANAGED_SECTION_END}\nb\n`;
1156+
const b = findManagedSectionBounds(c);
1157+
expect(b.state).toBe('present');
1158+
if (b.state === 'present') {
1159+
expect(c.slice(b.start, b.end)).toBe(
1160+
`${LEGACY_MANAGED_SECTION_BEGIN}\nx\n${LEGACY_MANAGED_SECTION_END}\n`,
1161+
);
1162+
// Removing the range leaves the surrounding content intact.
1163+
expect(c.slice(0, b.start) + c.slice(b.end)).toBe('a\nb\n');
1164+
}
1165+
});
1166+
it('corrupt when BEGIN has no END', () => {
1167+
const b = findManagedSectionBounds(`a\n${LEGACY_MANAGED_SECTION_BEGIN}\nx\n`);
1168+
expect(b.state).toBe('corrupt');
1169+
});
1170+
it('corrupt when END appears before BEGIN', () => {
1171+
const c = `${LEGACY_MANAGED_SECTION_END}\nx\n${LEGACY_MANAGED_SECTION_BEGIN}\n`;
1172+
expect(findManagedSectionBounds(c).state).toBe('corrupt');
1173+
});
1174+
});

0 commit comments

Comments
 (0)