Skip to content

[UPDATE PRIMITIVE] Accept singular file argument in codeql_bqrs_decode#303

Draft
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-codeql-bqrs-decode-file-argument
Draft

[UPDATE PRIMITIVE] Accept singular file argument in codeql_bqrs_decode#303
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-codeql-bqrs-decode-file-argument

Conversation

Copilot AI commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

📝 Update Information

Primitive Details

  • Type: Tool
  • Name: codeql_bqrs_decode
  • Update Category: Bug Fix

⚠️ CRITICAL: PR SCOPE VALIDATION

This PR is for updating an existing MCP server primitive and must ONLY include these file types:

ALLOWED FILES:

  • Server implementation files (server/src/**/*.ts)
  • Updated primitive implementations
  • Modified registration files (server/src/tools/*.ts)
  • Updated or new test files (server/test/**/*.ts)
  • Documentation updates (README.md, server docs)
  • Updated type definitions (server/src/types/*.ts)
  • Modified supporting library files (server/src/lib/*.ts)
  • Configuration updates if needed (package.json, tsconfig.json)

🚫 FORBIDDEN FILES:

  • Files unrelated to the primitive update
  • Temporary or test output files
  • IDE configuration files
  • Log files or debug output
  • Analysis or summary files

Rationale: This PR should contain only the files necessary to update and test the primitive.

🚨 PRs that include forbidden files will be rejected and must be revised.


🛑 MANDATORY PR VALIDATION CHECKLIST

BEFORE SUBMITTING THIS PR, CONFIRM:

  • ONLY server implementation files are included
  • NO temporary or output files are included
  • NO unrelated configuration files are included
  • ALL existing tests continue to pass
  • NEW functionality is properly tested

  • Impact Scope: Localized

Update Metadata

  • Breaking Changes: No
  • API Compatibility: Enhanced — files (array) still works; file (string) now also accepted
  • Performance Impact: Neutral

🎯 Changes Description

Current Behavior

codeql_bqrs_decode advertised only a required plural files array. Passing the singular file — the common single-BQRS case and the natural first attempt for LLM clients — failed schema validation with must have required property 'files', despite the registry handler already supporting file for all codeql_bqrs_* tools. The advertised schema and the handler were out of sync.

Updated Behavior

The tool accepts either file (a single string path) or files (an array); both are optional and normalized to positional CLI args. When neither is supplied, codeql_bqrs_* tools now return an actionable error naming both parameters instead of a generic "required property" rejection.

Motivation

Eliminate a recurring usability papercut and a common source of failed first attempts by automated/LLM clients; align the MCP-advertised contract with actual handler behavior.

🔄 Before vs. After Comparison

Functionality Changes

// BEFORE: only `files` advertised; `{ file: "..." }` rejected before reaching the handler.

// AFTER: registry validates BQRS tools have a path via either form, with a clear error.
if (name.startsWith('codeql_bqrs_') && positionalArgs.length === 0) {
  throw new Error(
    `The "${name}" tool requires a BQRS file path. Provide it via "file" (a single string path) or "files" (an array of string paths).`,
  );
}

API Changes

// Input Schema Changes (codeql_bqrs_decode)
// BEFORE:
inputSchema: {
  files: z.array(z.string()) // required
}

// AFTER:
inputSchema: {
  file: z.string().optional(),         // singular alias for the common case
  files: z.array(z.string()).optional() // array form; at least one required at runtime
}

Output Format Changes

// Unchanged — decoded BQRS output is identical regardless of which input form is used.

🧪 Testing & Validation

Test Coverage Updates

  • Existing Tests: All existing tests continue to pass
  • New Test Cases: Added tests for new functionality
  • Regression Tests: Added tests to prevent regression of fixed issues
  • Edge Case Tests: Enhanced edge case coverage

Validation Scenarios

  1. Backward Compatibility: { files: ["…"] } still parses and maps to positional args.
  2. New Functionality: { file: "…" } parses through the enhanced schema and maps to positional args.
  3. Error Handling: empty args returns a clear error naming both file and files.
  4. Performance: N/A.

Test Results

  • Unit Tests: All pass (1492 server tests)
  • Integration Tests: All pass (85 client integration tests; new decode_singular_file case included)
  • Manual Testing: Validated decoding a real .bqrs via the singular file argument
  • Performance Testing: N/A

📋 Implementation Details

Files Modified

  • Core Implementation: server/src/tools/codeql/bqrs-decode.ts
  • Supporting Libraries: server/src/lib/cli-tool-registry.ts
  • Type Definitions: N/A
  • Tests: server/test/src/tools/codeql/bqrs-decode.test.ts, server/test/src/lib/cli-tool-registry.test.ts, client/integration-tests/primitives/tools/codeql_bqrs_decode/decode_singular_file/*
  • Documentation: CHANGELOG.md

Code Changes Summary

  • Algorithm Improvements: Enhanced core logic
  • Error Handling: Improved error handling and messaging
  • Performance Optimization: Optimized execution paths
  • Type Safety: Enhanced TypeScript types
  • Input Validation: Improved input validation
  • Output Format: Enhanced output structure

Dependencies

  • No New Dependencies: Update uses existing dependencies only
  • Updated Dependencies: Updated to newer versions (list changes)
  • New Dependencies: Added new dependencies (justify need)

🔍 Quality Improvements

Bug Fixes (if applicable)

  • Issue: codeql_bqrs_decode rejected a singular file argument, requiring files (array).
  • Root Cause: The advertised MCP input schema required files, while the registry handler already accepted singular file for BQRS tools — a schema/handler mismatch not bridged by existing camelCase/snake_case normalization.
  • Solution: Advertise both file and files (optional) on codeql_bqrs_decode; add a runtime guard for codeql_bqrs_* that errors clearly when no path is provided.
  • Prevention: Unit tests assert both forms parse and map to positional args; integration test exercises the singular file end-to-end.

Performance Improvements

  • Baseline Performance: N/A
  • Improved Performance: N/A
  • Optimization Techniques: N/A

Code Quality Enhancements

  • Readability: Improved code clarity and documentation
  • Maintainability: Better code organization and structure
  • Testability: Enhanced test coverage and clarity
  • Reusability: More modular and reusable components

🔗 References

Related Issues/PRs

  • Related PRs: N/A

External References

  • codeql bqrs decode -h -vv (CLI accepts one or more positional BQRS files)

Validation Materials

  • Test Cases: decode_singular_file integration fixture decoding a real #select BQRS via singular file
  • Performance Benchmarks: N/A

🚀 Compatibility & Migration

Backward Compatibility

  • Fully Compatible: No breaking changes
  • Deprecation Warnings: Deprecated features with warnings
  • Breaking Changes: Changes that break existing usage (detailed below)

Breaking Changes (if any)

None.

API Evolution

  • Enhanced Parameters: New optional file parameter added
  • Improved Responses: Richer response formats
  • Better Error Messages: Clear error naming both file and files
  • Maintained Contracts: files contract preserved

👥 Review Guidelines

For Reviewers

Please verify:

  • ⚠️ SCOPE COMPLIANCE: PR contains only server implementation files
  • ⚠️ NO UNRELATED FILES: No temporary, output, or unrelated files
  • ⚠️ BACKWARD COMPATIBILITY: Existing functionality preserved (unless breaking changes approved)
  • Functionality: Updates work as described
  • Test Coverage: All existing tests pass, new tests comprehensive
  • Performance: No performance regressions
  • Code Quality: Maintains or improves code quality
  • Documentation: Updated documentation accurate
  • Error Handling: Improved error handling
  • Type Safety: TypeScript types properly updated

Testing Instructions

# Server unit tests
npm test --workspace=server

# Client integration tests (includes decode_singular_file)
make -C client test-integration

# Targeted manual validation
client/gh-ql-mcp-client integration-tests --mode stdio --tools codeql_bqrs_decode

Validation Checklist

  1. Regression Testing: files array path unchanged.
  2. New Feature Testing: singular file path decodes successfully.
  3. Performance Testing: N/A.
  4. Error Testing: empty args yields the parameter-naming error.
  5. Integration Testing: server, client, and bundled dist exercised.
  6. Documentation Review: CHANGELOG entry added.

📊 Impact Assessment

Performance Impact

  • Memory Usage: No change
  • Execution Time: No change
  • Throughput: No change

Server Impact

  • Startup Time: No significant impact on server startup
  • Runtime Stability: No impact on server stability
  • Resource Usage: Reasonable resource consumption
  • Concurrent Usage: Safe for concurrent access

AI Assistant Impact

  • Enhanced Accuracy: Improved AI assistant responses
  • Better Coverage: Expanded use case support
  • Improved Reliability: More reliable primitive behavior
  • Enhanced User Experience: Better AI assistant workflows

🔄 Deployment Strategy

Rollout Considerations

  • Safe Deployment: Can be deployed safely to production
  • Gradual Rollout: Consider gradual rollout if high-impact changes
  • Monitoring: Appropriate monitoring for the update
  • Rollback Plan: Clear rollback strategy if issues arise

Post-Deployment Validation

  • Monitoring: Key metrics to monitor after deployment
  • User Feedback: Channels for collecting user feedback
  • Performance Tracking: Performance metrics to track
  • Error Tracking: Watch for the new BQRS-path error message

Update Methodology: This update follows best practices:

  1. ✅ Comprehensive backward compatibility analysis
  2. ✅ Thorough testing of all changes
  3. ✅ Performance impact assessment
  4. ✅ Clear documentation of changes
  5. ✅ Robust error handling improvements
  6. ✅ Maintained code quality standards

Copilot AI requested review from Copilot and removed request for Copilot June 20, 2026 14:55
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 20, 2026 15:18
Copilot AI changed the title [WIP] Fix codeql_bqrs_decode to accept files array argument [UPDATE PRIMITIVE] Accept singular file argument in codeql_bqrs_decode Jun 20, 2026
Copilot AI requested a review from data-douser June 20, 2026 15:21
Copilot AI review requested due to automatic review settings June 20, 2026 15:38
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the codeql_bqrs_decode MCP server tool contract to accept a singular file argument (in addition to files), aligning the advertised schema with the existing registry normalization, and adds tests/docs plus a rebuilt server bundle.

Changes:

  • Extend codeql_bqrs_decode input schema to support optional file and optional files.
  • Add a runtime guard for codeql_bqrs_* tools to produce a clearer “file/files required” error when no BQRS path is provided.
  • Add unit + integration test coverage and update CHANGELOG.md (plus rebuilt server/dist bundle).
Show a summary per file
File Description
server/src/tools/codeql/bqrs-decode.ts Advertises file (singular) alongside files (plural) in the tool input schema.
server/src/lib/cli-tool-registry.ts Adds a BQRS-tool guard error when no positional BQRS path is provided.
server/test/src/tools/codeql/bqrs-decode.test.ts Adds schema-level tests to ensure both file and files parse.
server/test/src/lib/cli-tool-registry.test.ts Adds handler-level tests for file/files normalization and missing-path error behavior.
client/integration-tests/primitives/tools/codeql_bqrs_decode/decode_singular_file/test-config.json New integration test config exercising the singular file argument path.
client/integration-tests/primitives/tools/codeql_bqrs_decode/decode_singular_file/README.md Documents the new integration test’s purpose/inputs/outputs.
client/integration-tests/primitives/tools/codeql_bqrs_decode/decode_singular_file/before/monitoring-state.json New fixture (legacy) for integration test directory structure.
client/integration-tests/primitives/tools/codeql_bqrs_decode/decode_singular_file/after/monitoring-state.json New fixture (legacy) for integration test directory structure.
server/dist/codeql-development-mcp-server.js Rebuilt bundled server output reflecting the TypeScript changes.
CHANGELOG.md Adds an Unreleased “Fixed” entry for the file vs files schema mismatch.

Copilot's findings

  • Files reviewed: 9/12 changed files
  • Comments generated: 3

Comment thread server/src/lib/cli-tool-registry.ts Outdated
Comment thread CHANGELOG.md Outdated
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 9/12 changed files
  • Comments generated: 2

Comment on lines +250 to +257
if (
name.startsWith('codeql_bqrs_') &&
!positionalArgs.some(arg => typeof arg === 'string' && arg.trim() !== '')
) {
throw new Error(
`The "${name}" tool requires a BQRS file path. Provide it via "file" (a single string path) or "files" (an array of string paths).`,
);
}
});

it('should have files as required positional input', () => {
it('should accept a singular file path via the files array input', () => {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants