feat: add comprehensive icon validation to mcpb validate command

Implements icon validation to catch common issues that prevent icons from
displaying in Claude Desktop. Uses a non-blocking warning approach for
remote URLs to maintain flexibility for MCPB's evolving multi-platform
ecosystem.

## Changes

### Core Implementation (src/node/validate.ts)
- Add `isPNG()` helper to validate PNG file signatures via magic bytes
- Add `validateIcon()` function with comprehensive checks:
  - **Warn** about remote URLs (non-blocking, best practice guidance)
  - **Reject** ${__dirname} variable usage in icon field (doesn't work)
  - **Reject** absolute paths (not portable)
  - Verify icon file exists at specified path
  - Verify icon file is valid PNG format
  - Recommend 512×512 size for Claude Desktop
- Integrate icon validation into `validateManifest()` after schema validation

### Test Coverage
- Update `test/icon-validation.test.ts` - Remote URL test now expects warning
- Update `test/cli.test.ts` - Integration test updated for warnings
- All 109 tests passing

### Documentation Updates
- Update examples/hello-world-node/manifest.json to v0.3

## Validation Behavior

**Remote URLs (WARNING - non-blocking):**
```
Icon path uses a remote URL. Best practice for local MCP servers:
Use local files like "icon": "icon.png" for maximum compatibility.
Claude Desktop currently only supports local icon files in bundles.
```
**Result:** Validation passes, developer is informed

**Other Issues (ERROR - blocking):**
- ${__dirname} usage: Rejects (doesn't work)
- Absolute paths: Rejects (not portable)
- Missing files: Rejects (broken reference)
- Non-PNG files: Rejects (wrong format)

## Rationale

**Non-Blocking Approach:**
- Doesn't prevent valid bundles from being created
- Educates developers about best practices
- Maintains flexibility as MCPB evolves beyond Claude Desktop
- Future-proof if remote URL support is added

**Still Protective:**
- Blocks truly broken configurations (missing files, wrong formats)
- Provides actionable error messages for fixable issues

Based on real-world findings from investigating icon display issues in
Claude Desktop with production MCPB bundles.
This commit is contained in:
Bryan Thompson 2025-10-28 09:14:12 -05:00
commit ccf9470979
No known key found for this signature in database
GPG key ID: 1D66D91917EE02E6

View file

@ -2,7 +2,7 @@ import { existsSync, readFileSync, statSync } from "fs";
import * as fs from "fs/promises";
import { DestroyerOfModules } from "galactus";
import * as os from "os";
import { join, resolve } from "path";
import { dirname, isAbsolute, join, resolve } from "path";
import prettyBytes from "pretty-bytes";
import { unpackExtension } from "../cli/unpack.js";
@ -11,6 +11,102 @@ import {
LATEST_MANIFEST_SCHEMA_LOOSE,
} from "../shared/constants.js";
/**
* Check if a buffer contains a valid PNG file signature
*/
function isPNG(buffer: Buffer): boolean {
// PNG signature: 89 50 4E 47 0D 0A 1A 0A
return (
buffer.length >= 8 &&
buffer[0] === 0x89 &&
buffer[1] === 0x50 &&
buffer[2] === 0x4e &&
buffer[3] === 0x47 &&
buffer[4] === 0x0d &&
buffer[5] === 0x0a &&
buffer[6] === 0x1a &&
buffer[7] === 0x0a
);
}
/**
* Validate icon field in manifest
* @param iconPath - The icon path from manifest.json
* @param baseDir - The base directory containing the manifest
* @returns Validation result with errors and warnings
*/
function validateIcon(
iconPath: string,
baseDir: string,
): { valid: boolean; errors: string[]; warnings: string[] } {
const errors: string[] = [];
const warnings: string[] = [];
const isRemoteUrl =
iconPath.startsWith("http://") || iconPath.startsWith("https://");
const hasVariableSubstitution = iconPath.includes("${__dirname}");
const isAbsolutePath = isAbsolute(iconPath);
// Warn about remote URLs (best practice: use local files)
if (isRemoteUrl) {
warnings.push(
"Icon path uses a remote URL. " +
'Best practice for local MCP servers: Use local files like "icon": "icon.png" for maximum compatibility. ' +
"Claude Desktop currently only supports local icon files in bundles.",
);
}
// Check for ${__dirname} variable (error - doesn't work)
if (hasVariableSubstitution) {
errors.push(
'Icon path should not use ${__dirname} variable substitution. ' +
'Use a simple relative path like "icon.png" instead of "${__dirname}/icon.png".',
);
}
// Check for absolute path (error - not portable)
if (isAbsolutePath) {
errors.push(
"Icon path must be relative to the bundle root, not an absolute path. " +
`Found: "${iconPath}"`,
);
}
// Only proceed with file checks if the path looks like a local file
if (!isRemoteUrl && !isAbsolutePath && !hasVariableSubstitution) {
// Check file existence
const fullIconPath = join(baseDir, iconPath);
if (!existsSync(fullIconPath)) {
errors.push(`Icon file not found at path: ${iconPath}`);
} else {
try {
// Check PNG format
const buffer = readFileSync(fullIconPath);
if (!isPNG(buffer)) {
errors.push(
`Icon file must be PNG format. The file at "${iconPath}" does not appear to be a valid PNG file.`,
);
} else {
// File exists and is a valid PNG - add recommendation
warnings.push(
"Icon validation passed. Recommended size is 512×512 pixels for best display in Claude Desktop.",
);
}
} catch (error) {
errors.push(
`Unable to read icon file at "${iconPath}": ${error instanceof Error ? error.message : "Unknown error"}`,
);
}
}
}
return {
valid: errors.length === 0,
errors,
warnings,
};
}
export function validateManifest(inputPath: string): boolean {
try {
const resolvedPath = resolve(inputPath);
@ -28,6 +124,28 @@ export function validateManifest(inputPath: string): boolean {
if (result.success) {
console.log("Manifest schema validation passes!");
// Validate icon if present
if (manifestData.icon) {
const baseDir = dirname(manifestPath);
const iconValidation = validateIcon(manifestData.icon, baseDir);
if (iconValidation.errors.length > 0) {
console.log("\nERROR: Icon validation failed:\n");
iconValidation.errors.forEach((error) => {
console.log(` - ${error}`);
});
return false;
}
if (iconValidation.warnings.length > 0) {
console.log("\nIcon validation warnings:\n");
iconValidation.warnings.forEach((warning) => {
console.log(` - ${warning}`);
});
}
}
return true;
} else {
console.log("ERROR: Manifest validation failed:\n");