test: extract 3 modules from extension.ts + 38 new tests (194 → 232)

Three thin extractions out of the activate() closure so the behaviour
they encapsulate can be exercised in unit tests rather than waiting
for the @vscode/test-electron integration run:

- lspStart.ts: bounded-time client.start(). Internally restructured
  from Promise.race([start, timeout]) to a single deferred with a
  settled flag — Promise.race plus vitest's fake timers triggered
  "rejection handled asynchronously" warnings because Promise.race
  attaches its handler one microtask after the timer fires the
  rejection. The deferred avoids the two-promise dance.
- middleware.ts: pure transformDiagnostics(uri, diagnostics, config)
  composing the disabledProviders blanket-drop and the
  severityThreshold filter. extension.ts now plumbs the config in
  via deps; the composition (which the user sees) is testable in
  isolation.
- scanOnSave.ts: createScanOnSaveHandler(deps) returns a save handler
  that closes over a busy flag. Cheap-gates-first ordering
  (isEnabled → isPipelineFile → busy) is pinned; each handler
  instance has its own busy flag so two windows don't share a lock.

CodeLens provider class gets four new tests pinning the lens range
(line 0 col 0), the click target (pipelineCheck.findings.focus), the
"reads live counts each call" contract (so a future cache-once-at-
construction refactor trips), and the per-document scope (lens
reflects only this document's diagnostics, not the workspace total).

Tests breakdown:
- lspStart: 12 tests (timeout race, stop-on-timeout, settled-flag, copy)
- middleware: 13 tests (disabled stage, severity stage, composition)
- scanOnSave: 9 tests (gates, busy guard, per-instance lock)
- codeLens: +4 tests on the existing file

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

Author: dmartinochoa

src/codeLens.test.ts

@@ -176,4 +176,106 @@ describe("FindingsCodeLensProvider — pipelineCheck.codeLens.enabled toggle", (
     const p = new FindingsCodeLensProvider(ctx);
     expect(p.provideCodeLenses(document)).toEqual([]);
   });
+
+  it("anchors the lens at line 0 column 0 (top of file)", () => {
+    // The lens sits *above* line 1 so it doesn't push the first
+    // line of YAML out of view. A future refactor that moves it to
+    // the first finding's line would change UX without anyone
+    // noticing; lock it down.
+    const p = new FindingsCodeLensProvider(ctx);
+    const lenses = p.provideCodeLenses(document) as unknown[];
+    expect(lenses).toHaveLength(1);
+    const lens = lenses[0] as { range: { start: { line: number; character: number } } };
+    expect(lens.range.start.line).toBe(0);
+    expect(lens.range.start.character).toBe(0);
+  });
+
+  it("targets the pipelineCheck.findings.focus command so click reveals the panel", () => {
+    // Other plausible click targets (vscode.open, the file at the
+    // finding location, the rule docs URL) all do different things.
+    // The lens is meant as a *drill-in* — surface the panel grouped
+    // by severity so the user can scan the per-file count in context.
+    const p = new FindingsCodeLensProvider(ctx);
+    const lenses = p.provideCodeLenses(document) as unknown[];
+    const lens = lenses[0] as {
+      command?: { command: string; title: string };
+    };
+    expect(lens.command?.command).toBe("pipelineCheck.findings.focus");
+  });
+
+  it("renders the title from the live count, not the constructor snapshot", () => {
+    // Without rebuilding the provider, the lens text must reflect
+    // whatever the diagnostic store says right now. (The provider
+    // calls summariseCounts inside provideCodeLenses each time, but
+    // the test exists to catch a future "cache once at construction"
+    // refactor.)
+    const p = new FindingsCodeLensProvider(ctx);
+    const lenses1 = p.provideCodeLenses(document) as unknown[];
+    const title1 = (lenses1[0] as { command?: { title: string } }).command
+      ?.title;
+    expect(title1).toContain("1 critical");
+
+    // Now swap the stub diagnostics underneath the provider and
+    // request fresh lenses.
+    (globalThis as { __stubDiagnostics?: unknown }).__stubDiagnostics = [
+      [
+        { toString: () => "file:///a.yml" },
+        [
+          {
+            source: "pipeline-check",
+            message: "",
+            range: {
+              start: { line: 0, character: 0 },
+              end: { line: 0, character: 0 },
+            },
+            severity: 0,
+            data: { severity: "HIGH" },
+          },
+          {
+            source: "pipeline-check",
+            message: "",
+            range: {
+              start: { line: 0, character: 0 },
+              end: { line: 0, character: 0 },
+            },
+            severity: 0,
+            data: { severity: "HIGH" },
+          },
+        ],
+      ],
+    ];
+    const lenses2 = p.provideCodeLenses(document) as unknown[];
+    const title2 = (lenses2[0] as { command?: { title: string } }).command
+      ?.title;
+    expect(title2).toContain("2 high");
+    expect(title2).not.toContain("critical");
+  });
+
+  it("only considers the document's OWN diagnostics, not the workspace total", () => {
+    // The lens is per-file; the Findings panel is the workspace
+    // aggregate. Confusing the two would show "10 critical" on a
+    // file with zero findings just because the workspace has them.
+    (globalThis as { __stubDiagnostics?: unknown }).__stubDiagnostics = [
+      [
+        { toString: () => "file:///OTHER.yml" },
+        [
+          {
+            source: "pipeline-check",
+            message: "",
+            range: {
+              start: { line: 0, character: 0 },
+              end: { line: 0, character: 0 },
+            },
+            severity: 0,
+            data: { severity: "CRITICAL" },
+          },
+        ],
+      ],
+    ];
+    const p = new FindingsCodeLensProvider(ctx);
+    // The stub's getDiagnostics(uri) returns diagnostics keyed by
+    // `uri.toString()`; our document is "file:///a.yml", which is
+    // not in the stubbed store. Should yield no lenses.
+    expect(p.provideCodeLenses(document)).toEqual([]);
+  });
 });

src/extension.ts

@@ -26,14 +26,15 @@ import {
   installInTerminal,
 } from "./install";
 import * as clientLog from "./log";
+import { startWithTimeout } from "./lspStart";
 import { setLspReady } from "./lspState";
+import { transformDiagnostics } from "./middleware";
 import { goToFinding } from "./navigate";
 import {
   providerForPath,
-  type ProviderId,
   TRIGGER_DOCUMENT_SELECTOR,
 } from "./providers";
-import { filterByThreshold } from "./severityFilter";
+import { createScanOnSaveHandler } from "./scanOnSave";
 import { registerStatusBar } from "./statusBar";
 import { scanWorkspace } from "./workspaceScan";
 import { showWhatsNewIfUpgraded } from "./whatsNew";
@@ -155,30 +156,21 @@ function buildClient(): LanguageClient {
     },
     outputChannelName: OUTPUT_CHANNEL,
     middleware: {
-      // Drop diagnostics below the user-configured severity threshold
-      // before they reach VS Code's Problems panel. The config is
-      // re-read on each publish so a settings change takes effect on
-      // the next scan without needing a server restart. Diagnostics
-      // whose `data.severity` is missing (older server, or a
-      // not-from-pipeline-check publish that somehow flowed through)
-      // pass through unconditionally so the filter never hides
-      // legitimate signal when the metadata is absent.
+      // Two-stage filter (composition lives in middleware.ts): drop
+      // every diagnostic for a URI whose provider the user has
+      // silenced via `disabledProviders`, otherwise drop those below
+      // the configured `severityThreshold`. Re-reads the config on
+      // each publish so a settings change takes effect on the next
+      // scan without a server restart.
       handleDiagnostics: (uri, diagnostics, next) => {
         const config = vscode.workspace.getConfiguration("pipelineCheck");
-        // Per-provider toggle: if this URI maps to a provider the
-        // user has disabled, drop every diagnostic for it. We still
-        // accept the publish (so a future "unset disable" causes a
-        // fresh publish to reach us), we just blank the list.
-        const disabled = new Set(
-          config.get<string[]>("disabledProviders", []) as ProviderId[],
+        next(
+          uri,
+          transformDiagnostics(uri, diagnostics, {
+            disabledProviders: config.get<string[]>("disabledProviders", []),
+            severityThreshold: config.get<string>("severityThreshold", "low"),
+          }),
         );
-        const provider = providerForPath(uri.fsPath);
-        if (provider && disabled.has(provider)) {
-          next(uri, []);
-          return;
-        }
-        const threshold = config.get<string>("severityThreshold", "low");
-        next(uri, filterByThreshold(diagnostics, threshold));
       },
     },
   };
@@ -304,41 +296,6 @@ async function stopClient(): Promise<void> {
   }
 }
 
-/**
- * Race the LSP startup handshake against a hard ceiling. On timeout
- * we call `client.stop()` to kill the stranded subprocess (best
- * effort — the server may already be hung past saving) and throw a
- * recognisable error the caller's catch surfaces in the failure
- * toast.
- */
-async function startWithTimeout(
-  c: LanguageClient,
-  timeoutMs: number,
-): Promise<void> {
-  let timer: NodeJS.Timeout | undefined;
-  const timeout = new Promise<never>((_, reject) => {
-    timer = setTimeout(() => {
-      // Fire-and-forget the cleanup stop(); we don't await it because
-      // a hung interpreter probably won't respond, and the timeout
-      // toast wants to be on screen now, not in 30 seconds when stop
-      // gives up.
-      void c.stop().catch(() => undefined);
-      reject(
-        new Error(
-          `Language server did not finish startup within ${Math.round(timeoutMs / 1000)}s. Check the server log; common causes are an empty pipelineCheck.serverArgs, an interpreter that drops into the REPL, or a corrupted pipeline_check install.`,
-        ),
-      );
-    }, timeoutMs);
-  });
-  try {
-    await Promise.race([c.start(), timeout]);
-  } finally {
-    if (timer) {
-      clearTimeout(timer);
-    }
-  }
-}
-
 export async function activate(
   context: vscode.ExtensionContext,
 ): Promise<void> {
@@ -518,32 +475,18 @@ export async function activate(
   // `didSave`, so this is purely about picking up cross-file effects in
   // *other* CI files (a Jenkinsfile that includes the just-edited
   // library, a GHA workflow that calls the just-edited composite
-  // action). A simple in-flight guard prevents save-storms (autosave,
-  // Save All) from queueing redundant scans.
-  let scanOnSaveBusy = false;
-  context.subscriptions.push(
-    vscode.workspace.onDidSaveTextDocument(async (doc) => {
-      const config = vscode.workspace.getConfiguration("pipelineCheck");
-      if (!config.get<boolean>("scanOnSave", false)) {
-        return;
-      }
-      // Only react to saves of CI-relevant files. providerForPath
-      // returns `undefined` for anything that doesn't match our
-      // glob patterns, so package.json / random YAML never triggers.
-      if (!providerForPath(doc.uri.fsPath)) {
-        return;
-      }
-      if (scanOnSaveBusy) {
-        return;
-      }
-      scanOnSaveBusy = true;
-      try {
-        await scanWorkspace({ quiet: true });
-      } finally {
-        scanOnSaveBusy = false;
-      }
-    }),
-  );
+  // action). Busy-guard semantics + the gate logic live in
+  // src/scanOnSave.ts so they're unit-testable without a real save
+  // event source; this wiring just plumbs VS Code's dependencies in.
+  const onSave = createScanOnSaveHandler({
+    isEnabled: () =>
+      vscode.workspace
+        .getConfiguration("pipelineCheck")
+        .get<boolean>("scanOnSave", false),
+    isPipelineFile: (fsPath) => providerForPath(fsPath) !== undefined,
+    scan: () => scanWorkspace({ quiet: true }),
+  });
+  context.subscriptions.push(vscode.workspace.onDidSaveTextDocument(onSave));
 
   // Fire-and-forget the one-time "what's new" toast for users who
   // just upgraded. Detached so a not-yet-dismissed notification never