Skip to content

Use dynamic package version and respect CODEQL_MCP_TMP_DIR env var#27

Merged
data-douser merged 2 commits intomainfrom
fix/pr24-review-feedback
Feb 8, 2026
Merged

Use dynamic package version and respect CODEQL_MCP_TMP_DIR env var#27
data-douser merged 2 commits intomainfrom
fix/pr24-review-feedback

Conversation

@data-douser
Copy link
Collaborator

  • 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

- 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
@data-douser data-douser self-assigned this Feb 8, 2026
@data-douser data-douser requested a review from enyil as a code owner February 8, 2026 21:30
Copilot AI review requested due to automatic review settings February 8, 2026 21:30
@data-douser data-douser requested a review from a team as a code owner February 8, 2026 21:30
@data-douser data-douser added bug Something isn't working enhancement New feature or request javascript Pull requests that update javascript code labels Feb 8, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves runtime portability of the server across monorepo vs npm-installed layouts by making versioning/path/temp-dir handling dynamic and configurable via environment variables.

Changes:

  • Add getPackageVersion() and getUserWorkspaceDir() utilities and use them for LSP clientInfo.version and user-relative path resolution.
  • Respect CODEQL_MCP_TMP_DIR for project temp data, and move session tracking storage under the project tmp base.
  • Adjust/skip brittle tests (rename misleading test; skip sh-dependent PATH-prepend test on Windows) and update the bundled server/dist output.

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
server/test/src/lib/cli-executor.test.ts Fixes misleading test name; skips sh/shebang-dependent test on Windows.
server/src/utils/temp-dir.ts Allows overriding project tmp base via CODEQL_MCP_TMP_DIR.
server/src/utils/package-paths.ts Adds dynamic package version lookup + “user workspace” resolution helper.
server/src/lib/session-data-manager.ts Moves default monitoring storage to project tmp base.
server/src/lib/language-server.ts Uses dynamic package version for LSP clientInfo.version.
server/src/lib/cli-tool-registry.ts Resolves user-supplied relative paths against getUserWorkspaceDir().
server/dist/codeql-development-mcp-server.js Updates bundled output to reflect source changes.
Comments suppressed due to low confidence (3)

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

  • Pack command cwd resolution now uses getUserWorkspaceDir() for relative dir/packDir, but existing tests only pass absolute paths. Please add coverage with a relative dir/packDir and assert the computed cwd matches the effective user workspace base.
          if ((name === 'codeql_pack_install' || name === 'codeql_pack_ls') && (dir || packDir)) {
            const rawCwd = (dir || packDir) as string;
            // 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);
          }

server/src/utils/temp-dir.ts:23

  • The new CODEQL_MCP_TMP_DIR override path isn’t covered by tests. Please add unit tests to verify that when the env var is set, getProjectTmpBase() (and helpers) use it and still create directories under that base.
const PROJECT_TMP_BASE = process.env.CODEQL_MCP_TMP_DIR || join(getPackageRootDir(), '.tmp');

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

  • Relative options.database values for codeql_query_run are now resolved against getUserWorkspaceDir(), but unit tests only cover absolute database paths. Please add a test that passes a relative database path and asserts the resolved absolute path is based on the effective user workspace (including CODEQL_MCP_WORKSPACE).
          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(getUserWorkspaceDir(), options.database);
              logger.info(`Resolved database path to: ${options.database}`);

@data-douser
Copy link
Collaborator Author

@copilot apply changes based on the comments in this thread

Copy link
Contributor

Copilot AI commented Feb 8, 2026

@data-douser I've opened a new pull request, #28, to work on those changes. Once the pull request is ready, I'll request review from you.

…tions (#28)

* Initial plan

* Fix import ordering, restore dot prefix, normalize temp dir paths, add unit tests

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

* Remove weak test case and document limitation in comment

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
@data-douser data-douser added this pull request to the merge queue Feb 8, 2026
Merged via the queue into main with commit ca58e85 Feb 8, 2026
16 checks passed
@data-douser data-douser deleted the fix/pr24-review-feedback branch February 8, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request javascript Pull requests that update javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants