fix: Address PR #24 review feedback

- Replace hardcoded version in language-server.ts with dynamic
  getPackageVersion() that reads from package.json (cached)
- Add getUserWorkspaceDir() for user-relative path resolution that
  falls back to process.cwd() in npm-installed (non-monorepo) layouts
- Honor CODEQL_MCP_TMP_DIR env var in temp-dir.ts for read-only
  package root scenarios (e.g., npm global installs)
- Move session-data-manager default storage from packageRoot to
  getProjectTmpBase() so it respects CODEQL_MCP_TMP_DIR override
- Fix misleading test name: "should accept valid CODEQL_PATH" was
  actually testing rejection of non-existent paths
- Add skipIf(win32) guard on sh-dependent PATH prepend test

Author: data-douser

server/dist/codeql-development-mcp-server.js

@@ -8,7 +8,7 @@ import { executeCodeQLCommand, executeQLTCommand, CLIExecutionResult } from './c
 import { logger } from '../utils/logger';
 import { evaluateQueryResults, QueryEvaluationResult, extractQueryMetadata } from './query-results-evaluator';
 import { getOrCreateLogDirectory } from './log-directory-manager';
-import { packageRootDir, resolveToolQueryPackPath, workspaceRootDir } from '../utils/package-paths';
+import { getUserWorkspaceDir, packageRootDir, resolveToolQueryPackPath } from '../utils/package-paths';
 import { writeFileSync, rmSync, existsSync, mkdirSync } from 'fs';
 import { basename, dirname, isAbsolute, join, resolve } from 'path';
 import { createProjectTempDir } from '../utils/temp-dir';
@@ -215,19 +215,21 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
           case 'codeql_test_run':
           case 'codeql_resolve_tests':
             // Handle tests parameter as positional arguments for test tools.
-            // Resolve relative paths against workspaceRootDir since the MCP
-            // server's cwd may not be the repo root.
+            // Resolve relative paths against the user's effective workspace
+            // directory. In monorepo layouts this is the repo root; in npm-
+            // installed layouts it falls back to process.cwd().
             if (tests && Array.isArray(tests)) {
+              const userDir = getUserWorkspaceDir();
               positionalArgs = [...positionalArgs, ...(tests as string[]).map(
-                t => isAbsolute(t) ? t : resolve(workspaceRootDir, t)
+                t => isAbsolute(t) ? t : resolve(userDir, t)
               )];
             }
             break;
 
           case 'codeql_query_run': {
             // Resolve database path to absolute path if it's relative
             if (options.database && typeof options.database === 'string' && !isAbsolute(options.database)) {
-              options.database = resolve(workspaceRootDir, options.database);
+              options.database = resolve(getUserWorkspaceDir(), options.database);
               logger.info(`Resolved database path to: ${options.database}`);
             }
 
@@ -389,9 +391,9 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
           let cwd: string | undefined;
           if ((name === 'codeql_pack_install' || name === 'codeql_pack_ls') && (dir || packDir)) {
             const rawCwd = (dir || packDir) as string;
-            // Resolve relative paths against the workspace root, not process.cwd(),
-            // since the MCP server's cwd may differ (especially in VS Code).
-            cwd = isAbsolute(rawCwd) ? rawCwd : resolve(workspaceRootDir, rawCwd);
+            // Resolve relative paths against the user's effective workspace
+            // directory rather than a potentially read-only package root.
+            cwd = isAbsolute(rawCwd) ? rawCwd : resolve(getUserWorkspaceDir(), rawCwd);
           }
 
           // Add --additional-packs for commands that need to access local test packs.

server/dist/codeql-development-mcp-server.js.map

@@ -11,6 +11,7 @@ import { delimiter, join } from 'path';
 import { logger } from '../utils/logger';
 import { getProjectTmpDir } from '../utils/temp-dir';
 import { getResolvedCodeQLDir } from './cli-executor';
+import { getPackageVersion } from '../utils/package-paths';
 
 export interface LSPMessage {
   jsonrpc: '2.0';
@@ -243,7 +244,7 @@ export class CodeQLLanguageServer extends EventEmitter {
       processId: process.pid,
       clientInfo: {
         name: 'codeql-development-mcp-server',
-        version: '2.23.9'
+        version: getPackageVersion()
       },
       capabilities: {
         textDocument: {

server/src/lib/cli-tool-registry.ts

@@ -8,7 +8,7 @@ import { JSONFileSync } from 'lowdb/node';
 import { mkdirSync, writeFileSync } from 'fs';
 import { join } from 'path';
 import { randomUUID } from 'crypto';
-import { getPackageRootDir } from '../utils/package-paths';
+import { getProjectTmpBase } from '../utils/temp-dir';
 import {
   QueryDevelopmentSession,
   QueryState,
@@ -403,6 +403,6 @@ function parseBoolEnv(envVar: string | undefined, defaultValue: boolean): boolea
 
 // Export singleton instance with environment variable support
 export const sessionDataManager = new SessionDataManager({
-  storageLocation: process.env.MONITORING_STORAGE_LOCATION || join(getPackageRootDir(), '.ql-mcp-tracking'),
+  storageLocation: process.env.MONITORING_STORAGE_LOCATION || join(getProjectTmpBase(), 'ql-mcp-tracking'),
   enableMonitoringTools: parseBoolEnv(process.env.ENABLE_MONITORING_TOOLS, false),
 });

server/src/lib/language-server.ts

@@ -86,6 +86,48 @@ export function resolveToolQueryPackPath(language: string, packageRoot?: string)
   return resolve(pkgRoot, 'ql', language, 'tools', 'src');
 }
 
+/**
+ * Read the package version from the nearest package.json.
+ *
+ * Cached at first call so the file is read at most once per process.
+ */
+let _cachedVersion: string | undefined;
+export function getPackageVersion(): string {
+  if (_cachedVersion !== undefined) return _cachedVersion;
+  try {
+    const pkgPath = resolve(getPackageRootDir(), 'package.json');
+    const pkg = JSON.parse(readFileSync(pkgPath, 'utf8'));
+    _cachedVersion = pkg.version ?? '0.0.0';
+  } catch {
+    _cachedVersion = '0.0.0';
+  }
+  return _cachedVersion as string;
+}
+
+/**
+ * Get the effective workspace directory for resolving user-supplied relative
+ * paths (test directories, database paths, pack dirs, etc.).
+ *
+ * In a monorepo checkout the workspace root is the monorepo parent.  In an
+ * npm-installed layout, `workspaceRootDir` falls back to `packageRootDir`
+ * which may be read-only and is not the user's project.  In that case we
+ * fall back to `process.cwd()` so that relative paths resolve against the
+ * directory the user actually invoked the server from.
+ *
+ * Override with `CODEQL_MCP_WORKSPACE` for deterministic behavior.
+ */
+export function getUserWorkspaceDir(): string {
+  if (process.env.CODEQL_MCP_WORKSPACE) {
+    return process.env.CODEQL_MCP_WORKSPACE;
+  }
+  // When workspaceRootDir === packageRootDir we are NOT in a monorepo
+  // (npm-installed), so fall back to process.cwd().
+  if (workspaceRootDir === packageRootDir) {
+    return process.cwd();
+  }
+  return workspaceRootDir;
+}
+
 // Pre-computed values for use throughout the server
 export const packageRootDir = getPackageRootDir();
 export const workspaceRootDir = getWorkspaceRootDir(packageRootDir);

server/src/lib/session-data-manager.ts

@@ -13,9 +13,14 @@ import { getPackageRootDir } from './package-paths';
 
 /**
  * Base directory for all project-local temporary data.
- * Stored under `<packageRoot>/.tmp` and excluded from version control.
+ *
+ * Resolution order:
+ * 1. `CODEQL_MCP_TMP_DIR` environment variable — for read-only package root
+ *    scenarios (e.g., npm global installs where the package directory is not
+ *    writable).
+ * 2. `<packageRoot>/.tmp` — default; excluded from version control.
  */
-const PROJECT_TMP_BASE = join(getPackageRootDir(), '.tmp');
+const PROJECT_TMP_BASE = process.env.CODEQL_MCP_TMP_DIR || join(getPackageRootDir(), '.tmp');
 
 /**
  * Return the project-local `.tmp` base directory, creating it if needed.

server/src/utils/package-paths.ts

@@ -635,11 +635,9 @@ describe('resolveCodeQLBinary', () => {
     expect(() => resolveCodeQLBinary()).toThrow('does not exist');
   });
 
-  it('should accept a valid absolute CODEQL_PATH pointing to an existing file', () => {
-    // Use /bin/echo as a stand-in for an existing file named "codeql"
-    // We can't actually test with a real codeql binary in unit tests,
-    // so test the basename validation and existence check separately.
-    // Here we test that a non-existent but well-named path is rejected for non-existence.
+  it('should reject non-existent CODEQL_PATH even with valid basename', () => {
+    // Verify that a well-named but non-existent path is rejected for non-existence
+    // (the basename 'codeql' passes validation, but the file doesn't exist).
     process.env.CODEQL_PATH = '/tmp/nonexistent-dir/codeql';
     expect(() => resolveCodeQLBinary()).toThrow('does not exist');
   });
@@ -694,8 +692,9 @@ describe('CODEQL_PATH - PATH prepend integration', () => {
     expect(getResolvedCodeQLDir()).toBeNull();
   });
 
-  it('should prepend CODEQL_PATH directory to child process PATH', async () => {
+  it.skipIf(process.platform === 'win32')('should prepend CODEQL_PATH directory to child process PATH', async () => {
     // Create a temporary directory with a fake "codeql" script
+    // Skipped on Windows: uses sh and #!/bin/sh shebang
     const tmpDir = createProjectTempDir('codeql-path-prepend-test-');
     const codeqlPath = join(tmpDir, 'codeql');
     writeFileSync(codeqlPath, '#!/bin/sh\necho test', { mode: 0o755 });