Fix deterministic vscode-codeql discovery tests and dual-casing path probe in CliResolver

[Copilot]

Address two review issues from PR #91: discoverVsCodeCodeQLDistribution tests were non-asserting tautologies dependent on machine state, and CliResolver.resolveFromVsCodeDistribution() would miss the managed CLI on case-sensitive filesystems when the on-disk casing differed from the StoragePaths-provided path.

📝 Update Information

Primitive Details

• Type: Both
• Name: discoverVsCodeCodeQLDistribution (server), CliResolver (vscode extension)
• Update Category: Bug Fix, Test Quality

⚠️ CRITICAL: PR SCOPE VALIDATION

✅ 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

---

🛑 MANDATORY PR VALIDATION CHECKLIST

• 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 — candidateStorageRoots is an optional additive parameter
• API Compatibility: Enhanced
• Performance Impact: Neutral

🎯 Changes Description

Current Behavior

• discoverVsCodeCodeQLDistribution() used hardcoded platform paths; tests either never called the function or asserted result === undefined || typeof result === 'string' (always true). Tests also called getResolvedCodeQLDir() and asserted a tautology.
• CliResolver.resolveFromVsCodeDistribution() treated vsCodeCodeqlStoragePath as exact. StoragePaths always produces GitHub.vscode-codeql (publisher casing), but VS Code lowercases the directory to github.vscode-codeql on some Linux installations — discovery silently failed.

Updated Behavior

• discoverVsCodeCodeQLDistribution(candidateStorageRoots?) accepts injectable roots; tests use project temp dirs with real file structures and assert the exact returned binary path.
• CliResolver.resolveFromVsCodeDistribution() always probes both GitHub.vscode-codeql and github.vscode-codeql siblings under the same parent, using a Set to deduplicate if the provided path already matches one casing.

Motivation

Non-asserting tests give false confidence and mask regressions. The casing mismatch caused silent discovery failure on case-sensitive filesystems (common Linux CI/developer setups).

🔄 Before vs. After Comparison

Functionality Changes

// BEFORE: not injectable, real platform paths only
export function discoverVsCodeCodeQLDistribution(): string | undefined {
  const globalStorageCandidates = getVsCodeGlobalStorageCandidates();
  ...
}

// AFTER: injectable for deterministic testing
export function discoverVsCodeCodeQLDistribution(
  candidateStorageRoots?: string[]
): string | undefined {
  const globalStorageCandidates = candidateStorageRoots ?? getVsCodeGlobalStorageCandidates();
  ...
}

// BEFORE: single exact path probe in CliResolver
private async resolveFromVsCodeDistribution(): Promise<string | undefined> {
  if (!this.vsCodeCodeqlStoragePath) return undefined;
  const hintPath = await this.resolveFromDistributionJson(); // uses this.vsCodeCodeqlStoragePath
  ...
}

// AFTER: probes both casings
private async resolveFromVsCodeDistribution(): Promise<string | undefined> {
  if (!this.vsCodeCodeqlStoragePath) return undefined;
  const parent = dirname(this.vsCodeCodeqlStoragePath);
  const candidatePaths = [...new Set([
    this.vsCodeCodeqlStoragePath,
    join(parent, 'github.vscode-codeql'),
    join(parent, 'GitHub.vscode-codeql'),
  ])];
  for (const storagePath of candidatePaths) { ... }
}

API Changes

// BEFORE:
export function discoverVsCodeCodeQLDistribution(): string | undefined

// AFTER (backward compatible):
export function discoverVsCodeCodeQLDistribution(
  candidateStorageRoots?: string[]
): string | undefined

Output Format Changes

No output format changes.

🧪 Testing & Validation

Test Coverage Updates

• Existing Tests: All 920 server unit tests pass
• New Test Cases: discoverVsCodeCodeQLDistribution tests rewritten with real FS structure and concrete path assertions
• Regression Tests: Precedence and sort-order behavior explicitly tested
• Edge Case Tests: Alternate-casing discovery added to cli-resolver.test.ts

Validation Scenarios

1. distribution.json fast path: temp dir with distribution.json pointing to index 3 → asserts exact binary path returned
2. Directory scan fallback: no distribution.json, two distribution dirs → asserts highest-numbered dir picked
3. Precedence: distribution.json pointing at index 2 with index 3 also present → asserts index 2 returned
4. Numeric sort order: distribution1, distribution2, distribution10 → asserts distribution10 picked
5. Alternate casing (CliResolver): GitHub.vscode-codeql provided but only github.vscode-codeql on disk → asserts binary resolved

Test Results

• Unit Tests: All pass (920/920)
• Integration Tests: Pre-existing codeql_pack_install failure unrelated to these changes
• Manual Testing: Validated with real scenarios

📋 Implementation Details

Files Modified

• Supporting Libraries: server/src/lib/cli-executor.ts — added candidateStorageRoots param
• Core Implementation: extensions/vscode/src/codeql/cli-resolver.ts — dual-casing probe, refactored private methods to accept explicit storagePath
• Tests: server/test/src/lib/cli-executor.test.ts — deterministic discoverVsCodeCodeQLDistribution tests, removed tautological assertions
• Tests: extensions/vscode/test/codeql/cli-resolver.test.ts — alternate-casing test added

Code Changes Summary

• Error Handling: resolveFromDistributionJson/Scan now accept explicit path, eliminating reliance on this.vsCodeCodeqlStoragePath
• Testability: Injectable candidate roots enable deterministic unit tests without mocking the FS
• Type Safety: Parameter is string[] | undefined; no runtime behavior change when omitted

Dependencies

• No New Dependencies: Update uses existing dependencies only

🔍 Quality Improvements

Bug Fixes

• Issue: CliResolver missed the managed CLI when VS Code lowercases the extension storage directory
• Root Cause: Single-path probe trusting the StoragePaths-provided casing exactly
• Solution: Probe all known casings (github.vscode-codeql, GitHub.vscode-codeql) from the same parent
• Prevention: Covered by new alternate-casing test in cli-resolver.test.ts

Code Quality Enhancements

• Testability: discoverVsCodeCodeQLDistribution tests are now fully deterministic and machine-independent
• Readability: Removed misleading tautological assertions and dead existsSync import
• Maintainability: Private helpers accept explicit storagePath instead of implicit instance field

🔗 References

Related Issues/PRs

• Related PRs: Support automatic discovery of codeql CLI distributions installed off-PATH by VS Code extension #91 (parent PR whose review comments this addresses)

🚀 Compatibility & Migration

Backward Compatibility

• Fully Compatible: No breaking changes

API Evolution

• Enhanced Parameters: Optional candidateStorageRoots added to discoverVsCodeCodeQLDistribution
• 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: Dual-casing probe covers all known VS Code storage directory variants
• Test Coverage: Deterministic tests assert exact paths and precedence
• Code Quality: Maintains or improves code quality

Testing Instructions

# Full build and test
npm run build-and-test

# Server unit tests only
cd server && npm test

# Extension lint check
cd extensions/vscode && npx tsc --noEmit

📊 Impact Assessment

Server Impact

• Startup Time: No significant impact
• Runtime Stability: No impact
• Resource Usage: Negligible (two extra existsSync checks at startup)
• Concurrent Usage: Safe for concurrent access

AI Assistant Impact

• Improved Reliability: CLI auto-discovery now works on case-sensitive Linux filesystems where VS Code lowercases the extension storage directory

🔄 Deployment Strategy

Rollout Considerations

• Safe Deployment: Fully backward compatible additive change

---

Update Methodology:

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

---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.