[UPDATE PRIMITIVE] Fix additionalArgs precedence for --dump-dil/--no-dump-dil in codeql_query_compile

[Copilot]

effectiveDumpDilEnabled was computed before additionalArgs were extracted, so --dump-dil in additionalArgs could not override dump-dil: false set as a named parameter — and --dump-dil/--no-dump-dil flags were passed to the CLI as raw args alongside the conflicting named option.

📝 Update Information

Primitive Details

• Type: Tool
• Name: codeql_query_compile
• 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: Maintained
• Performance Impact: Neutral

🎯 Changes Description

Current Behavior

effectiveDumpDilEnabled was computed by reading options.additionalArgs directly before those args were extracted from options. This caused two bugs:

1. dump-dil: false + --dump-dil in additionalArgs → DIL disabled (wrong; additionalArgs should win)
2. --dump-dil/--no-dump-dil remained in rawAdditionalArgs and were forwarded to the CLI alongside the named option, producing conflicting duplicate flags

Updated Behavior

rawAdditionalArgs is now extracted (as let) and options.additionalArgs deleted before normalization. The last --dump-dil/--no-dump-dil flag found in additionalArgs is used to override options['dump-dil'], and those flags are then filtered out of rawAdditionalArgs. effectiveDumpDilEnabled is derived from the resulting normalized options['dump-dil'].

Motivation

Review comment r3067391920 identified the precedence inversion and duplicate-flag issue.

🔄 Before vs. After Comparison

Functionality Changes

// BEFORE: effectiveDumpDilEnabled computed before additionalArgs extracted;
// dump-dil:false + --dump-dil in additionalArgs incorrectly disabled DIL
const pendingArgs = Array.isArray(options.additionalArgs) ? options.additionalArgs : [];
const effectiveDumpDilDisabled = options['dump-dil'] === false
  || pendingArgs.includes('--no-dump-dil');
effectiveDumpDilEnabled = !effectiveDumpDilDisabled;

const rawAdditionalArgs = Array.isArray(options.additionalArgs) ? options.additionalArgs : [];
delete options.additionalArgs;

// AFTER: extract additionalArgs first, normalize dump-dil flags, then compute flag
let rawAdditionalArgs = Array.isArray(options.additionalArgs) ? options.additionalArgs : [];
delete options.additionalArgs;

// Last --dump-dil / --no-dump-dil in additionalArgs overrides the named param
const lastDumpDilFlag = [...rawAdditionalArgs].reverse().find(
  (arg) => arg === '--dump-dil' || arg === '--no-dump-dil',
);
if (lastDumpDilFlag === '--dump-dil')       options['dump-dil'] = true;
else if (lastDumpDilFlag === '--no-dump-dil') options['dump-dil'] = false;
if (lastDumpDilFlag !== undefined) {
  rawAdditionalArgs = rawAdditionalArgs.filter(
    (arg) => arg !== '--dump-dil' && arg !== '--no-dump-dil',
  );
}

effectiveDumpDilEnabled = options['dump-dil'] !== false; // simplified; derived from normalized options

API Changes

No schema changes. Behavioral fix only.

Output Format Changes

No output format changes.

🧪 Testing & Validation

Test Coverage Updates

• Existing Tests: All existing tests continue to pass
• New Test Cases: Added test for dump-dil:false + --dump-dil in additionalArgs → additionalArgs wins
• Regression Tests: Updated assertions for --no-dump-dil / --dump-dil in additionalArgs tests to reflect new normalization behavior
• Edge Case Tests: Covered flag precedence edge case that was previously untested

Validation Scenarios

1. Backward Compatibility: No named-param-only usage is affected; default behavior (dump-dil enabled) unchanged
2. New Functionality: dump-dil:false + --dump-dil in additionalArgs now correctly enables DIL
3. Error Handling: No change to error paths
4. Deduplication: --dump-dil/--no-dump-dil no longer appear in raw CLI args when normalized via options

Test Results

• Unit Tests: All pass (1329/1329)
• Integration Tests: Not applicable for this change
• Manual Testing: Validated with real scenarios
• Performance Testing: No regressions detected

📋 Implementation Details

Files Modified

• Supporting Libraries: server/src/lib/cli-tool-registry.ts
• Tests: server/test/src/lib/cli-tool-registry.test.ts

Code Changes Summary

• Algorithm Improvements: Reordered rawAdditionalArgs extraction before effectiveDumpDilEnabled computation; added normalization pass for --dump-dil/--no-dump-dil
• Type Safety: rawAdditionalArgs changed from const to let to allow filtering

Dependencies

• No New Dependencies: Update uses existing dependencies only

🔍 Quality Improvements

Bug Fixes (if applicable)

• Issue: additionalArgs: ['--dump-dil'] could not override dump-dil: false named param; conflicting flags were forwarded to the CLI
• Root Cause: effectiveDumpDilEnabled was computed before options.additionalArgs was extracted, so the flag check and normalization operated on the wrong state
• Solution: Extract and delete options.additionalArgs first, normalize dump-dil flags (with additionalArgs taking precedence), filter them from rawAdditionalArgs, then compute effectiveDumpDilEnabled
• Prevention: Flag normalization is now a single, ordered pass with a clear precedence rule

Code Quality Enhancements

• Readability: Normalization logic is self-contained and clearly commented
• Maintainability: effectiveDumpDilEnabled derivation simplified to a single expression
• Testability: New test case covers the previously broken precedence scenario

🔗 References

Related Issues/PRs

• Related PRs: [UPDATE PRIMITIVE] codeql_query_compile enables --dump-dil by default with .dil file persistence #235 (original --dump-dil feature PR)

🚀 Compatibility & Migration

Backward Compatibility

• Fully Compatible: No breaking changes

API Evolution

• Maintained Contracts: Core API contracts 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
• Functionality: additionalArgs --dump-dil/--no-dump-dil correctly overrides named dump-dil param in both directions
• Test Coverage: 1329/1329 tests pass; new precedence test added
• Code Quality: Normalization is a clean, ordered single pass

Testing Instructions

npm install
npm run build
npm test

# Target the specific new/updated tests
npm test -- --grep "dump-dil"

Validation Checklist

1. Regression Testing: 1329/1329 unit tests pass
2. New Feature Testing: dump-dil:false + --dump-dil in additionalArgs → DIL enabled ✓
3. Error Testing: No change to error paths
4. Integration Testing: N/A

📊 Impact Assessment

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

• Improved Reliability: codeql_query_compile now honors flag precedence correctly

🔄 Deployment Strategy

Rollout Considerations

• Safe Deployment: Can be deployed safely to production

---

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