Merge pull request #7 from MCDxAI/fix/unobfuscated-mapping-service

fix: add unobfuscated version awareness to MappingService

Author: GhostTypes

__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);
+});

__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);
   });
 });

__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)) {

package-lock.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",

package.json

@@ -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<string, Promise<string>>();
 
   /**
-   * 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<string> {
     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<string>;
+    // 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<string> {
+    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<void> {
+    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
    */