diff --git a/__tests__/core/mapping-service.test.ts b/__tests__/core/mapping-service.test.ts index 2d573db..d9c96ee 100644 --- a/__tests__/core/mapping-service.test.ts +++ b/__tests__/core/mapping-service.test.ts @@ -1,7 +1,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { describe, expect, it } from 'vitest'; import { getMappingService } from '../../src/services/mapping-service.js'; -import { TEST_MAPPING, TEST_VERSION } from '../test-constants.js'; +import { TEST_MAPPING, TEST_VERSION, UNOBFUSCATED_TEST_VERSION } from '../test-constants.js'; /** * Mapping Service Tests @@ -434,3 +434,63 @@ describe('Mojmap Tiny v2 Structure Verification', () => { expect(firstLine).toContain('named'); }, 180000); }); + +/** + * Unobfuscated version handling (26.1+) + * + * Unobfuscated Minecraft versions ship JARs without obfuscation. + * No intermediary, yarn, or mojmap mapping files exist for these versions. + * MappingService.getMappings() and lookupMapping() must fail with clear, + * actionable error messages instead of cryptic download failures. + * + * Reproduces: https://github.com/MCDxAI/minecraft-dev-mcp/issues/5 + */ +describe('Unobfuscated version handling', () => { + it('should throw actionable error for getMappings(intermediary) on unobfuscated version', async () => { + const mappingService = getMappingService(); + await expect( + mappingService.getMappings(UNOBFUSCATED_TEST_VERSION, 'intermediary'), + ).rejects.toThrow(/unobfuscated.*mojmap/is); + }, 30000); + + it('should throw actionable error for getMappings(yarn) on unobfuscated version', async () => { + const mappingService = getMappingService(); + await expect( + mappingService.getMappings(UNOBFUSCATED_TEST_VERSION, 'yarn'), + ).rejects.toThrow(/unobfuscated.*mojmap/is); + }, 30000); + + it('should throw actionable error for getMappings(mojmap) on unobfuscated version', async () => { + const mappingService = getMappingService(); + await expect( + mappingService.getMappings(UNOBFUSCATED_TEST_VERSION, 'mojmap'), + ).rejects.toThrow(/unobfuscated.*already in Mojang/is); + }, 30000); + + it('should throw actionable error for lookupMapping on unobfuscated version', async () => { + const mappingService = getMappingService(); + // lookupMapping calls getMappings internally, which throws for unobfuscated versions + await expect( + mappingService.lookupMapping( + UNOBFUSCATED_TEST_VERSION, + 'Entity', + 'mojmap', + 'yarn', + ), + ).rejects.toThrow(/unobfuscated/i); + }, 30000); + + it('should allow same-type lookupMapping on unobfuscated version (identity)', async () => { + const mappingService = getMappingService(); + // Same source and target mapping should still return identity (no mapping file needed) + const result = await mappingService.lookupMapping( + UNOBFUSCATED_TEST_VERSION, + 'net/minecraft/world/entity/Entity', + 'mojmap', + 'mojmap', + ); + expect(result.found).toBe(true); + expect(result.source).toBe('net/minecraft/world/entity/Entity'); + expect(result.target).toBe('net/minecraft/world/entity/Entity'); + }, 10000); +}); diff --git a/__tests__/core/version-manager.test.ts b/__tests__/core/version-manager.test.ts index fcd942d..17acc3d 100644 --- a/__tests__/core/version-manager.test.ts +++ b/__tests__/core/version-manager.test.ts @@ -70,5 +70,16 @@ describe('Version Management', () => { const result = await versionManager.isVersionUnobfuscated('26.1-snapshot-9'); expect(result).toBe(true); }, 30000); + + // Regression tests for https://github.com/MCDxAI/minecraft-dev-mcp/issues/5 + it.each([ + '26.1-snapshot-10', + '26.1-snapshot-11', + '26.1-rc-3', + ])('should return true for %s (issue #5)', async (version) => { + const versionManager = getVersionManager(); + const result = await versionManager.isVersionUnobfuscated(version); + expect(result).toBe(true); + }, 30000); }); }); diff --git a/__tests__/tools/core-tools.test.ts b/__tests__/tools/core-tools.test.ts index 6cec7c9..3c077ec 100644 --- a/__tests__/tools/core-tools.test.ts +++ b/__tests__/tools/core-tools.test.ts @@ -560,6 +560,20 @@ describe('Decompile and Remap Tools', () => { expect(text).toMatch(/use ['"]mojmap['"] mapping/i); }, 120000); + it('should return actionable error for find_mapping on unobfuscated version', async () => { + const result = await handleFindMapping({ + symbol: 'Entity', + version: UNOBFUSCATED_TEST_VERSION, + sourceMapping: 'mojmap', + targetMapping: 'yarn', + }); + + expect(result).toBeDefined(); + expect(result.isError).toBe(true); + const text = result.content[0].text; + expect(text).toMatch(/unobfuscated/i); + }, 60000); + it('should handle remap_mod_jar with Fabric mod', async () => { // Skip if fixture doesn't exist if (!existsSync(METEOR_JAR_PATH)) { diff --git a/package-lock.json b/package-lock.json index 767abdf..e13e314 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,11 +1,11 @@ { - "name": "@minecraft-dev/mcp-server", + "name": "@mcdxai/minecraft-dev-mcp", "version": "1.0.0", "lockfileVersion": 3, "requires": true, "packages": { "": { - "name": "@minecraft-dev/mcp-server", + "name": "@mcdxai/minecraft-dev-mcp", "version": "1.0.0", "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 3da2f35..b0c6eb2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@mcdxai/minecraft-dev-mcp", - "version": "1.0.0", + "version": "1.1.0", "description": "MCP server for Minecraft mod development - decompile, remap, and explore Minecraft source code", "type": "module", "main": "./dist/index.js", diff --git a/src/services/mapping-service.ts b/src/services/mapping-service.ts index 5f1e91e..45d917e 100644 --- a/src/services/mapping-service.ts +++ b/src/services/mapping-service.ts @@ -11,6 +11,7 @@ import { MappingNotFoundError } from '../utils/errors.js'; import { ensureDir } from '../utils/file-utils.js'; import { logger } from '../utils/logger.js'; import { getMojmapTinyPath } from '../utils/paths.js'; +import { getVersionManager } from './version-manager.js'; /** * Manages mapping downloads and caching @@ -19,87 +20,88 @@ export class MappingService { private mojangDownloader = getMojangDownloader(); private fabricMaven = getFabricMaven(); private cache = getCacheManager(); + private versionManager = getVersionManager(); // Lock to prevent concurrent downloads of the same mappings private downloadLocks = new Map>(); /** - * Get or download mappings for a version - * Uses locking to prevent concurrent downloads of the same mapping + * Get or download mappings for a version. + * Flow: cache → dedupe lock → unobfuscated guard → recheck lock → download. */ async getMappings(version: string, mappingType: MappingType): Promise { const lockKey = `${version}-${mappingType}`; - // For Mojmap, check for converted Tiny file first (not raw ProGuard) - if (mappingType === 'mojmap') { - const convertedPath = getMojmapTinyPath(version); - if (existsSync(convertedPath)) { - logger.info(`Using cached Mojmap (Tiny format) mappings for ${version}: ${convertedPath}`); - return convertedPath; - } - - // Check if download is already in progress - const existingDownload = this.downloadLocks.get(lockKey); - if (existingDownload) { - logger.info(`Waiting for existing Mojmap download of ${version} to complete`); - return existingDownload; - } - - // Download and convert Mojmap with lock - const downloadPromise = this.downloadAndConvertMojmap(version); - this.downloadLocks.set(lockKey, downloadPromise); - try { - return await downloadPromise; - } finally { - this.downloadLocks.delete(lockKey); - } - } - - // Check cache first for other mapping types - const cachedPath = this.cache.getMappingPath(version, mappingType); + // 1. Return immediately from cache without any network access + const cachedPath = this.getCachedMapping(version, mappingType); if (cachedPath) { logger.info(`Using cached ${mappingType} mappings for ${version}: ${cachedPath}`); return cachedPath; } - // Check if download is already in progress + // 2. Deduplicate concurrent downloads for the same version+type const existingDownload = this.downloadLocks.get(lockKey); if (existingDownload) { logger.info(`Waiting for existing ${mappingType} download of ${version} to complete`); return existingDownload; } - // Download based on type with lock - logger.info(`Downloading ${mappingType} mappings for ${version}`); - let downloadPromise: Promise; + // 3. Unobfuscated versions (26.1+) have no mapping files — check before attempting download + await this.throwIfUnobfuscated(version, mappingType); - switch (mappingType) { - case 'yarn': - downloadPromise = this.downloadAndExtractYarn(version); - break; - case 'intermediary': - downloadPromise = this.downloadAndExtractIntermediary(version); - break; - default: - throw new MappingNotFoundError( - version, - mappingType, - `Unsupported mapping type: ${mappingType}`, - ); + // 4. Recheck lock — another caller may have started a download during the async check above + const postCheckDownload = this.downloadLocks.get(lockKey); + if (postCheckDownload) { + return postCheckDownload; } + // 5. Download with lock + logger.info(`Downloading ${mappingType} mappings for ${version}`); + const downloadPromise = this.startDownload(version, mappingType); this.downloadLocks.set(lockKey, downloadPromise); - let mappingPath: string; try { - mappingPath = await downloadPromise; + return await downloadPromise; } finally { this.downloadLocks.delete(lockKey); } + } - // Cache the mapping - this.cache.cacheMapping(version, mappingType, mappingPath); + /** + * Check for a locally cached mapping file without hitting the network. + */ + private getCachedMapping(version: string, mappingType: MappingType): string | null { + if (mappingType === 'mojmap') { + const convertedPath = getMojmapTinyPath(version); + return existsSync(convertedPath) ? convertedPath : null; + } + return this.cache.getMappingPath(version, mappingType) ?? null; + } - return mappingPath; + /** + * Start the actual download for a mapping type. + * Mojmap handles its own caching internally; yarn/intermediary are cached here. + */ + private async startDownload(version: string, mappingType: MappingType): Promise { + switch (mappingType) { + case 'mojmap': + return this.downloadAndConvertMojmap(version); + case 'yarn': { + const path = await this.downloadAndExtractYarn(version); + this.cache.cacheMapping(version, mappingType, path); + return path; + } + case 'intermediary': { + const path = await this.downloadAndExtractIntermediary(version); + this.cache.cacheMapping(version, mappingType, path); + return path; + } + default: + throw new MappingNotFoundError( + version, + mappingType, + `Unsupported mapping type: ${mappingType}`, + ); + } } /** @@ -192,8 +194,7 @@ export class MappingService { !parsed.header.namespaces.includes('named') ) { throw new Error( - `Invalid mapping-io output: expected namespaces 'intermediary' and 'named', ` + - `got ${parsed.header.namespaces.join(', ')}` + `Invalid mapping-io output: expected namespaces 'intermediary' and 'named', got ${parsed.header.namespaces.join(', ')}`, ); } @@ -227,6 +228,29 @@ export class MappingService { // Intermediary should exist for all Fabric-supported versions } + /** + * Throw a clear error if the version is unobfuscated and no mapping files exist. + * Called just before attempting a download, AFTER cache checks, so that cached + * mappings still work without hitting the network. + */ + private async throwIfUnobfuscated(version: string, mappingType: MappingType): Promise { + const isUnobfuscated = await this.versionManager.isVersionUnobfuscated(version); + if (!isUnobfuscated) return; + + if (mappingType === 'mojmap') { + throw new MappingNotFoundError( + version, + mappingType, + `Mojmap mapping files are not available for unobfuscated version ${version}. The JAR is already in Mojang's human-readable names — decompile it directly with mapping 'mojmap'.`, + ); + } + throw new MappingNotFoundError( + version, + mappingType, + `${mappingType} mappings are not available for unobfuscated version ${version}. Use 'mojmap' mapping instead — the JAR ships without obfuscation.`, + ); + } + /** * Lookup result type */