Skip to content

Fix multi-file packages dropping supporting files on format conversion #272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
khaliqgant merged 1 commit into from
Jun 24, 2026
Merged

Fix multi-file packages dropping supporting files on format conversion #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 55 additions & 3 deletions packages/cli/src/__tests__/install-multifile.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,53 @@ describe('install command - multi-file packages', () => {
);
});

it('should preserve supporting files when converting a multi-file skill to codex (--as codex)', async () => {
// Regression: converting a multi-file package used to collapse extractedFiles
// down to the converted main file, dropping references/, helpers/, etc.
const mockPackage = {
id: 'complex-skill',
name: 'complex-skill',
format: 'claude', subtype: 'skill',
tags: [],
total_downloads: 100,
verified: true,
latest_version: {
version: '1.0.0',
tarball_url: 'https://example.com/package.tar.gz',
},
};

const tarGz = await createTarGz({
'SKILL.md': '---\nname: complex-skill\ndescription: A complex skill\n---\n\n# Main Skill File\n\nBody content here.',
'helpers/utils.md': '# Utility Functions',
'references/guide.md': '# Reference Guide',
}, { format: 'claude', subtype: 'skill', packageName: 'complex-skill' });

mockClient.getPackage.mockResolvedValue(mockPackage);
mockClient.downloadPackage.mockResolvedValue(tarGz);

await handleInstall('complex-skill', { as: 'codex' });

// All three files must land in the codex skill directory
expect(saveFile).toHaveBeenCalledTimes(3);

// Main file is converted (content regenerated) but still written as SKILL.md
expect(saveFile).toHaveBeenCalledWith(
'.agents/skills/complex-skill/SKILL.md',
expect.stringContaining('Body content here.')
);

// Supporting files are copied verbatim, preserving subdirectories
expect(saveFile).toHaveBeenCalledWith(
'.agents/skills/complex-skill/helpers/utils.md',
'# Utility Functions'
);
expect(saveFile).toHaveBeenCalledWith(
'.agents/skills/complex-skill/references/guide.md',
'# Reference Guide'
);
});

it('should reject tampered tarballs with corrupted headers (security)', async () => {
// Security: tampered archives should be rejected with strict mode enabled
const mockPackage = {
Expand Down Expand Up @@ -394,13 +441,18 @@ describe('install command - multi-file packages', () => {
// Should succeed and convert only the main file (SKILL.md)
await handleInstall('complex-skill', { as: 'cursor' });

// Cursor doesn't have native skill support - uses progressive disclosure
// Should save to .openskills/ with AGENTS.md reference
expect(saveFile).toHaveBeenCalledTimes(1);
// Cursor doesn't have native skill support - uses progressive disclosure.
// The main file is converted and kept as SKILL.md; supporting files are
// preserved verbatim alongside it (not dropped).
expect(saveFile).toHaveBeenCalledTimes(2);
expect(saveFile).toHaveBeenCalledWith(
'.openskills/complex-skill/SKILL.md',
expect.stringContaining('Main Skill')
);
expect(saveFile).toHaveBeenCalledWith(
'.openskills/complex-skill/helper.md',
'# Helper'
);
});

it('should use native skill location for OpenCode skill install', async () => {
Expand Down
36 changes: 29 additions & 7 deletions packages/cli/src/commands/install.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,10 @@ export async function handleInstall(
// For single-file packages, use the only file
// For multi-file packages, find the main file based on format/subtype conventions
let sourceContent: string;
// Index of the main entry point within extractedFiles. Only this file is
// format-converted; for multi-file packages the remaining (supporting)
// files are copied verbatim so the full directory tree is preserved.
let mainFileIndex = 0;

if (extractedFiles.length === 1) {
sourceContent = extractedFiles[0].content;
Expand All @@ -731,6 +735,7 @@ export async function handleInstall(
`Expected files like SKILL.md, agent.md, or similar main entry points.`
);
}
mainFileIndex = extractedFiles.indexOf(mainFile);
console.log(` 📄 Using main file: ${mainFile.name}`);
sourceContent = mainFile.content;
}
Expand Down Expand Up @@ -902,11 +907,25 @@ export async function handleInstall(
throw new CLIError('Conversion failed: No content generated');
}

// Replace extracted content with converted content
extractedFiles = [{
name: extractedFiles[0].name,
content: convertedContent
}];
// Replace the main file's content with the converted content.
// For multi-file packages (e.g. skills with references/, agents/, etc.),
// preserve every other file so the full directory tree is installed —
// only the main entry point needs format conversion; supporting files are
// copied verbatim by the multi-file install loop below. Collapsing to a
// single file here would silently drop all supporting files in the target
// format (the cause of codex installs only receiving SKILL.md).
if (extractedFiles.length === 1) {
extractedFiles = [{
name: extractedFiles[0].name,
content: convertedContent,
}];
} else {
extractedFiles = extractedFiles.map((file, index) =>
index === mainFileIndex
? { ...file, content: convertedContent }
: file
);
}

console.log(` ✓ Converted from ${pkg.format} to ${format}`);
}
Expand Down Expand Up @@ -1476,8 +1495,11 @@ export async function handleInstall(

// Multi-file package - create directory for package
// For Claude skills, destDir already includes package name, so use it directly
// For Cursor rules converted from Claude skills, use flat structure
const isCursorConversion = (effectiveFormat === 'cursor' && pkg.format === 'claude' && pkg.subtype === 'skill');
// For Cursor rules converted from Claude skills, use flat structure.
// Skip the Cursor .mdc rename when the skill is installed via progressive
// disclosure (.openskills/) — there it stays a SKILL.md, matching the
// single-file install path, and supporting files are preserved verbatim.
const isCursorConversion = (effectiveFormat === 'cursor' && pkg.format === 'claude' && pkg.subtype === 'skill') && !needsProgressiveDisclosureMulti;

@codeant-ai codeant-ai Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The added !needsProgressiveDisclosureMulti guard makes isCursorConversion false for multi-file skill installs that go through progressive disclosure (.openskills/), which skips the Cursor-specific processing block entirely. As a result, converted Cursor installs no longer get Cursor post-processing (like header/config application) in the multi-file path, creating behavior drift from the single-file path and potentially producing improperly configured installed content. Keep progressive-disclosure filename behavior, but still run the Cursor post-processing on the converted main file. [logic error]

Severity Level: Major ⚠️ 
- ❌ Multi-file Claude→Cursor skills ignore user cursor config overrides.
- ⚠️ Progressive `.openskills` installs behave differently from single-file installs.
Steps of Reproduction ✅ 
1. Configure a Cursor MDC override in user config as defined in
`packages/cli/src/core/user-config.ts:11-17,54-59` (e.g., set `cursor: { alwaysApply:
true, globs: ["src/**/*.ts"] }` in `~/.prpmrc` so `config.cursor` is non-empty when
`handleInstall` runs).

2. Create a multi-file Claude skill tarball using the `createTarGz` helper from
`packages/cli/src/__tests__/install-multifile.test.ts:56-104`, with at least `SKILL.md`
and one supporting file (e.g., `helpers/utils.md`), and a mocked registry package `{
format: 'claude', subtype: 'skill' }` as in the multi-file test at lines 175-187.

3. In a Vitest test similar to
`packages/cli/src/__tests__/install-format-conversion.test.ts:68-118`, mock `getConfig` to
return the config from step 1, mock `getPackage`/`downloadPackage` to return the
multi-file tarball from step 2, and call `handleInstall('complex-skill', { as: 'cursor'
})` so execution enters the conversion block at
`packages/cli/src/commands/install.ts:701-232` and produces `convertedContent` via
`toCursor(canonicalPkg)` (see `packages/converters/src/to-cursor.ts:28-51` which always
emits an MDC header).

4. Because the tarball is multi-file, `handleInstall` enters the multi-file branch at
`packages/cli/src/commands/install.ts:1400-1599`, computes
`needsProgressiveDisclosureMulti` as `true` for `effectiveFormat === 'cursor'` and
`effectiveSubtype === 'skill'` using `FORMAT_NATIVE_SUBTYPES.cursor` from
`packages/types/src/package.ts:12-26`, then evaluates `const isCursorConversion =
(effectiveFormat === 'cursor' && pkg.format === 'claude' && pkg.subtype === 'skill') &&
!needsProgressiveDisclosureMulti;` at line 1502, making `isCursorConversion` `false`.

5. In the multi-file write loop at `packages/cli/src/commands/install.ts:1540-1659`, the
Cursor post-processing block guarded by `if (isCursorConversion)` (around line 1575) is
skipped entirely, so the converted SKILL content is written without running
`addMDCHeader`/`applyCursorConfig(fileContent, config.cursor)` for this multi-file
progressive install, while the single-file path still applies Cursor post-processing
unconditionally for cursor installs via the `if (format === 'cursor' && effectiveFormat
=== 'cursor')` block at lines 1351-1359.

6. Inspect the arguments passed to the mocked `saveFile` (as done in the existing tests)
and observe that `.openskills/complex-skill/SKILL.md` contains the default MDC header
generated by `toCursor` (including `alwaysApply` from
`packages/converters/src/to-cursor.ts:110-129`) but does not reflect user-specified
overrides from `config.cursor` (e.g., custom `globs` or `alwaysApply` values),
demonstrating that multi-file progressive Claude→Cursor installs no longer receive the
Cursor config post-processing that single-file installs do.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** packages/cli/src/commands/install.ts
**Line:** 1502:1502
**Comment:**
	*Logic Error: The added `!needsProgressiveDisclosureMulti` guard makes `isCursorConversion` false for multi-file skill installs that go through progressive disclosure (`.openskills/`), which skips the Cursor-specific processing block entirely. As a result, converted Cursor installs no longer get Cursor post-processing (like header/config application) in the multi-file path, creating behavior drift from the single-file path and potentially producing improperly configured installed content. Keep progressive-disclosure filename behavior, but still run the Cursor post-processing on the converted main file.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

const nativeSubtypeConfig = getSubtypeConfig(effectiveFormat, effectiveSubtype);
const usesNativePackageSubdirectory = !needsProgressiveDisclosureMulti && Boolean(nativeSubtypeConfig?.usesPackageSubdirectory);
const packageDir = (effectiveFormat === 'claude' && effectiveSubtype === 'skill')
Expand Down
Loading