Commit 23ffb65
Improve prompt error handling and relative path support (#153)
* Initial plan
* Add path resolution and error handling for prompt file path parameters
- Add resolvePromptFilePath() utility that resolves relative paths against
workspace root and returns user-friendly warnings for invalid paths
- Update all 9 prompt handlers with file path parameters to resolve paths
and embed warnings in the response instead of throwing protocol errors
- Add integration test in mcp-test-suite.js for explain_codeql_query with
invalid path
- Add file-based integration test fixtures in primitives/prompts/
- Add 10 unit tests for resolvePromptFilePath and handler path resolution
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
* Address code review: add advisory comment on existsSync, improve test assertions
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
* plan: add extension integration test for prompt error handling
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
* Add extension integration test for MCP prompt error handling with invalid paths
Add mcp-prompt-e2e.integration.test.ts to the VS Code extension test suite
that spawns the real MCP server inside the Extension Development Host and
verifies that prompts return user-friendly warnings (not protocol errors)
when given invalid file paths:
- explain_codeql_query with nonexistent relative path returns "does not exist"
- explain_codeql_query with valid absolute path returns no warning
- document_codeql_query with nonexistent path returns "does not exist"
- Server lists prompts including explain_codeql_query
Register the new test in esbuild.config.js so it is bundled for CI.
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
* Initial plan
* fix: return inline errors for invalid prompt inputs
The MCP SDK validates prompt arguments before calling handlers. When
validation fails (e.g. unsupported language like "rust"), the SDK throws
McpError(-32602) with a raw JSON dump — poor UX for VS Code slash commands.
Server-side changes (server/src/prompts/workflow-prompts.ts):
- Add toPermissiveShape() to widen z.enum() to z.string() in registered
schemas so the SDK never rejects user input at the protocol layer
- Add createSafePromptHandler() to wrap all 11 prompt handlers with:
(1) strict Zod validation that returns user-friendly inline errors
(2) exception recovery that catches handler crashes gracefully
- Add formatValidationError() to convert ZodError into actionable markdown
with field names, received values, and valid options listed
Integration tests (extensions/vscode/test/suite/mcp-prompt-e2e.integration.test.ts):
- Expand from 4 to 15 e2e tests covering all prompts with file-path params
- Add test for invalid language returning inline error (not protocol error)
- Add test for path traversal detection
- Add tests for all-optional prompts with empty args
Unit tests (server/test/src/prompts/workflow-prompts.test.ts):
- Add 17 new tests for toPermissiveShape, formatValidationError,
createSafePromptHandler, and handler-level invalid argument handling
- Update schema-to-registration consistency tests for permissive shapes
* fix: promote key prompt parameters from optional to required
Make databasePath, language, queryPath, and sarifPath required where
the prompt depends on them. VS Code now correctly marks these fields
as required in the slash-command input dialog.
* Addres PR review feedback
* Sync server/dist/codeql-development-mcp-server.js.map
* [UPDATE PRIMITIVE] Fix path traversal check, add prompt fixture runner, fix comment mismatch (#155)
* Apply suggestions from code review
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com>
* Enforce tidy and rebuild server dist
* address PR review comments for prompt path handling
- Treat path traversal as hard failure: return blocked=true with empty
resolvedPath instead of leaking the outside-root absolute path
- Handle file:// URIs in resolvePromptFilePath via fileURLToPath so all
callers (queryPath, workspaceUri, etc.) get consistent URI handling
- Replace synchronous existsSync with async fs/promises.access to avoid
blocking the event loop on the prompt hot path
- Fix integration test success condition to fail when no tests executed
- Make VS Code e2e invalid-language test deterministic (assert inline
validation instead of accepting both throw and inline error)
- Fix misleading test name "should return the original path" to match
actual behavior (resolves relative to workspace root)
* [WIP] Improve prompt error handling and relative path support (#157)
* Initial plan
* Fix prompt test runner: propagate failures and check sessions[].expectedContentPatterns
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/2840e119-cc2c-4f60-91ce-ba685ce25b5c
---------
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>
* More fixes for PR review feedback
Improves end-to-end extension integration tests such that the
'CODEQL_MCP_WORKSPACE' env var is passed to StdioClientTransport,
which ensures that the server's relative-path resolution uses a
deterministic diretory instead of depending upon the Extension
Host's working directory.
* [UPDATE PRIMITIVE] Fix markdown injection and platform-dependent path tests in prompt handlers (#162)
* Initial plan
* Fix markdown injection and platform-dependent path tests in prompt handlers
- Add sanitizeForInlineCode() helper to escape backticks and newlines in user-supplied values embedded in markdown code spans
- Apply sanitizer to resolvePromptFilePath 'does not exist' warning (filePath and absolutePath)
- Apply sanitizer to formatValidationError issue.received display
- Fix POSIX path separator assumptions in tests: use basename only ('mydb', 'database')
- Rename createSafePromptHandler tests to clarify they validate the handler wrapper, not MCP SDK validation
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/2660567b-5822-4505-91c2-37fe3ef00f4e
* Replace sanitizeForInlineCode with CommonMark-compliant markdownInlineCode
- markdownInlineCode() uses a fence length = maxRun+1 per CommonMark spec,
preserving the original string (no information loss from backtick→apostrophe)
- Normalises CR/CRLF to LF before wrapping (inline spans can't span lines)
- Export markdownInlineCode for testability
- Add 6 unit tests for markdownInlineCode (plain text, single/double backtick,
CRLF normalisation, backtick-only values)
- Add regression test for formatValidationError with backtick in received value
- Add regression test for resolvePromptFilePath warning with backtick in path
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/ec7c534b-93ac-40e5-bcb6-023bc7496940
* Fix markdownInlineCode to replace newlines with spaces for single-line output
Replace \r\n, \r, and \n with a space (not just normalize CRLF to LF) so
the returned inline code span never contains a literal newline character.
Update docstring and test to reflect space-replacement behavior.
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/87cfd54e-9d66-4871-a581-601aff3c6c8d
---------
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>
* Add 'actions/cache@v5' for VS Code integration test download
Adds a common cache for actions workflows that download the
latest instance of VS Code for integration testing purposes.
* Address latest feedback for PR #153
* Add retries to download-vscode.js script
* fix: resilient VS Code download for integration tests
- Add retry logic (3 attempts) with backoff to download-vscode.js
- Handle unhandled promise rejections from @vscode/test-electron stream errors
- Clean partial downloads between retries to avoid checksum mismatches
- Default to 'stable' version to match .vscode-test.mjs configuration
- Chain download:vscode before vscode-test in test:integration script
- Cache VS Code downloads in build-and-test-extension.yml and copilot-setup-steps.yml
- Increase download timeout from 15s (default) to 120s
Fixes intermittent CI failures from ECONNRESET and empty-file checksum errors
when downloading VS Code for Extension Host integration tests.
* Address PR #153 review comments
- resolvePromptFilePath: only enforce workspace-root boundary for
relative path inputs; absolute paths pass through since users
legitimately reference databases/queries outside the workspace
- download-vscode.js: use fileURLToPath() instead of URL.pathname
for cross-platform compatibility
- integration-test-runner.js: track execution counts separately
from pass/fail; runWorkflowIntegrationTests and
runPromptIntegrationTests return { executed, passed } objects
- Add test for absolute paths outside workspace being allowed
* Apply suggestions from code review
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com>
* fix: address PR #153 review comments for path resolution
- Remove absolute path from 'does not exist' warning to avoid leaking
local machine paths into prompt context sent to LLMs
- Add blockedPathError() helper and check blocked flag at all 12
resolvePromptFilePath call sites so traversal attempts short-circuit
with a clear inline error instead of silently proceeding
- Add tests for both behaviors
---------
Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>1 parent 504152c commit 23ffb65
File tree
17 files changed
+2759
-810
lines changed- .github/workflows
- client
- integration-tests/primitives
- prompts/explain_codeql_query/invalid_query_path
- after
- before
- tools/codeql_bqrs_interpret/sarif_format/after
- src/lib
- extensions/vscode
- scripts
- test/suite
- server
- dist
- src/prompts
- test/src/prompts
17 files changed
+2759
-810
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
66 | 76 | | |
67 | 77 | | |
68 | 78 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
80 | 86 | | |
81 | 87 | | |
82 | 88 | | |
Lines changed: 22 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
Lines changed: 9 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
Lines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
226 | 226 | | |
227 | 227 | | |
228 | 228 | | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
229 | 235 | | |
230 | | - | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
231 | 266 | | |
232 | | - | |
| 267 | + | |
233 | 268 | | |
234 | 269 | | |
235 | 270 | | |
| |||
1095 | 1130 | | |
1096 | 1131 | | |
1097 | 1132 | | |
| 1133 | + | |
| 1134 | + | |
| 1135 | + | |
| 1136 | + | |
| 1137 | + | |
| 1138 | + | |
| 1139 | + | |
| 1140 | + | |
| 1141 | + | |
| 1142 | + | |
| 1143 | + | |
| 1144 | + | |
| 1145 | + | |
| 1146 | + | |
| 1147 | + | |
| 1148 | + | |
| 1149 | + | |
| 1150 | + | |
| 1151 | + | |
| 1152 | + | |
| 1153 | + | |
| 1154 | + | |
| 1155 | + | |
| 1156 | + | |
| 1157 | + | |
| 1158 | + | |
| 1159 | + | |
| 1160 | + | |
| 1161 | + | |
| 1162 | + | |
| 1163 | + | |
| 1164 | + | |
| 1165 | + | |
| 1166 | + | |
| 1167 | + | |
| 1168 | + | |
| 1169 | + | |
| 1170 | + | |
| 1171 | + | |
| 1172 | + | |
| 1173 | + | |
| 1174 | + | |
| 1175 | + | |
| 1176 | + | |
| 1177 | + | |
| 1178 | + | |
| 1179 | + | |
| 1180 | + | |
| 1181 | + | |
| 1182 | + | |
| 1183 | + | |
| 1184 | + | |
| 1185 | + | |
| 1186 | + | |
| 1187 | + | |
| 1188 | + | |
| 1189 | + | |
| 1190 | + | |
| 1191 | + | |
| 1192 | + | |
| 1193 | + | |
| 1194 | + | |
| 1195 | + | |
| 1196 | + | |
| 1197 | + | |
| 1198 | + | |
| 1199 | + | |
| 1200 | + | |
| 1201 | + | |
| 1202 | + | |
| 1203 | + | |
| 1204 | + | |
| 1205 | + | |
| 1206 | + | |
| 1207 | + | |
| 1208 | + | |
| 1209 | + | |
| 1210 | + | |
| 1211 | + | |
| 1212 | + | |
| 1213 | + | |
| 1214 | + | |
| 1215 | + | |
| 1216 | + | |
| 1217 | + | |
| 1218 | + | |
| 1219 | + | |
| 1220 | + | |
| 1221 | + | |
| 1222 | + | |
| 1223 | + | |
| 1224 | + | |
| 1225 | + | |
| 1226 | + | |
| 1227 | + | |
| 1228 | + | |
| 1229 | + | |
| 1230 | + | |
| 1231 | + | |
| 1232 | + | |
| 1233 | + | |
| 1234 | + | |
| 1235 | + | |
| 1236 | + | |
| 1237 | + | |
| 1238 | + | |
| 1239 | + | |
| 1240 | + | |
| 1241 | + | |
| 1242 | + | |
| 1243 | + | |
| 1244 | + | |
| 1245 | + | |
| 1246 | + | |
| 1247 | + | |
| 1248 | + | |
| 1249 | + | |
| 1250 | + | |
| 1251 | + | |
| 1252 | + | |
| 1253 | + | |
| 1254 | + | |
| 1255 | + | |
| 1256 | + | |
| 1257 | + | |
| 1258 | + | |
| 1259 | + | |
| 1260 | + | |
| 1261 | + | |
| 1262 | + | |
| 1263 | + | |
| 1264 | + | |
| 1265 | + | |
| 1266 | + | |
| 1267 | + | |
| 1268 | + | |
| 1269 | + | |
| 1270 | + | |
| 1271 | + | |
| 1272 | + | |
| 1273 | + | |
| 1274 | + | |
| 1275 | + | |
| 1276 | + | |
| 1277 | + | |
| 1278 | + | |
| 1279 | + | |
| 1280 | + | |
| 1281 | + | |
| 1282 | + | |
| 1283 | + | |
| 1284 | + | |
| 1285 | + | |
| 1286 | + | |
| 1287 | + | |
1098 | 1288 | | |
1099 | 1289 | | |
1100 | 1290 | | |
| |||
1107 | 1297 | | |
1108 | 1298 | | |
1109 | 1299 | | |
1110 | | - | |
| 1300 | + | |
1111 | 1301 | | |
1112 | 1302 | | |
1113 | 1303 | | |
| |||
1118 | 1308 | | |
1119 | 1309 | | |
1120 | 1310 | | |
1121 | | - | |
| 1311 | + | |
1122 | 1312 | | |
1123 | 1313 | | |
1124 | 1314 | | |
| |||
1131 | 1321 | | |
1132 | 1322 | | |
1133 | 1323 | | |
1134 | | - | |
| 1324 | + | |
1135 | 1325 | | |
1136 | 1326 | | |
1137 | | - | |
| 1327 | + | |
1138 | 1328 | | |
1139 | 1329 | | |
1140 | 1330 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| |||
132 | 133 | | |
133 | 134 | | |
134 | 135 | | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
135 | 189 | | |
136 | 190 | | |
137 | 191 | | |
| |||
0 commit comments