Fix getExtensionVersion() and getBundledQlRoot() per review comments

[Copilot]

Two issues flagged in review on extensions/vscode/src/server/server-manager.ts: getExtensionVersion() used context.extension.packageJSON.version (opaque runtime API), and getBundledQlRoot() had a docstring and log message claiming "VSIX-bundled" even when returning the monorepo dev path.

📝 Update Information

Primitive Details

• Type: Tool
• Name: ServerManager (extensions/vscode/src/server/server-manager.ts)
• Update Category: Bug Fix / Code Quality

⚠️ CRITICAL: PR SCOPE VALIDATION

✅ ALLOWED FILES:

• Server implementation files (extensions/vscode/src/**/*.ts)
• Updated or new test files (extensions/vscode/test/**/*.ts)

🚫 FORBIDDEN FILES: None included.

---

🛑 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
• API Compatibility: Maintained
• Performance Impact: Neutral

🎯 Changes Description

Current Behavior

• getExtensionVersion() reads context.extension.packageJSON.version — depends on the VS Code extension registry being populated, fragile in test contexts.
• getBundledQlRoot() docstring says "VSIX-bundled" and ensureInstalled() logs "Using VSIX-bundled server" even when the function returns the monorepo ../../server path (e.g. Extension Development Host without prepublish).

Updated Behavior

• getExtensionVersion() reads package.json directly from context.extensionUri.fsPath via readFileSync, explicit and context-independent.
• getBundledQlRoot() docstring documents both resolution paths (VSIX layout and monorepo dev layout). Log message changed to "Using bundled server" to avoid misleading output in both cases.

Motivation

Review feedback: runtime context.extension access was opaque and hard to test; "VSIX-bundled" label in logs/docs was actively misleading when the monorepo path was resolved.

🔄 Before vs. After Comparison

Functionality Changes

// BEFORE
getExtensionVersion(): string {
  return this.context.extension.packageJSON.version as string;
}

// AFTER
getExtensionVersion(): string {
  try {
    const pkgPath = join(this.context.extensionUri.fsPath, 'package.json');
    const pkg = JSON.parse(readFileSync(pkgPath, 'utf-8')) as { version?: string };
    return pkg.version ?? 'unknown';
  } catch {
    return 'unknown';
  }
}

// BEFORE: ensureInstalled() log
`Using VSIX-bundled server (v${this.getExtensionVersion()}). No npm install required.`

// AFTER
`Using bundled server (v${this.getExtensionVersion()}). No npm install required.`

API Changes

No schema changes. Internal method signatures preserved.

Output Format Changes

No output format changes.

🧪 Testing & Validation

Test Coverage Updates

• Existing Tests: All 98 existing tests continue to pass
• New Test Cases: readFileSync mock added for getExtensionVersion() and relevant ensureInstalled() scenarios
• Regression Tests: N/A — no regression risk introduced
• Edge Case Tests: getExtensionVersion() returns 'unknown' on read/parse failure

Validation Scenarios

1. Backward Compatibility: getExtensionVersion() returns the correct semver string when package.json is present; returns 'unknown' gracefully on failure.
2. New Functionality: N/A — behavior-equivalent refactor.
3. Error Handling: try/catch in getExtensionVersion() handles missing or malformed package.json.

Test Results

• Unit Tests: All pass (98/98)
• Integration Tests: N/A (VS Code runtime unavailable in CI sandbox)
• Manual Testing: Build verified clean, lint passes
• Performance Testing: No regressions detected

📋 Implementation Details

Files Modified

• Core Implementation: extensions/vscode/src/server/server-manager.ts
• Tests: extensions/vscode/test/server/server-manager.test.ts

Code Changes Summary

• Error Handling: getExtensionVersion() now catches I/O/parse failures and returns 'unknown'
• Readability: getBundledQlRoot() docstring now accurately describes both resolution paths

Dependencies

• No New Dependencies: Added readFileSync from the already-imported fs module

🔍 Quality Improvements

Bug Fixes (if applicable)

• Issue: ensureInstalled() logged "Using VSIX-bundled server" even in monorepo dev layout.
• Root Cause: getBundledQlRoot() resolved two distinct paths but was labelled/documented as if VSIX-only.
• Solution: Updated docstring and log message to correctly reflect both locations.
• Prevention: Accurate docstrings validated via code review.

Code Quality Enhancements

• Readability: getBundledQlRoot() docstring documents both VSIX and monorepo resolution steps
• Maintainability: getExtensionVersion() no longer depends on VS Code runtime extension registry
• Testability: readFileSync is directly mockable; removed implicit dependency on context.extension

🔗 References

Related Issues/PRs

• Related PRs: Addresses review comments from PR Prep for v2.24.2 release #81 review thread

🚀 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 extension implementation files
• ⚠️ NO UNRELATED FILES: No temporary, output, or unrelated files
• ⚠️ BACKWARD COMPATIBILITY: getExtensionVersion() and getBundledQlRoot() semantics unchanged
• Functionality: Log message and docstring now accurately reflect both VSIX and monorepo paths
• Test Coverage: 98/98 tests pass, readFileSync properly mocked
• Code Quality: Improved clarity and testability

Testing Instructions

cd extensions/vscode
npm run build          # lint + bundle
npm run test:coverage  # 98 unit tests

📊 Impact Assessment

Server Impact

• Startup Time: No impact
• Runtime Stability: No impact
• Resource Usage: Negligible — one synchronous readFileSync at version check time
• Concurrent Usage: Safe (read-only)

AI Assistant Impact

• Improved Reliability: Accurate version reporting in logs regardless of execution context

🔄 Deployment Strategy

Rollout Considerations

• Safe Deployment: Fully safe — behavior-equivalent with improved logging accuracy and error resilience

---

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

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• update.code.visualstudio.com
  • Triggering command: /opt/hostedtoolcache/node/24.13.0/x64/bin/node node /home/REDACTED/work/codeql-development-mcp-server/codeql-development-mcp-server/node_modules/.bin/vscode-test (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)