fix: review-batch — restart race, semver compare, glob over-match, scan rejection paths

Three review rounds turned up these:

- Concurrent-restart race in extension.ts where startClient read the
  module-level `client` after awaiting startup; a parallel stopClient
  could clear it before the post-start wiring ran. Captures the client
  in a local and checks identity after the await.
- whatsNew lexicographic prerelease compare misranked rc.10 < rc.2
  (so a user on rc.2 would miss the rc.10 upgrade toast). Replaced
  with semver §11.4 per-identifier compare (numeric segments compare
  numerically, longer set wins on tie).
- providers.ts `**` glob emitted `.*` and crossed segment boundaries —
  `**/Dockerfile` would match `myDockerfile`. `**/` now translates to
  `(?:.*/)?`.
- scanOnSave propagated scan rejections as unhandled promise rejections
  (onDidSaveTextDocument is fire-and-forget). Added optional `onError`
  hook wired to clientLog.
- scanWorkspace command rejection now goes through a `runScanCommand`
  wrapper that logs and shows a Pipeline-Check-branded toast instead
  of VS Code's generic "Command failed".

Housekeeping:

- log.setLogChannel signature widened to accept `undefined` (drops the
  `as unknown as OutputChannel` test cast).
- manifest.test.ts welcome-link regex now captures dotted command IDs.
- workspaceScan and navigate test names/coverage updated to match what
  they actually test, with new sibling tests for the propagation paths
  (findScannableFiles rejection, two-finding strict-advance).
- Test stub reset consistency in codeLens / findingsView (full
  resetStubState in beforeEach).
- codeql.yml cleanup — drops template scaffolding, keeps the same
  three languages and the pinned action SHAs.
- Two void-prefix consistency fixes on showInformationMessage in
  extension.ts.

Unit tests: 245 → 254 (+9). Typecheck and lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: dmartinochoa

.github/workflows/codeql.yml

@@ -1,106 +1,42 @@
-# For most projects, this workflow file will not need changing; you simply need
-# to commit it to your repository.
-#
-# You may wish to alter this file to override the set of languages analyzed,
-# or to provide custom queries or build logic.
-#
-# ******** NOTE ********
-# We have attempted to detect the languages in your repository. Please check
-# the `language` matrix defined below to confirm you have the correct set of
-# supported CodeQL languages.
-#
-name: "CodeQL Advanced"
+name: CodeQL
+
+# Static analysis on push, PR, and a weekly schedule. The `actions`
+# language catches GHA-specific issues in this repo's own workflows;
+# `javascript-typescript` covers the extension source; `python`
+# covers scripts/gen_icon.py.
 
 on:
   push:
-    branches: [ "main" ]
+    branches: [main]
   pull_request:
-    branches: [ "main" ]
+    branches: [main]
   schedule:
     - cron: '45 3 * * 2'
 
 jobs:
   analyze:
-    timeout-minutes: 30
     name: Analyze (${{ matrix.language }})
-    # Runner size impacts CodeQL analysis time. To learn more, please see:
-    #   - https://gh.io/recommended-hardware-resources-for-running-codeql
-    #   - https://gh.io/supported-runners-and-hardware-resources
-    #   - https://gh.io/using-larger-runners (GitHub.com only)
-    # Consider using larger runners or machines with greater resources for possible analysis time improvements.
-    runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }}
+    timeout-minutes: 30
+    runs-on: ubuntu-latest
     permissions:
-      # required for all workflows
       security-events: write
-
-      # required to fetch internal or private CodeQL packs
       packages: read
-
-      # only required for workflows in private repositories
       actions: read
       contents: read
-
     strategy:
       fail-fast: false
       matrix:
-        include:
-        - language: actions
-          build-mode: none
-        - language: javascript-typescript
-          build-mode: none
-        - language: python
-          build-mode: none
-        # CodeQL supports the following values keywords for 'language': 'actions', 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'rust', 'swift'
-        # Use `c-cpp` to analyze code written in C, C++ or both
-        # Use 'java-kotlin' to analyze code written in Java, Kotlin or both
-        # Use 'javascript-typescript' to analyze code written in JavaScript, TypeScript or both
-        # To learn more about changing the languages that are analyzed or customizing the build mode for your analysis,
-        # see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning.
-        # If you are analyzing a compiled language, you can modify the 'build-mode' for that language to customize how
-        # your codebase is analyzed, see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages
+        language: [actions, javascript-typescript, python]
     steps:
-    - name: Checkout repository
-      uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6
-      with:
-        persist-credentials: false
-
-    # Add any setup steps before running the `github/codeql-action/init` action.
-    # This includes steps like installing compilers or runtimes (`actions/setup-node`
-    # or others). This is typically only required for manual builds.
-    # - name: Setup runtime (example)
-    #   uses: actions/setup-example@v1
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6
+        with:
+          persist-credentials: false
 
-    # Initializes the CodeQL tools for scanning.
-    - name: Initialize CodeQL
-      uses: github/codeql-action/init@9e0d7b8d25671d64c341c19c0152d693099fb5ba  # v4
-      with:
-        languages: ${{ matrix.language }}
-        build-mode: ${{ matrix.build-mode }}
-        # If you wish to specify custom queries, you can do so here or in a config file.
-        # By default, queries listed here will override any specified in a config file.
-        # Prefix the list here with "+" to use these queries and those in the config file.
-
-        # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
-        # queries: security-extended,security-and-quality
-
-    # If the analyze step fails for one of the languages you are analyzing with
-    # "We were unable to automatically build your code", modify the matrix above
-    # to set the build mode to "manual" for that language. Then modify this step
-    # to build your code.
-    # ℹ️ Command-line programs to run using the OS shell.
-    # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun
-    - name: Run manual build steps
-      if: matrix.build-mode == 'manual'
-      shell: bash
-      run: |
-        echo 'If you are using a "manual" build mode for one or more of the' \
-          'languages you are analyzing, replace this with the commands to build' \
-          'your code, for example:'
-        echo '  make bootstrap'
-        echo '  make release'
-        exit 1
+      - uses: github/codeql-action/init@9e0d7b8d25671d64c341c19c0152d693099fb5ba  # v4
+        with:
+          languages: ${{ matrix.language }}
+          build-mode: none
 
-    - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@9e0d7b8d25671d64c341c19c0152d693099fb5ba  # v4
-      with:
-        category: "/language:${{matrix.language}}"
+      - uses: github/codeql-action/analyze@9e0d7b8d25671d64c341c19c0152d693099fb5ba  # v4
+        with:
+          category: "/language:${{ matrix.language }}"

src/__testStubs__/vscode.ts

@@ -82,6 +82,12 @@ declare global {
   var __stubFindFilesByPattern:
     | Record<string, Array<{ toString: () => string; fsPath: string }>>
     | undefined;
+  // When set, every `findFiles` call rejects with this error. Used by
+  // the workspaceScan test that pins propagation of a pre-loop failure
+  // (workspace closed mid-call, fs error) — the case where the scan
+  // never reaches the per-file try/catch and the rejection escapes
+  // scanWorkspace's outer try/finally.
+  var __stubFindFilesError: Error | undefined;
   // What `workspace.workspaceFolders` should return for the duration
   // of a test. `undefined` mimics "no workspace open".
   var __stubWorkspaceFolders:
@@ -147,6 +153,7 @@ export function resetStubState(): void {
   globalThis.__stubDiagnostics = [];
   globalThis.__stubFindFiles = undefined;
   globalThis.__stubFindFilesByPattern = undefined;
+  globalThis.__stubFindFilesError = undefined;
   globalThis.__stubWorkspaceFolders = undefined;
   globalThis.__stubOpenTextDocumentFailures = undefined;
   globalThis.__stubOpenTextDocumentGate = undefined;
@@ -311,6 +318,9 @@ export function vscodeStub(): Record<string, unknown> {
       findFiles: (include: string, exclude?: string, maxResults?: number) => {
         const calls = ensureCalls();
         calls.findFiles.push({ include, exclude, maxResults });
+        if (globalThis.__stubFindFilesError) {
+          return Promise.reject(globalThis.__stubFindFilesError);
+        }
         const byPattern = globalThis.__stubFindFilesByPattern;
         if (byPattern) {
           return Promise.resolve(byPattern[include] ?? []);

src/codeLens.test.ts

@@ -5,6 +5,7 @@ vi.mock("vscode", async () => {
   return vscodeStub();
 });
 
+import { resetStubState } from "./__testStubs__/vscode";
 import {
   FindingsCodeLensProvider,
   composeLensTitle,
@@ -137,6 +138,12 @@ describe("FindingsCodeLensProvider — pipelineCheck.codeLens.enabled toggle", (
   } as unknown as import("vscode").TextDocument;
 
   beforeEach(() => {
+    // Reset every shared global before seeding the slots this suite
+    // actually reads. Resetting only `__stubDiagnostics` + `__stubConfig`
+    // (the old pattern) leaves `__stubCalls.executeCommand` etc.
+    // accumulating from neighbouring tests — fine today, fragile if a
+    // future test in this file starts asserting on the call history.
+    resetStubState();
     (globalThis as { __stubDiagnostics?: unknown }).__stubDiagnostics = [
       [
         { toString: () => "file:///a.yml" },
@@ -154,7 +161,6 @@ describe("FindingsCodeLensProvider — pipelineCheck.codeLens.enabled toggle", (
         ],
       ],
     ];
-    (globalThis as { __stubConfig?: Record<string, unknown> }).__stubConfig = {};
   });
 
   it("emits a lens when codeLens.enabled is true (default)", () => {

src/extension.ts

@@ -88,6 +88,32 @@ async function changeGrouping(
     provider.setGroupMode(choice.mode);
   }
 }
+/**
+ * Wrapper around `scanWorkspace()` for the two user-fired commands
+ * (`Scan workspace` and `Refresh findings`). `scanWorkspace` re-throws
+ * if `findScannableFiles` rejects (workspace closed mid-scan, fs error
+ * before the loop is set up); without this wrapper, the command's
+ * rejected promise lands as a generic "Command 'X' resulted in an
+ * error" toast divorced from the click. The wrapper writes a real
+ * breadcrumb to the log and shows a Pipeline-Check-branded toast so
+ * the user can act on the failure.
+ *
+ * Per-file errors during the loop are already counted as `failed` and
+ * surfaced through `formatSummary`; this path is strictly for failures
+ * that prevent the scan from running at all.
+ */
+async function runScanCommand(): Promise<void> {
+  try {
+    await scanWorkspace();
+  } catch (err) {
+    const message = err instanceof Error ? err.message : String(err);
+    clientLog.error(`scan: failed to start — ${message}`);
+    void vscode.window.showErrorMessage(
+      `Pipeline-Check: scan could not start — ${message}`,
+    );
+  }
+}
+
 const LANGUAGE_ID = "pipelineCheck";
 const LANGUAGE_NAME = "Pipeline-Check";
 const OUTPUT_CHANNEL = "Pipeline-Check";
@@ -190,19 +216,38 @@ async function startClient(): Promise<void> {
     // this guard is the genuinely-duplicate case.
     return;
   }
-  client = buildClient();
+  // Capture the client we just built into a local. The module-level
+  // `client` can be reassigned by a concurrent `stopClient()` while we
+  // sit on the `await` below; reading off the local keeps the
+  // post-start wiring referencing the SAME client we awaited, and lets
+  // us cleanly detect the "ours got swapped out" case before touching
+  // shared state.
+  const local = buildClient();
+  client = local;
   // The output channel is created by LanguageClient as a side effect of
   // construction, so it is always present here. Capture it before we
   // drop the broken client on failure (the "Open server log" action
   // below still needs to focus it to surface the server's traceback).
-  const outputChannel: vscode.OutputChannel = client.outputChannel;
+  const outputChannel: vscode.OutputChannel = local.outputChannel;
   // Point the client-side logger at the same channel the LSP server
   // writes to, so [client] and [server] lines interleave with shared
   // timestamps — much easier to read when triaging a bug report.
   clientLog.setLogChannel(outputChannel);
   try {
     clientLog.info("language server: starting");
-    await startWithTimeout(client, START_TIMEOUT_MS);
+    await startWithTimeout(local, START_TIMEOUT_MS);
+    // Concurrent-restart race: if stopClient() ran while we were
+    // awaiting, the module-level `client` was already reassigned (or
+    // cleared). The LSP we just started is orphaned — best-effort kill
+    // it so we don't leak a subprocess, and bail without flipping any
+    // shared state that now belongs to a different client.
+    if (client !== local) {
+      clientLog.warn(
+        "language server: start completed but client was swapped during startup; killing orphan",
+      );
+      void local.stop().catch(() => undefined);
+      return;
+    }
     clientLog.info("language server: started");
     setLspReady(true);
     // Watch the post-start lifecycle so a mid-session crash (server
@@ -213,7 +258,7 @@ async function startClient(): Promise<void> {
     // "Scan workspace" even though clicking it produces no findings.
     // The listener lives in module scope so stopClient can tear it
     // down before a restart builds a new one against the new client.
-    clientStateChangeDisposable = client.onDidChangeState((event) => {
+    clientStateChangeDisposable = local.onDidChangeState((event) => {
       if (event.newState === State.Stopped) {
         clientLog.warn("language server: state transitioned to stopped");
         setLspReady(false);
@@ -250,10 +295,14 @@ async function startClient(): Promise<void> {
           outputChannel.show();
         }
       });
-    // Drop the broken client so a subsequent restart starts fresh
-    // rather than trying to recover from a half-initialised state.
-    client = undefined;
-    setLspReady(false);
+    // Only clear the module slot if it still points at OUR client. If
+    // stopClient already swapped in a new one (concurrent restart),
+    // clobbering would strand the new client. Same idea on the ready
+    // flag — leave it alone unless we're the live client.
+    if (client === local) {
+      client = undefined;
+      setLspReady(false);
+    }
   }
 }
 
@@ -337,15 +386,15 @@ export async function activate(
     // didOpen pipeline on each. Findings panel updates as the server
     // publishes; no extra state to manage.
     vscode.commands.registerCommand("pipelineCheck.scanWorkspace", () =>
-      scanWorkspace(),
+      runScanCommand(),
     ),
     // "Refresh Findings" was historically a tree-only re-render. Now
     // that we have a real scan command, refresh runs an actual scan so
     // the button matches the user's mental model — clicking "refresh"
     // should fetch fresh data, not re-paint stale data. The tree
     // updates automatically as scan publishes arrive (R10).
     vscode.commands.registerCommand("pipelineCheck.findings.refresh", () =>
-      scanWorkspace(),
+      runScanCommand(),
     ),
     vscode.commands.registerCommand(
       "pipelineCheck.findings.changeGrouping",
@@ -452,14 +501,14 @@ export async function activate(
       // If start failed it surfaced its own error toast; we'd otherwise
       // show "failed to start" and "restarted" at the same time.
       if (client) {
-        vscode.window.showInformationMessage("Language server restarted.");
+        void vscode.window.showInformationMessage("Language server restarted.");
       }
     }),
     vscode.commands.registerCommand("pipelineCheck.showLog", () => {
       if (client?.outputChannel) {
         client.outputChannel.show();
       } else {
-        vscode.window.showInformationMessage(
+        void vscode.window.showInformationMessage(
           "The language server is not running yet. Open a supported file " +
             "or run 'Pipeline-Check: Restart language server'.",
         );
@@ -498,6 +547,15 @@ export async function activate(
       return !disabled.includes(provider);
     },
     scan: () => scanWorkspace({ quiet: true }),
+    // Surface a scan failure to the log instead of letting it bubble
+    // out as an unhandled rejection. onDidSaveTextDocument doesn't
+    // await its listener, so without this hook the only trace would
+    // be a generic extension-host error nobody connects back to a
+    // save event — the log line names the symptom clearly.
+    onError: (err) => {
+      const msg = err instanceof Error ? err.message : String(err);
+      clientLog.error(`scan-on-save: scan failed — ${msg}`);
+    },
   });
   context.subscriptions.push(vscode.workspace.onDidSaveTextDocument(onSave));
 

src/findingsView.test.ts

@@ -10,6 +10,7 @@ vi.mock("vscode", async () => {
 });
 
 // Import after the mock is registered.
+import { resetStubState } from "./__testStubs__/vscode";
 import { FindingsTreeProvider } from "./findingsView";
 
 const ctx = {
@@ -65,7 +66,12 @@ function setStubDiagnostics(findings: FakeFinding[]): void {
 }
 
 beforeEach(() => {
-  (globalThis as { __stubDiagnostics?: unknown }).__stubDiagnostics = [];
+  // Full reset (not just `__stubDiagnostics`) so the shared `__stubCalls`
+  // — populated by every FindingsTreeProvider constructor via the
+  // `setContext` executeCommand — doesn't accumulate across tests.
+  // Currently no test asserts on that history; the reset keeps a
+  // future assertion honest.
+  resetStubState();
 });
 
 describe("FindingsTreeProvider — collection from diagnostics", () => {

src/log.test.ts

@@ -35,10 +35,9 @@ function fakeChannel() {
 
 beforeEach(() => {
   // Reset the module-scope channel so a test that doesn't call
-  // setLogChannel can verify the no-op path.
-  setLogChannel(
-    undefined as unknown as import("vscode").OutputChannel,
-  );
+  // setLogChannel can verify the no-op path. The signature now
+  // accepts `undefined` directly — no `as unknown as` cast needed.
+  setLogChannel(undefined);
 });
 
 describe("formatTimestamp", () => {

src/log.ts

@@ -17,8 +17,13 @@ let channel: vscode.OutputChannel | undefined;
  * Set the output channel logs are written to. Called once from
  * activate() after the LanguageClient has constructed its channel,
  * so client logs and server logs share the same surface.
+ *
+ * Accepts `undefined` so a test (or any future caller that needs to
+ * reset state) can detach without an `as unknown as OutputChannel`
+ * cast. The module already treated a missing channel as a no-op; the
+ * signature now documents that explicitly.
  */
-export function setLogChannel(c: vscode.OutputChannel): void {
+export function setLogChannel(c: vscode.OutputChannel | undefined): void {
   channel = c;
 }