Ensure cross-platform support via client integration tests run on ubuntu-latest and windows-latest#22
Merged
data-douser merged 12 commits intomainfrom Feb 7, 2026
Conversation
- Use pathToFileURL() instead of file:// string concatenation in:
- server/src/ql-mcp-server.ts (entrypoint check)
- server/src/tools/codeql/language-server-eval.ts (workspace URI)
- client/src/ql-mcp-client.js (entrypoint check)
- Fix path.includes() to normalize separators in cli-tool-registry.ts
- Replace .split('/').pop() with path.basename() for cross-platform path handling
- Update client-integration-tests.yml to run on matrix of ubuntu-latest and windows-latest
- Add platform-specific CI steps for process cleanup and OS dependencies
- Update setup-codeql-environment action with Windows cache paths
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
…ead of split/join Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
On Windows, gh codeql install-stub creates a bash script (codeql) which
is not discoverable by Node.js child_process.spawn(). This causes
'spawn codeql ENOENT' errors in integration tests.
Add a step that creates a codeql.cmd wrapper delegating to 'gh codeql'
so that spawn('codeql', ...) resolves correctly on Windows.
Aligns with github/gh-codeql#21 which adds native Windows support to
install-stub. This workaround can be removed once that PR is merged.
46 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens cross-platform behavior (Linux/Windows) by removing POSIX-specific path handling and OS temp directory usage, and by expanding CI coverage to run client integration tests on both ubuntu-latest and windows-latest.
Changes:
- Replace fragile path/URI string manipulation with cross-platform Node APIs (
pathToFileURL,path.basename, separator normalization). - Move integration test temporary-file usage to a repo-local
.tmp/directory via{{tmpdir}}placeholders resolved at runtime. - Update GitHub Actions workflows/actions to run client integration tests on a Windows + Ubuntu matrix and improve CodeQL CLI availability on Windows.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/tools/codeql/language-server-eval.ts | Uses pathToFileURL(...).href to construct a valid workspace URI on Windows. |
| server/src/ql-mcp-server.ts | Updates “run directly” check to use pathToFileURL(...). |
| server/src/prompts/workflow-prompts.ts | Replaces POSIX-only split('/') filename extraction with path.basename. |
| server/src/lib/cli-tool-registry.ts | Normalizes separators for bundle/source detection; uses path.basename for query name extraction. |
| server/dist/ql-mcp-server.js | Regenerated bundle reflecting the cross-platform path changes. |
| client/src/ql-mcp-client.js | Improves Windows CLI handling and updates “run directly” check to use pathToFileURL(...). |
| client/src/lib/integration-test-runner.js | Moves temp execution dirs under <repoRoot>/.tmp and introduces {{tmpdir}} placeholder resolution. |
| client/integration-tests/** | Updates fixtures to use {{tmpdir}} instead of /tmp. |
| client/integration-tests/README.md | Documents {{tmpdir}} usage and rationale. |
| .github/workflows/client-integration-tests.yml | Runs integration tests on Windows + Ubuntu; adds CodeQL spawnability check and bash shell usage. |
| .github/actions/setup-codeql-environment/action.yml | Adds Windows-specific caching and PATH workaround for codeql.exe. |
| .github/instructions/server_test_ts.instructions.md | Adds explicit guidance to avoid OS temp dirs in tests/fixtures. |
| .github/agents/ql-mcp-tool-tester.md | Updates agent constraints to avoid OS temp dirs and prefer {{tmpdir}}. |
...sts/primitives/tools/codeql_query_run/evaluator_logging_with_tuple_counting/test-config.json
Outdated
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #19
Summary of Changes
This pull request introduces significant improvements to how temporary files are handled in integration tests, ensuring that all test-related temporary files are stored in a project-local
.tmp/directory instead of OS-level temp directories. This change addresses security concerns (CWE-377/CWE-378) around world-readable temp files and standardizes temp file handling across platforms. Additionally, the PR enhances Windows support in CI workflows, updates documentation and fixtures to use the new temp directory convention, and improves the reliability and consistency of integration tests across both Linux and Windows environments.Outline of Changes
Key changes:
Security and Temp Directory Handling
{{tmpdir}}placeholder that resolves to the project-local.tmp/directory, replacing any direct use of/tmpor OS temp directories. This affects test JSON files, test runner logic, and developer instructions. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]client/src/lib/integration-test-runner.js) is refactored to create and use temp directories under.tmp/, and a newresolvePathPlaceholdersutility is added to substitute{{tmpdir}}in test parameters. [1] [2] [3] [4]GitHub Actions and CI Improvements
client-integration-testsworkflow is updated to run on both Ubuntu and Windows runners, with a matrix strategy and OS-specific dependency installation (usingchocofor Windows andapt-getfor Ubuntu). [1] [2]Documentation and Developer Guidance
{{tmpdir}}placeholder and avoid OS temp directories in tests and fixtures, explaining the security rationale. [1] [2] [3]Miscellaneous
These changes collectively ensure secure, cross-platform temporary file handling for integration tests and robust CI support for both Linux and Windows environments.