Skip to content

Commit 9d517aa

Browse files
benymbenym
authored
fix: protect installs and artifact routing (rpamis#164)
* fix: protect installs and artifact routing * fix: tighten superpowers artifact matching * fix: avoid ambiguous superpowers artifact fallback * fix: block unmatched superpowers artifacts
1 parent 3de32b7 commit 9d517aa

7 files changed

Lines changed: 393 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ All notable changes to @rpamis/comet will be documented in this file.
77
### Fixed
88

99
- **Single-language rule install**: `comet init` and `comet update` now install only the Comet phase-guard rule file matching the selected/detected Skill language (e.g. `.claude/rules/comet-phase-guard.md`), instead of always installing both the Chinese and English rule variants side by side regardless of language choice.
10+
- **Symlink install safety**: `comet init` and `comet update` now refuse to replace an existing platform `skills/` directory with a symlink when it contains files outside Comet's managed manifest, preserving local or third-party Skills instead of deleting them during symlink-mode installs ([#159](https://github.com/rpamis/comet/issues/159)).
11+
- **Parallel change artifact writes**: Classic phase guards now route `docs/superpowers/` writes to the matching design/build/verify change instead of letting an unrelated earlier active change block shared Design Doc and planning artifacts ([#160](https://github.com/rpamis/comet/issues/160)).
1012

1113
## What's Changed [0.4.0-beta.1] - 2026-07-06
1214

‎assets/skills/comet/scripts/comet-runtime.mjs‎

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11278,6 +11278,60 @@ function blocksSourceWrites(governing) {
1127811278
}
1127911279
return governing.phase === "build" && governing.classic?.workflow === "full" && !governing.classic.designDoc;
1128011280
}
11281+
function isSuperpowersArtifactPath(relativePath2) {
11282+
return relativePath2.startsWith("docs/superpowers/");
11283+
}
11284+
function allowsSuperpowersArtifacts(governing) {
11285+
return governing.phase === "design" || governing.phase === "build" || governing.phase === "verify";
11286+
}
11287+
function governingChangeName(governing) {
11288+
return governing.changeDir ? path16.basename(governing.changeDir) : null;
11289+
}
11290+
var SUPERPOWERS_ARTIFACT_SUFFIXES = /* @__PURE__ */ new Set([
11291+
"design",
11292+
"plan",
11293+
"verify",
11294+
"verification",
11295+
"verification-report",
11296+
"report"
11297+
]);
11298+
function escapeRegex(value) {
11299+
return value.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&");
11300+
}
11301+
function matchesRecordedSuperpowersArtifact(relativePath2, governing) {
11302+
const artifactPaths = [
11303+
governing.classic?.designDoc,
11304+
governing.classic?.plan,
11305+
governing.classic?.verificationReport
11306+
];
11307+
return artifactPaths.some(
11308+
(artifactPath) => artifactPath && normalized(artifactPath) === relativePath2
11309+
);
11310+
}
11311+
function matchesSuperpowersArtifactName(relativePath2, changeName) {
11312+
const fileName = relativePath2.split("/").at(-1) ?? relativePath2;
11313+
const stem = fileName.replace(/\.[^.]+$/u, "");
11314+
if (stem === changeName) return true;
11315+
const suffixes = [...SUPERPOWERS_ARTIFACT_SUFFIXES].map(escapeRegex).join("|");
11316+
const pattern = new RegExp(`(^|[-_.])${escapeRegex(changeName)}[-_.](${suffixes})$`, "u");
11317+
return pattern.test(stem);
11318+
}
11319+
async function superpowersArtifactGoverningChange(relativePath2, projectRoot) {
11320+
const active = await activeChanges(projectRoot);
11321+
const recorded = active.find(
11322+
(governing) => matchesRecordedSuperpowersArtifact(relativePath2, governing)
11323+
);
11324+
if (recorded) return recorded;
11325+
const eligible = active.filter(allowsSuperpowersArtifacts);
11326+
const named = eligible.filter((governing) => {
11327+
const name = governingChangeName(governing);
11328+
return name !== null && matchesSuperpowersArtifactName(relativePath2, name);
11329+
}).sort(
11330+
(a, b) => (governingChangeName(b)?.length ?? 0) - (governingChangeName(a)?.length ?? 0)
11331+
)[0];
11332+
if (named) return named;
11333+
return null;
11334+
}
1128111335
async function repoSourceGoverningChange(projectRoot) {
1128211336
const active = await activeChanges(projectRoot);
1128311337
return active.find(blocksSourceWrites) ?? active[0] ?? null;
@@ -11298,6 +11352,12 @@ async function governingChange(relativePath2, projectRoot) {
1129811352
return { changeDir, phase: "open", classic: null, archived: false };
1129911353
}
1130011354
}
11355+
if (isSuperpowersArtifactPath(relativePath2)) {
11356+
const superpowers = await superpowersArtifactGoverningChange(relativePath2, projectRoot);
11357+
if (superpowers) return { ...superpowers, superpowersArtifact: "matched" };
11358+
const fallback = await repoSourceGoverningChange(projectRoot);
11359+
return fallback ? { ...fallback, superpowersArtifact: "unmatched" } : null;
11360+
}
1130111361
return repoSourceGoverningChange(projectRoot);
1130211362
}
1130311363
function isRootMarkdown(relativePath2) {
@@ -11383,6 +11443,25 @@ function blockedMissingDesignDoc(relativePath2) {
1138311443
].join("\n")
1138411444
);
1138511445
}
11446+
function blockedUnmatchedSuperpowersArtifact(relativePath2, phase) {
11447+
return result(
11448+
2,
11449+
[
11450+
"",
11451+
"╔══════════════════════════════════════════╗",
11452+
"║ COMET PHASE GUARD — WRITE BLOCKED ║",
11453+
"╚══════════════════════════════════════════╝",
11454+
"",
11455+
` Current phase: ${phase}`,
11456+
` Target file: ${relativePath2}`,
11457+
"",
11458+
" BLOCKED: unmatched Superpowers artifact",
11459+
" This docs/superpowers/ path does not match any active change artifact",
11460+
" NEXT: record the artifact path in .comet.yaml or include the change name in the artifact filename",
11461+
""
11462+
].join("\n")
11463+
);
11464+
}
1138611465
var classicHookGuardCommand = async (args) => {
1138711466
const projectRoot = parseProjectRoot(args);
1138811467
const target = inputTarget();
@@ -11414,8 +11493,13 @@ var classicHookGuardCommand = async (args) => {
1141411493
const phase = governing.phase;
1141511494
const openSpec = openSpecAllowed(relativePath2, phase);
1141611495
if (openSpec) return allowed(openSpec);
11417-
if (relativePath2.startsWith("docs/superpowers/") && (phase === "design" || phase === "build" || phase === "verify")) {
11418-
return allowed(`${relativePath2} (phase: ${phase}, superpowers)`);
11496+
if (isSuperpowersArtifactPath(relativePath2)) {
11497+
if (governing.superpowersArtifact === "matched" && allowsSuperpowersArtifacts(governing)) {
11498+
return allowed(`${relativePath2} (phase: ${phase}, superpowers)`);
11499+
}
11500+
if (governing.superpowersArtifact === "unmatched") {
11501+
return blockedUnmatchedSuperpowersArtifact(relativePath2, phase);
11502+
}
1141911503
}
1142011504
if (phase === "build" && governing.classic?.workflow === "full" && !governing.classic.designDoc) {
1142111505
return blockedMissingDesignDoc(relativePath2);

‎domains/comet-classic/classic-hook-guard.ts‎

Lines changed: 112 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ interface GoverningChange {
9494
phase: ClassicPhase;
9595
classic: ClassicState | null;
9696
archived: boolean;
97+
superpowersArtifact?: 'matched' | 'unmatched';
9798
}
9899

99100
async function loadGoverningChange(changeDir: string): Promise<GoverningChange | null> {
@@ -148,6 +149,81 @@ function blocksSourceWrites(governing: GoverningChange): boolean {
148149
);
149150
}
150151

152+
function isSuperpowersArtifactPath(relativePath: string): boolean {
153+
return relativePath.startsWith('docs/superpowers/');
154+
}
155+
156+
function allowsSuperpowersArtifacts(governing: GoverningChange): boolean {
157+
return (
158+
governing.phase === 'design' || governing.phase === 'build' || governing.phase === 'verify'
159+
);
160+
}
161+
162+
function governingChangeName(governing: GoverningChange): string | null {
163+
return governing.changeDir ? path.basename(governing.changeDir) : null;
164+
}
165+
166+
const SUPERPOWERS_ARTIFACT_SUFFIXES = new Set([
167+
'design',
168+
'plan',
169+
'verify',
170+
'verification',
171+
'verification-report',
172+
'report',
173+
]);
174+
175+
function escapeRegex(value: string): string {
176+
return value.replace(/[.*+?^${}()|[\]\\]/gu, '\\$&');
177+
}
178+
179+
function matchesRecordedSuperpowersArtifact(
180+
relativePath: string,
181+
governing: GoverningChange,
182+
): boolean {
183+
const artifactPaths = [
184+
governing.classic?.designDoc,
185+
governing.classic?.plan,
186+
governing.classic?.verificationReport,
187+
];
188+
return artifactPaths.some(
189+
(artifactPath) => artifactPath && normalized(artifactPath) === relativePath,
190+
);
191+
}
192+
193+
function matchesSuperpowersArtifactName(relativePath: string, changeName: string): boolean {
194+
const fileName = relativePath.split('/').at(-1) ?? relativePath;
195+
const stem = fileName.replace(/\.[^.]+$/u, '');
196+
if (stem === changeName) return true;
197+
198+
const suffixes = [...SUPERPOWERS_ARTIFACT_SUFFIXES].map(escapeRegex).join('|');
199+
const pattern = new RegExp(`(^|[-_.])${escapeRegex(changeName)}[-_.](${suffixes})$`, 'u');
200+
return pattern.test(stem);
201+
}
202+
203+
async function superpowersArtifactGoverningChange(
204+
relativePath: string,
205+
projectRoot: string,
206+
): Promise<GoverningChange | null> {
207+
const active = await activeChanges(projectRoot);
208+
const recorded = active.find((governing) =>
209+
matchesRecordedSuperpowersArtifact(relativePath, governing),
210+
);
211+
if (recorded) return recorded;
212+
213+
const eligible = active.filter(allowsSuperpowersArtifacts);
214+
const named = eligible
215+
.filter((governing) => {
216+
const name = governingChangeName(governing);
217+
return name !== null && matchesSuperpowersArtifactName(relativePath, name);
218+
})
219+
.sort(
220+
(a, b) => (governingChangeName(b)?.length ?? 0) - (governingChangeName(a)?.length ?? 0),
221+
)[0];
222+
if (named) return named;
223+
224+
return null;
225+
}
226+
151227
async function repoSourceGoverningChange(projectRoot: string): Promise<GoverningChange | null> {
152228
const active = await activeChanges(projectRoot);
153229
return active.find(blocksSourceWrites) ?? active[0] ?? null;
@@ -172,6 +248,12 @@ async function governingChange(
172248
return { changeDir, phase: 'open', classic: null, archived: false };
173249
}
174250
}
251+
if (isSuperpowersArtifactPath(relativePath)) {
252+
const superpowers = await superpowersArtifactGoverningChange(relativePath, projectRoot);
253+
if (superpowers) return { ...superpowers, superpowersArtifact: 'matched' };
254+
const fallback = await repoSourceGoverningChange(projectRoot);
255+
return fallback ? { ...fallback, superpowersArtifact: 'unmatched' } : null;
256+
}
175257
return repoSourceGoverningChange(projectRoot);
176258
}
177259

@@ -281,6 +363,29 @@ function blockedMissingDesignDoc(relativePath: string): ClassicCommandResult {
281363
);
282364
}
283365

366+
function blockedUnmatchedSuperpowersArtifact(
367+
relativePath: string,
368+
phase: ClassicPhase,
369+
): ClassicCommandResult {
370+
return result(
371+
2,
372+
[
373+
'',
374+
'╔══════════════════════════════════════════╗',
375+
'║ COMET PHASE GUARD — WRITE BLOCKED ║',
376+
'╚══════════════════════════════════════════╝',
377+
'',
378+
` Current phase: ${phase}`,
379+
` Target file: ${relativePath}`,
380+
'',
381+
' BLOCKED: unmatched Superpowers artifact',
382+
' This docs/superpowers/ path does not match any active change artifact',
383+
' NEXT: record the artifact path in .comet.yaml or include the change name in the artifact filename',
384+
'',
385+
].join('\n'),
386+
);
387+
}
388+
284389
export const classicHookGuardCommand: ClassicCommandHandler = async (args) => {
285390
const projectRoot = parseProjectRoot(args);
286391
const target = inputTarget();
@@ -320,11 +425,13 @@ export const classicHookGuardCommand: ClassicCommandHandler = async (args) => {
320425

321426
const openSpec = openSpecAllowed(relativePath, phase);
322427
if (openSpec) return allowed(openSpec);
323-
if (
324-
relativePath.startsWith('docs/superpowers/') &&
325-
(phase === 'design' || phase === 'build' || phase === 'verify')
326-
) {
327-
return allowed(`${relativePath} (phase: ${phase}, superpowers)`);
428+
if (isSuperpowersArtifactPath(relativePath)) {
429+
if (governing.superpowersArtifact === 'matched' && allowsSuperpowersArtifacts(governing)) {
430+
return allowed(`${relativePath} (phase: ${phase}, superpowers)`);
431+
}
432+
if (governing.superpowersArtifact === 'unmatched') {
433+
return blockedUnmatchedSuperpowersArtifact(relativePath, phase);
434+
}
328435
}
329436
if (phase === 'build' && governing.classic?.workflow === 'full' && !governing.classic.designDoc) {
330437
return blockedMissingDesignDoc(relativePath);

‎domains/skill/platform-install.ts‎

Lines changed: 69 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import path from 'path';
22
import { existsSync } from 'fs';
3-
import { readFile, writeFile, lstat, unlink, symlink, rm } from 'fs/promises';
3+
import { readFile, writeFile, lstat, unlink, symlink, rm, readdir } from 'fs/promises';
44
import { fileURLToPath } from 'url';
55
import { parseDocument } from 'yaml';
66

@@ -59,6 +59,51 @@ function getUserFacingSkillNames(manifest: Manifest): string[] {
5959
return getTopLevelSkillNames(manifest.skills);
6060
}
6161

62+
function getManagedSkillReplacementPaths(manifest: Manifest): Set<string> {
63+
const allowed = new Set<string>();
64+
65+
for (const skillPath of getManagedSkillPaths(manifest)) {
66+
const parts = skillPath.split('/').filter(Boolean);
67+
for (let depth = 1; depth <= parts.length; depth++) {
68+
allowed.add(parts.slice(0, depth).join('/'));
69+
}
70+
}
71+
72+
return allowed;
73+
}
74+
75+
async function collectDirectoryEntryPaths(root: string, current = root): Promise<string[]> {
76+
const entries = await readdir(current, { withFileTypes: true });
77+
const paths: string[] = [];
78+
79+
for (const entry of entries) {
80+
const fullPath = path.join(current, entry.name);
81+
const relativePath = path.relative(root, fullPath).split(path.sep).join('/');
82+
paths.push(relativePath);
83+
84+
if (entry.isDirectory() && !entry.isSymbolicLink()) {
85+
paths.push(...(await collectDirectoryEntryPaths(root, fullPath)));
86+
}
87+
}
88+
89+
return paths;
90+
}
91+
92+
async function assertDirectoryContainsOnlyManagedEntries(
93+
dirPath: string,
94+
managedEntries: Set<string>,
95+
): Promise<void> {
96+
const entries = await collectDirectoryEntryPaths(dirPath);
97+
const unmanagedEntries = entries.filter((entry) => !managedEntries.has(entry));
98+
if (unmanagedEntries.length === 0) return;
99+
100+
const preview = unmanagedEntries.slice(0, 5).join(', ');
101+
const suffix = unmanagedEntries.length > 5 ? `, and ${unmanagedEntries.length - 5} more` : '';
102+
throw new Error(
103+
`Refusing to replace ${dirPath} with a symlink because it contains unmanaged entries: ${preview}${suffix}. Move them aside or use copy install mode.`,
104+
);
105+
}
106+
62107
const OPENCODE_COMMAND_HEADER = `---
63108
description: Run the {skillName} Comet workflow
64109
---
@@ -94,24 +139,33 @@ function getCentralSkillsDir(baseDir: string, _scope: InstallScope): string {
94139
* Create a symlink from linkPath pointing to target.
95140
* On Windows, uses 'junction' type for directory symlinks (no admin required).
96141
*/
97-
async function createSymlink(target: string, linkPath: string): Promise<void> {
142+
async function createSymlink(
143+
target: string,
144+
linkPath: string,
145+
managedEntries: Set<string>,
146+
): Promise<void> {
98147
await ensureDir(path.dirname(linkPath));
99148

100149
// Remove existing link/directory if present
150+
let stat: Awaited<ReturnType<typeof lstat>> | null = null;
101151
try {
102-
const stat = await lstat(linkPath);
103-
if (stat.isSymbolicLink()) {
152+
stat = await lstat(linkPath);
153+
} catch (err) {
154+
if ((err as NodeJS.ErrnoException).code !== 'ENOENT') {
155+
throw err;
156+
}
157+
}
158+
159+
if (stat?.isSymbolicLink()) {
160+
await unlink(linkPath);
161+
} else if (stat?.isDirectory()) {
162+
// For directories, try unlink first (handles Windows junctions)
163+
try {
104164
await unlink(linkPath);
105-
} else if (stat.isDirectory()) {
106-
// For directories, try unlink first (handles Windows junctions)
107-
try {
108-
await unlink(linkPath);
109-
} catch {
110-
await rm(linkPath, { recursive: true, force: true });
111-
}
165+
} catch {
166+
await assertDirectoryContainsOnlyManagedEntries(linkPath, managedEntries);
167+
await rm(linkPath, { recursive: true, force: true });
112168
}
113-
} catch {
114-
// Path doesn't exist, continue
115169
}
116170

117171
// Windows uses 'junction' for directory symlinks (no admin privileges required)
@@ -143,6 +197,7 @@ async function installSkillsAsSymlink(
143197
if (!manifest || !Array.isArray(manifest.skills)) {
144198
throw new Error(`Invalid manifest at ${manifestPath}: "skills" must be an array`);
145199
}
200+
const managedSkillReplacementPaths = getManagedSkillReplacementPaths(manifest);
146201

147202
// Step 1: Copy skills to central store
148203
let copied = 0;
@@ -176,7 +231,7 @@ async function installSkillsAsSymlink(
176231
const centralSkillsDir = path.join(centralDir, 'skills');
177232

178233
try {
179-
await createSymlink(centralSkillsDir, platformSkillsDir);
234+
await createSymlink(centralSkillsDir, platformSkillsDir, managedSkillReplacementPaths);
180235
} catch (err) {
181236
failedCount++;
182237
console.error(

0 commit comments

Comments
 (0)