From d28652eb670672df043767ece1ec08c904aba047 Mon Sep 17 00:00:00 2001 From: Daniel Martin <56157528+dmartinochoa@users.noreply.github.com> Date: Tue, 19 May 2026 17:27:59 +0200 Subject: [PATCH 1/4] refactor: extract install + lspState modules from extension.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extension.ts has been growing a long tail of one-off helpers — the install-in-terminal flow, the clipboard-fallback copier, and the `pipelineCheck.lspReady` context-key setter — all anchored to that file because they read or write vscode-namespace APIs. Each one is small but they collectively obscure the extension's main job (activation orchestration), and none of them were unit-testable while they lived inside the activate() closure. Extract three things: - install.ts: `installInTerminal`, `copyInstallCommandToClipboard`, `PIP_INSTALL_COMMAND`. Both the welcome-panel CTA and the LSP- failure toast now share one implementation per CTA. - lspState.ts: `setLspReady` + the `LSP_READY_CONTEXT_KEY` constant that drives the conditional viewsWelcome entries. Single writer, single key name, single hop to VS Code's setContext channel. - workspaceScan.ts now exports `findScannableFiles` so the unit suite can pin the per-pattern findFiles contract that fences against the nested-brace bug returning. No behaviour change. extension.ts shrinks; the new modules are pure and unit-testable. Follow-up commit wires up the tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/extension.ts | 48 +++++++++----------------------------------- src/install.ts | 44 ++++++++++++++++++++++++++++++++++++++++ src/lspState.ts | 28 ++++++++++++++++++++++++++ src/workspaceScan.ts | 6 +++++- 4 files changed, 87 insertions(+), 39 deletions(-) create mode 100644 src/install.ts create mode 100644 src/lspState.ts diff --git a/src/extension.ts b/src/extension.ts index 2076fda..95829e5 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -20,7 +20,12 @@ import { } from "vscode-languageclient/node"; import { FindingsCodeLensProvider } from "./codeLens"; import { FindingsTreeProvider, GroupMode } from "./findingsView"; +import { + copyInstallCommandToClipboard, + installInTerminal, +} from "./install"; import * as clientLog from "./log"; +import { setLspReady } from "./lspState"; import { goToFinding } from "./navigate"; import { providerForPath, @@ -105,35 +110,6 @@ type LeafLike = { let client: LanguageClient | undefined; -// Drives the `when` clauses on the two `viewsWelcome` entries: an -// install-prompt panel before the LSP comes up, a "Scan workspace" -// panel once it is connected. Defaults to false on registration, so -// the install panel is what users see on the very first activation -// before startClient runs. -const LSP_READY_CONTEXT_KEY = "pipelineCheck.lspReady"; - -function setLspReady(ready: boolean): void { - void vscode.commands.executeCommand( - "setContext", - LSP_READY_CONTEXT_KEY, - ready, - ); -} - -const PIP_INSTALL_COMMAND = 'pip install "pipeline-check[lsp]"'; - -// Opens a new integrated terminal and types the pip install command -// without pressing Enter. The user reviews the command (and, when -// needed, activates their conda env / venv first) before running it. -// Auto-running here would install into whatever Python the shell's -// default `pip` points at — usually wrong when the user has a project -// venv they haven't activated yet. -function installInTerminal(): void { - const terminal = vscode.window.createTerminal("Pipeline-Check install"); - terminal.show(); - terminal.sendText(PIP_INSTALL_COMMAND, false); -} - function buildClient(): LanguageClient { const config = vscode.workspace.getConfiguration("pipelineCheck"); const command = config.get("serverCommand", "python"); @@ -419,20 +395,16 @@ export async function activate( // welcome panel — it opens a terminal with the pip command typed // but not executed, so the user reviews / activates their venv // first. copyInstallCommand stays registered as a fallback for - // users in headless / non-terminal flows. + // users in headless / non-terminal flows. Both bodies live in + // install.ts so the welcome-panel CTAs and the LSP-failure toast + // share one code path. vscode.commands.registerCommand( "pipelineCheck.installInTerminal", - () => installInTerminal(), + installInTerminal, ), vscode.commands.registerCommand( "pipelineCheck.copyInstallCommand", - async () => { - await vscode.env.clipboard.writeText(PIP_INSTALL_COMMAND); - vscode.window.setStatusBarMessage( - `Copied: ${PIP_INSTALL_COMMAND}`, - CONFIRM_TTL_MS, - ); - }, + copyInstallCommandToClipboard, ), vscode.commands.registerCommand("pipelineCheck.goToNextFinding", () => goToFinding("next"), diff --git a/src/install.ts b/src/install.ts new file mode 100644 index 0000000..7a6f2e8 --- /dev/null +++ b/src/install.ts @@ -0,0 +1,44 @@ +// LSP install helpers, factored out of extension.ts so the welcome- +// panel CTAs and the LSP-failure toast share one implementation — and +// so the behaviour is unit-testable without booting an extension host. + +import * as vscode from "vscode"; + +export const PIP_INSTALL_COMMAND = 'pip install "pipeline-check[lsp]"'; + +const TERMINAL_NAME = "Pipeline-Check install"; +const CONFIRM_TTL_MS = 2500; + +/** + * Open a new integrated terminal, type the pip install command, and + * focus the terminal — but do NOT press Enter. The user reviews the + * command (and activates their conda env / venv first when relevant) + * before running it. Auto-running here would install into whatever + * Python the shell's default `pip` points at — usually wrong when the + * user has a project venv they haven't activated yet. + * + * Pulled out as a module-level function (rather than an extension- + * internal closure) so the welcome-panel command, the LSP-failure + * toast, and the test suite all hit the same code path. + */ +export function installInTerminal(): vscode.Terminal { + const terminal = vscode.window.createTerminal(TERMINAL_NAME); + terminal.show(); + // The second argument to sendText is `addNewLine`; passing `false` + // suppresses the Enter press, which is the whole point. + terminal.sendText(PIP_INSTALL_COMMAND, false); + return terminal; +} + +/** + * Copy the pip install command to the clipboard and surface a short + * status-bar confirmation. Kept around as a fallback for headless + * flows where opening a terminal would be wrong. + */ +export async function copyInstallCommandToClipboard(): Promise { + await vscode.env.clipboard.writeText(PIP_INSTALL_COMMAND); + vscode.window.setStatusBarMessage( + `Copied: ${PIP_INSTALL_COMMAND}`, + CONFIRM_TTL_MS, + ); +} diff --git a/src/lspState.ts b/src/lspState.ts new file mode 100644 index 0000000..a685d89 --- /dev/null +++ b/src/lspState.ts @@ -0,0 +1,28 @@ +// Drives the `when` clauses on the two `viewsWelcome` entries in +// package.json — an install-prompt panel before the LSP comes up, a +// "Scan workspace" panel once it is connected. Pulled out of +// extension.ts so the state flip can be exercised in isolation by the +// unit suite. + +import * as vscode from "vscode"; + +export const LSP_READY_CONTEXT_KEY = "pipelineCheck.lspReady"; + +/** + * Set the `pipelineCheck.lspReady` context key. The two viewsWelcome + * entries — install-prompt vs scan-workspace — read from this key, so + * flipping it is the only signal the welcome panel needs to swap. + * + * Detached on purpose: `setContext` returns a Thenable, but every + * caller fires this as a side effect that the caller does not await. + * Using `void` keeps the call non-blocking without losing the + * returned promise's rejection handling (VS Code's setContext does + * not reject in practice). + */ +export function setLspReady(ready: boolean): void { + void vscode.commands.executeCommand( + "setContext", + LSP_READY_CONTEXT_KEY, + ready, + ); +} diff --git a/src/workspaceScan.ts b/src/workspaceScan.ts index 912e85d..4caee92 100644 --- a/src/workspaceScan.ts +++ b/src/workspaceScan.ts @@ -27,7 +27,11 @@ const EXCLUDE_GLOB = // found" even when workflows are present. One findFiles per pattern keeps // each glob shallow (at most one brace level for `.{yml,yaml}`) and the // result is deduped on the URI string. -async function findScannableFiles( +// +// Exported so the unit suite can pin the "one findFiles call per +// pattern, deduped on URI string" contract that prevents a future +// re-introduction of the nested-brace bug. +export async function findScannableFiles( patterns: readonly string[], exclude: string, ): Promise { From 50e1929019afb23c4ecd3e7feccf3d9d6dd57365 Mon Sep 17 00:00:00 2001 From: Daniel Martin <56157528+dmartinochoa@users.noreply.github.com> Date: Tue, 19 May 2026 17:28:15 +0200 Subject: [PATCH 2/4] test: expand coverage to 187 unit + integration tests (was 134) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backstop the recent fixes and add coverage for surfaces that had none. The most load-bearing additions: - findScannableFiles: per-pattern findFiles, dedupe-on-toString, first-seen order. Locks the fence against the nested-brace glob that produced "no scannable files found" returning. - scanWorkspace orchestration: no-workspace path, no-files path, quiet vs notification toasts, openTextDocument-failure counting, user cancellation. Previously only formatSummary had tests. - installInTerminal: opens a terminal, types pip command, asserts addNewLine=false so the user's unactivated venv isn't violated. copyInstallCommandToClipboard: clipboard write + status-bar confirmation TTL. The PIP_INSTALL_COMMAND literal is pinned. - setLspReady: exact setContext payload for both true and false transitions. The conditional viewsWelcome panel rides on this. - registerStatusBar visibility latch: hidden until either a CI candidate file or a pipeline-check diagnostic is observed; non- pipeline-check sources don't flip the latch. - manifest.test.ts: viewsWelcome shape (two gated entries, primary CTAs, no "Copy install command" button), every welcome-link command target declared, workspace-trust capability locked. - Integration: runs the actual pipelineCheck.scanWorkspace command against the fixture workspace so a real VS Code findFiles walks the glob — end-to-end regression fence for the nested-brace bug. Shared infra: the __testStubs__/vscode.ts module gains findFiles, createTerminal, createStatusBarItem, openTextDocument, clipboard, setStatusBarMessage, withProgress, configurable workspaceFolders, and a resetStubState() helper that beforeEach hooks use to keep each test's globalThis observations isolated. 134 → 187 vitest assertions; lint, typecheck, integration typecheck, and bundle all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/__testStubs__/vscode.ts | 303 ++++++++++++++++++++++-- src/install.test.ts | 98 ++++++++ src/lspState.test.ts | 69 ++++++ src/manifest.test.ts | 175 ++++++++++++++ src/statusBar.test.ts | 239 +++++++++++++------ src/test/integration/activation.test.ts | 80 +++++++ src/workspaceScan.test.ts | 231 +++++++++++++++++- 7 files changed, 1101 insertions(+), 94 deletions(-) create mode 100644 src/install.test.ts create mode 100644 src/lspState.test.ts create mode 100644 src/manifest.test.ts diff --git a/src/__testStubs__/vscode.ts b/src/__testStubs__/vscode.ts index f2834e7..e058cb8 100644 --- a/src/__testStubs__/vscode.ts +++ b/src/__testStubs__/vscode.ts @@ -11,9 +11,134 @@ // a fresh object per call keeps tests isolated — none of the classes // or stubs leak state between files. // -// `getDiagnostics` reads from `globalThis.__stubDiagnostics`, which -// tests populate via the per-file `setStubDiagnostics` helper they -// keep close to their fixtures. +// Tests drive behaviour through `globalThis.__stub*` slots and read +// observations back through `globalThis.__stubCalls`. Per-file +// `beforeEach` is expected to reset both. Each slot is documented +// inline below so a new test can find its hook without re-reading the +// whole stub. + +export interface StubFindFilesCall { + readonly include: string; + readonly exclude?: string; + readonly maxResults?: number; +} + +export interface StubTerminalCall { + readonly name: string; + readonly shown: boolean; + readonly sent: ReadonlyArray<{ readonly text: string; readonly addNewLine: boolean }>; +} + +export interface StubExecuteCommandCall { + readonly command: string; + readonly args: readonly unknown[]; +} + +export interface StubClipboardWrite { + readonly text: string; +} + +export interface StubStatusBarMessage { + readonly text: string; + readonly hideAfterMs?: number; +} + +export interface StubStatusBarItem { + text: string; + tooltip: unknown; + command: unknown; + name: string; + backgroundColor: unknown; + accessibilityInformation: { label: string } | undefined; + shown: boolean; + disposed: boolean; + // Observable counters help tests pin "did update() fire?" without + // racing against the implementation's debouncing. + showCount: number; + hideCount: number; +} + +interface StubCalls { + readonly findFiles: StubFindFilesCall[]; + readonly terminals: StubTerminalCall[]; + readonly executeCommand: StubExecuteCommandCall[]; + readonly clipboardWrites: StubClipboardWrite[]; + readonly statusBarMessages: StubStatusBarMessage[]; + readonly statusBarItems: StubStatusBarItem[]; + readonly infoMessages: string[]; + readonly warningMessages: string[]; + readonly errorMessages: string[]; +} + +declare global { + var __stubConfig: Record | undefined; + var __stubDiagnostics: + | Array<[{ toString: () => string }, unknown[]]> + | undefined; + // Findings of `findFiles`. Tests may either set a single `__stubFindFiles` + // (used for every include glob) or `__stubFindFilesByPattern` (a map + // from include glob → URI list, queried per call). + var __stubFindFiles: Array<{ toString: () => string; fsPath: string }> | undefined; + var __stubFindFilesByPattern: + | Record string; fsPath: string }>> + | undefined; + // What `workspace.workspaceFolders` should return for the duration + // of a test. `undefined` mimics "no workspace open". + var __stubWorkspaceFolders: + | Array<{ uri: { toString: () => string; fsPath: string } }> + | undefined; + // URI strings that `openTextDocument` should reject for (simulating + // a read error / unsupported encoding). All other URIs resolve. + var __stubOpenTextDocumentFailures: Set | undefined; + // When true, `withProgress` reports cancellation back to the task + // immediately. Used by the scanWorkspace cancellation test. + var __stubProgressCancelled: boolean | undefined; + var __stubCalls: StubCalls | undefined; +} + +function ensureCalls(): StubCalls { + if (!globalThis.__stubCalls) { + globalThis.__stubCalls = { + findFiles: [], + terminals: [], + executeCommand: [], + clipboardWrites: [], + statusBarMessages: [], + statusBarItems: [], + infoMessages: [], + warningMessages: [], + errorMessages: [], + }; + } + return globalThis.__stubCalls; +} + +/** + * Reset every observable slot in one place. Tests call this in + * `beforeEach` to keep their assertions isolated. The factory itself + * does not reset; calling `vscodeStub()` returns a fresh module shape + * but reuses the global slots so tests across files don't fight. + */ +export function resetStubState(): void { + globalThis.__stubConfig = {}; + globalThis.__stubDiagnostics = []; + globalThis.__stubFindFiles = undefined; + globalThis.__stubFindFilesByPattern = undefined; + globalThis.__stubWorkspaceFolders = undefined; + globalThis.__stubOpenTextDocumentFailures = undefined; + globalThis.__stubProgressCancelled = undefined; + globalThis.__stubCalls = { + findFiles: [], + terminals: [], + executeCommand: [], + clipboardWrites: [], + statusBarMessages: [], + statusBarItems: [], + infoMessages: [], + warningMessages: [], + errorMessages: [], + }; +} export function vscodeStub(): Record { class ThemeIcon { @@ -72,6 +197,7 @@ export function vscodeStub(): Record { }; const TreeItemCollapsibleState = { None: 0, Collapsed: 1, Expanded: 2 }; const StatusBarAlignment = { Left: 1, Right: 2 }; + const ProgressLocation = { SourceControl: 1, Window: 10, Notification: 15 }; class Range { constructor( @@ -102,12 +228,20 @@ export function vscodeStub(): Record { MarkdownString, TreeItemCollapsibleState, StatusBarAlignment, + ProgressLocation, Range, CodeLens, Uri, workspace: { asRelativePath: (uri: { fsPath?: string; path?: string }) => uri.fsPath ?? uri.path ?? "", + // Getter so a single per-test override (writing to + // `globalThis.__stubWorkspaceFolders`) reaches the consumer + // without each test having to mutate the `vscode.workspace` + // module reference. + get workspaceFolders() { + return globalThis.__stubWorkspaceFolders; + }, // `getConfiguration(section).get(key, fallback)` reads from // `globalThis.__stubConfig`, a `Record` keyed // by `
.` (or just `` if no section was @@ -115,30 +249,49 @@ export function vscodeStub(): Record { // test's expectations are isolated. getConfiguration: (section?: string) => ({ get: (key: string, fallback?: T): T => { - const store = - (globalThis as { __stubConfig?: Record }) - .__stubConfig ?? {}; + const store = globalThis.__stubConfig ?? {}; const fullKey = section ? `${section}.${key}` : key; if (fullKey in store) return store[fullKey] as T; return fallback as T; }, }), onDidChangeConfiguration: () => ({ dispose: () => undefined }), + onDidSaveTextDocument: () => ({ dispose: () => undefined }), + // Resolves with whatever the test stashed on + // `globalThis.__stubFindFiles` (a flat URI list reused for every + // include glob) or `globalThis.__stubFindFilesByPattern[include]` + // (when a per-pattern map is set, more useful for asserting that + // each pattern is queried separately). Every call is captured on + // `globalThis.__stubCalls.findFiles` so a test can assert on the + // include/exclude/maxResults the caller passed. + findFiles: (include: string, exclude?: string, maxResults?: number) => { + const calls = ensureCalls(); + calls.findFiles.push({ include, exclude, maxResults }); + const byPattern = globalThis.__stubFindFilesByPattern; + if (byPattern) { + return Promise.resolve(byPattern[include] ?? []); + } + const flat = globalThis.__stubFindFiles; + return Promise.resolve(flat ?? []); + }, + // Resolves with a minimal TextDocument-shaped object for any + // URI not listed in `__stubOpenTextDocumentFailures`, where it + // rejects instead. scanWorkspace counts those rejections as + // `failed` without aborting the rest of the scan. + openTextDocument: (uri: { toString: () => string }) => { + const key = uri.toString(); + if (globalThis.__stubOpenTextDocumentFailures?.has(key)) { + return Promise.reject(new Error(`stub: open failed for ${key}`)); + } + return Promise.resolve({ uri }); + }, }, languages: { // Two call shapes: // - `getDiagnostics()` returns every [uri, diagnostic[]] pair // - `getDiagnostics(uri)` returns just that uri's diagnostics getDiagnostics: (uri?: { toString: () => string }) => { - const all = - ( - globalThis as { - __stubDiagnostics?: Array<[ - { toString: () => string }, - unknown[], - ]>; - } - ).__stubDiagnostics ?? []; + const all = globalThis.__stubDiagnostics ?? []; if (uri === undefined) return all; const key = uri.toString(); const match = all.find(([u]) => u.toString() === key); @@ -146,7 +299,123 @@ export function vscodeStub(): Record { }, onDidChangeDiagnostics: () => ({ dispose: () => undefined }), }, - commands: { executeCommand: () => Promise.resolve() }, - window: {}, + commands: { + // Captures every executeCommand invocation on the shared + // `__stubCalls.executeCommand` slot. Tests assert on the call + // history (e.g. setContext / pipelineCheck.lspReady / true). + executeCommand: (command: string, ...args: unknown[]) => { + ensureCalls().executeCommand.push({ command, args }); + return Promise.resolve(); + }, + registerCommand: () => ({ dispose: () => undefined }), + }, + env: { + clipboard: { + writeText: (text: string) => { + ensureCalls().clipboardWrites.push({ text }); + return Promise.resolve(); + }, + }, + openExternal: () => Promise.resolve(true), + }, + window: { + // Terminal factory captures the name and returns a stub whose + // show/sendText calls land on the shared slot. Each call returns + // a fresh terminal with its own observation buffer. + createTerminal: (name: string) => { + const sent: Array<{ text: string; addNewLine: boolean }> = []; + const record = { + name, + shown: false, + sent, + }; + ensureCalls().terminals.push(record); + return { + name, + show: () => { + // Mutate the captured record so tests see `shown: true` + // without having to drill into a closure. + (record as { shown: boolean }).shown = true; + }, + sendText: (text: string, addNewLine?: boolean) => { + sent.push({ text, addNewLine: addNewLine ?? true }); + }, + dispose: () => undefined, + }; + }, + createStatusBarItem: ( + _alignment?: number, + _priority?: number, + ): StubStatusBarItem & { + show: () => void; + hide: () => void; + dispose: () => void; + } => { + const item: StubStatusBarItem & { + show: () => void; + hide: () => void; + dispose: () => void; + } = { + text: "", + tooltip: undefined, + command: undefined, + name: "", + backgroundColor: undefined, + accessibilityInformation: undefined, + shown: false, + disposed: false, + showCount: 0, + hideCount: 0, + show() { + this.shown = true; + this.showCount += 1; + }, + hide() { + this.shown = false; + this.hideCount += 1; + }, + dispose() { + this.disposed = true; + }, + }; + ensureCalls().statusBarItems.push(item); + return item; + }, + setStatusBarMessage: (text: string, hideAfterMs?: number) => { + ensureCalls().statusBarMessages.push({ text, hideAfterMs }); + return { dispose: () => undefined }; + }, + showInformationMessage: (message: string) => { + ensureCalls().infoMessages.push(message); + return Promise.resolve(undefined); + }, + showWarningMessage: (message: string) => { + ensureCalls().warningMessages.push(message); + return Promise.resolve(undefined); + }, + showErrorMessage: (message: string) => { + ensureCalls().errorMessages.push(message); + return Promise.resolve(undefined); + }, + // Progress UI: invokes the task immediately with a no-op + // `progress` reporter and a never-cancelled token. Good enough + // to drive scanWorkspace's loop in a unit test without mocking + // out the full Progress API surface. + withProgress: async ( + _options: unknown, + task: ( + progress: { report: (value: unknown) => void }, + token: { isCancellationRequested: boolean }, + ) => Thenable, + ): Promise => { + const progress = { report: () => undefined }; + const token = { + get isCancellationRequested() { + return globalThis.__stubProgressCancelled === true; + }, + }; + return task(progress, token); + }, + }, }; } diff --git a/src/install.test.ts b/src/install.test.ts new file mode 100644 index 0000000..9d527aa --- /dev/null +++ b/src/install.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("vscode", async () => { + const { vscodeStub } = await import("./__testStubs__/vscode"); + return vscodeStub(); +}); + +import { resetStubState } from "./__testStubs__/vscode"; +import { + PIP_INSTALL_COMMAND, + copyInstallCommandToClipboard, + installInTerminal, +} from "./install"; + +beforeEach(() => { + resetStubState(); +}); + +describe("PIP_INSTALL_COMMAND", () => { + // The literal string is the contract — UI surfaces (welcome panel, + // failure toast, copy command) and the terminal-install path all + // depend on it being exactly `pip install "pipeline-check[lsp]"`. + // Test failures here are intentional friction: an accidental rename + // (or missing quoting around the extra) should fail loudly. + it("is the canonical PyPI install line, with the [lsp] extra quoted", () => { + expect(PIP_INSTALL_COMMAND).toBe('pip install "pipeline-check[lsp]"'); + }); +}); + +describe("installInTerminal", () => { + it("creates a new terminal with the Pipeline-Check name", () => { + installInTerminal(); + const terminals = globalThis.__stubCalls?.terminals ?? []; + expect(terminals).toHaveLength(1); + expect(terminals[0].name).toBe("Pipeline-Check install"); + }); + + it("focuses the new terminal so the command is visible", () => { + installInTerminal(); + const terminals = globalThis.__stubCalls?.terminals ?? []; + expect(terminals[0].shown).toBe(true); + }); + + it("types the pip command without auto-executing (addNewLine=false)", () => { + // This is the load-bearing invariant: a user with an unactivated + // venv must see the command, NOT run it. Asserting addNewLine === + // false pins the safer behaviour. + installInTerminal(); + const t = globalThis.__stubCalls!.terminals[0]; + expect(t.sent).toHaveLength(1); + expect(t.sent[0].text).toBe(PIP_INSTALL_COMMAND); + expect(t.sent[0].addNewLine).toBe(false); + }); + + it("never sends a second line — one command, end of story", () => { + installInTerminal(); + const t = globalThis.__stubCalls!.terminals[0]; + expect(t.sent).toHaveLength(1); + }); + + it("does not write to the clipboard (separate code path)", () => { + installInTerminal(); + expect(globalThis.__stubCalls?.clipboardWrites).toEqual([]); + }); +}); + +describe("copyInstallCommandToClipboard", () => { + it("writes exactly the pip command to the clipboard", async () => { + await copyInstallCommandToClipboard(); + expect(globalThis.__stubCalls?.clipboardWrites).toEqual([ + { text: PIP_INSTALL_COMMAND }, + ]); + }); + + it("surfaces a status-bar confirmation so the silent copy is visible", async () => { + await copyInstallCommandToClipboard(); + const messages = globalThis.__stubCalls?.statusBarMessages ?? []; + expect(messages).toHaveLength(1); + expect(messages[0].text).toContain(PIP_INSTALL_COMMAND); + expect(messages[0].text.toLowerCase()).toContain("copied"); + }); + + it("expires the status-bar confirmation after a short TTL (a few seconds)", async () => { + // The TTL is a UX detail (long enough to read, short enough not + // to stick around). Bound the assertion loosely — anything between + // a perceptible flash (1s) and an over-stay (10s) is fine. + await copyInstallCommandToClipboard(); + const ttl = + globalThis.__stubCalls?.statusBarMessages[0]?.hideAfterMs ?? 0; + expect(ttl).toBeGreaterThanOrEqual(1000); + expect(ttl).toBeLessThanOrEqual(10000); + }); + + it("never opens a terminal (separate code path)", async () => { + await copyInstallCommandToClipboard(); + expect(globalThis.__stubCalls?.terminals).toEqual([]); + }); +}); diff --git a/src/lspState.test.ts b/src/lspState.test.ts new file mode 100644 index 0000000..ed936e3 --- /dev/null +++ b/src/lspState.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("vscode", async () => { + const { vscodeStub } = await import("./__testStubs__/vscode"); + return vscodeStub(); +}); + +import { resetStubState } from "./__testStubs__/vscode"; +import { LSP_READY_CONTEXT_KEY, setLspReady } from "./lspState"; + +beforeEach(() => { + resetStubState(); +}); + +describe("setLspReady", () => { + // The two viewsWelcome entries in package.json read + // `pipelineCheck.lspReady` via `when` clauses. setLspReady is the + // ONLY writer for that key. These tests pin the contract that the + // value reaches VS Code's `setContext` channel verbatim — a missed + // flip means the wrong welcome panel renders. + + it("uses the documented context key name", () => { + // Hard-coded literal here mirrors the `when` clauses in + // package.json: a manifest rename without updating this module + // (or vice versa) silently breaks the welcome panel. + expect(LSP_READY_CONTEXT_KEY).toBe("pipelineCheck.lspReady"); + }); + + it("propagates `true` to setContext for the ready state", () => { + setLspReady(true); + const calls = globalThis.__stubCalls?.executeCommand ?? []; + expect(calls).toEqual([ + { + command: "setContext", + args: [LSP_READY_CONTEXT_KEY, true], + }, + ]); + }); + + it("propagates `false` to setContext for the not-ready state", () => { + setLspReady(false); + const calls = globalThis.__stubCalls?.executeCommand ?? []; + expect(calls).toEqual([ + { + command: "setContext", + args: [LSP_READY_CONTEXT_KEY, false], + }, + ]); + }); + + it("fires once per call so repeated transitions stay observable", () => { + setLspReady(false); + setLspReady(true); + setLspReady(false); + const calls = globalThis.__stubCalls?.executeCommand ?? []; + expect(calls.map((c) => c.args[1])).toEqual([false, true, false]); + }); + + it("does not invoke any other VS Code command", () => { + // Defensive: a future refactor that bundles setContext with a + // toast / showWelcome / focus call would silently change the UX. + // Pinning the call to ONLY setContext catches that drift. + setLspReady(true); + const calls = globalThis.__stubCalls?.executeCommand ?? []; + for (const c of calls) { + expect(c.command).toBe("setContext"); + } + }); +}); diff --git a/src/manifest.test.ts b/src/manifest.test.ts new file mode 100644 index 0000000..01967d0 --- /dev/null +++ b/src/manifest.test.ts @@ -0,0 +1,175 @@ +import { describe, it, expect, vi } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; + +// No vscode runtime touched here — the manifest is pure JSON. +vi.mock("vscode", () => ({})); + +import { LSP_READY_CONTEXT_KEY } from "./lspState"; + +interface ManifestViewsWelcome { + readonly view: string; + readonly contents: string; + readonly when?: string; +} + +interface ManifestCommand { + readonly command: string; + readonly title: string; + readonly category?: string; +} + +interface Manifest { + readonly contributes: { + readonly viewsWelcome: ManifestViewsWelcome[]; + readonly commands: ManifestCommand[]; + }; + readonly activationEvents: string[]; + readonly capabilities?: { + readonly untrustedWorkspaces?: { readonly supported: string }; + readonly virtualWorkspaces?: boolean; + }; +} + +const manifest = JSON.parse( + readFileSync(resolve(__dirname, "..", "package.json"), "utf8"), +) as Manifest; + +const welcome = manifest.contributes.viewsWelcome; + +describe("viewsWelcome — conditional install/scan panels", () => { + // The findings panel ships two welcome entries — one for the pre- + // LSP-ready state (install prompt), one for the ready state (scan + // workspace). The user complaint that prompted this rework was a + // single dense panel that mixed install instructions with feature + // copy. These tests pin the structure so a future edit that + // collapses the two states back together fails loudly. + + it("contributes exactly two entries on the findings view", () => { + const onFindings = welcome.filter( + (w) => w.view === "pipelineCheck.findings", + ); + expect(onFindings).toHaveLength(2); + }); + + it("gates one entry behind the LSP-ready context key (positive case)", () => { + const ready = welcome.find( + (w) => w.when === LSP_READY_CONTEXT_KEY, + ); + expect(ready, "ready-state welcome entry missing").toBeDefined(); + }); + + it("gates the other entry on the negation (install-prompt case)", () => { + const notReady = welcome.find( + (w) => w.when === `!${LSP_READY_CONTEXT_KEY}`, + ); + expect(notReady, "install-prompt welcome entry missing").toBeDefined(); + }); + + it("ready entry promotes 'Scan workspace' as the primary CTA", () => { + // A button-styled link is a markdown link alone on its line. The + // contents string uses literal \n separators, so the regex below + // matches the line shape directly. + const ready = welcome.find((w) => w.when === LSP_READY_CONTEXT_KEY); + expect(ready?.contents).toMatch( + /^\[Scan workspace\]\(command:pipelineCheck\.scanWorkspace\)$/m, + ); + }); + + it("install-prompt entry exposes 'Install in terminal' as the primary CTA", () => { + const notReady = welcome.find( + (w) => w.when === `!${LSP_READY_CONTEXT_KEY}`, + ); + expect(notReady?.contents).toMatch( + /^\[Install in terminal\]\(command:pipelineCheck\.installInTerminal\)$/m, + ); + }); + + it("install-prompt entry offers 'Retry connection' as a secondary CTA", () => { + const notReady = welcome.find( + (w) => w.when === `!${LSP_READY_CONTEXT_KEY}`, + ); + expect(notReady?.contents).toMatch( + /^\[Retry connection\]\(command:pipelineCheck\.restart\)$/m, + ); + }); + + it("install-prompt entry references pipeline-check[lsp] so users know what to install", () => { + const notReady = welcome.find( + (w) => w.when === `!${LSP_READY_CONTEXT_KEY}`, + ); + expect(notReady?.contents).toContain("pipeline-check[lsp]"); + }); + + it("ready entry tells users about the keyboard navigation shortcuts", () => { + // Alt+F8 / Shift+Alt+F8 is the navigation surface; users + // typically discover it through this welcome screen. Pinning the + // text guards against a regression that strips the discoverability. + const ready = welcome.find((w) => w.when === LSP_READY_CONTEXT_KEY); + expect(ready?.contents).toContain("Alt+F8"); + expect(ready?.contents).toContain("Shift+Alt+F8"); + }); + + it("neither entry surfaces 'Copy install command' as a primary button (the rejected UX)", () => { + // Copy-install-command is still registered for headless flows but + // must NOT appear as a top-level button in either welcome state. + // The rework was specifically about this CTA being out of place. + for (const w of welcome.filter( + (e) => e.view === "pipelineCheck.findings", + )) { + expect(w.contents).not.toMatch( + /^\[Copy install command\]/m, + ); + } + }); +}); + +describe("commands — install paths registered", () => { + // The welcome panel references both `installInTerminal` and + // `copyInstallCommand`. These tests guard against a manifest edit + // that removes a command the welcome panel still tries to invoke. + + const commands = new Set( + manifest.contributes.commands.map((c) => c.command), + ); + + it("declares pipelineCheck.installInTerminal", () => { + expect(commands.has("pipelineCheck.installInTerminal")).toBe(true); + }); + + it("declares pipelineCheck.copyInstallCommand", () => { + expect(commands.has("pipelineCheck.copyInstallCommand")).toBe(true); + }); + + it("declares every command the welcome panels link to", () => { + // Extract every `command:pipelineCheck.…` link target from the + // welcome contents and confirm each one is a declared command. + for (const w of welcome.filter( + (e) => e.view === "pipelineCheck.findings", + )) { + const targets = [...w.contents.matchAll(/command:(pipelineCheck\.[A-Za-z]+)/g)] + .map((m) => m[1]); + for (const target of targets) { + expect( + commands.has(target), + `welcome panel links to ${target} but it is not in contributes.commands`, + ).toBe(true); + } + } + }); +}); + +describe("capabilities — locked-down workspace trust", () => { + // Pipeline-Check spawns a Python child process; this MUST stay + // declared as 'limited' for untrusted workspaces so VS Code's + // workspace-trust gate kicks in for the process-spawning settings. + it("declares untrustedWorkspaces.supported = 'limited'", () => { + expect(manifest.capabilities?.untrustedWorkspaces?.supported).toBe( + "limited", + ); + }); + + it("declares virtualWorkspaces = false", () => { + expect(manifest.capabilities?.virtualWorkspaces).toBe(false); + }); +}); diff --git a/src/statusBar.test.ts b/src/statusBar.test.ts index 261184e..374ab48 100644 --- a/src/statusBar.test.ts +++ b/src/statusBar.test.ts @@ -1,28 +1,22 @@ -import { describe, it, expect, vi } from "vitest"; - -// statusBar.ts imports `vscode` for the runtime wiring; the pure -// helpers (formatStatusBarText, formatStatusBarTooltip, -// countDiagnostics, pickBackgroundColor) don't touch it but the -// module-level import has to resolve. Tiny stub covers it; ThemeColor -// is a class so `new vscode.ThemeColor(id)` works and tests can read -// `.id` off the result. -vi.mock("vscode", () => { - class ThemeColor { - constructor(public readonly id: string) {} - } - return { - ThemeColor, - StatusBarAlignment: { Left: 1, Right: 2 }, - window: {}, - languages: {}, - }; +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Use the shared stub so the registerStatusBar tests below have a +// real-shaped `vscode.window.createStatusBarItem`, `workspace.findFiles`, +// and `languages.onDidChangeDiagnostics`. The pure-helper tests don't +// need any of that, but a single mock keeps the file consistent. +vi.mock("vscode", async () => { + const { vscodeStub } = await import("./__testStubs__/vscode"); + return vscodeStub(); }); +import { resetStubState } from "./__testStubs__/vscode"; import { countDiagnostics, formatStatusBarAccessibilityLabel, formatStatusBarText, formatStatusBarTooltip, + pickBackgroundColor, + registerStatusBar, } from "./statusBar"; // Helpers @@ -37,6 +31,16 @@ const make = (sev?: string) => ({ data: sev ? { severity: sev } : undefined, }); +// URI factory shaped like `vscode.Uri` for the stub's purposes. +const uri = (path: string) => ({ + toString: () => `file://${path}`, + fsPath: path, +}); + +beforeEach(() => { + resetStubState(); +}); + describe("formatStatusBarText", () => { it("returns 'clean' when there are no findings", () => { expect( @@ -104,6 +108,29 @@ describe("formatStatusBarTooltip", () => { formatStatusBarTooltip({ CRITICAL: 0, HIGH: 1, MEDIUM: 0, LOW: 0, INFO: 0 }), ).toContain("1 finding"); }); + + it("teaches the Alt+F8 keyboard shortcut on the trailing line", () => { + const tip = formatStatusBarTooltip({ + CRITICAL: 1, + HIGH: 0, + MEDIUM: 0, + LOW: 0, + INFO: 0, + }); + expect(tip).toContain("Alt+F8"); + expect(tip).toContain("Shift+Alt+F8"); + }); + + it("does not include the keyboard hint when there are no findings", () => { + const tip = formatStatusBarTooltip({ + CRITICAL: 0, + HIGH: 0, + MEDIUM: 0, + LOW: 0, + INFO: 0, + }); + expect(tip).not.toContain("Alt+F8"); + }); }); describe("countDiagnostics", () => { @@ -202,83 +229,43 @@ describe("formatStatusBarAccessibilityLabel", () => { }); }); -describe("formatStatusBarTooltip", () => { - it("teaches the Alt+F8 keyboard shortcut on the trailing line", () => { - const tip = formatStatusBarTooltip({ - CRITICAL: 1, - HIGH: 0, - MEDIUM: 0, - LOW: 0, - INFO: 0, - }); - expect(tip).toContain("Alt+F8"); - expect(tip).toContain("Shift+Alt+F8"); - }); - - it("does not include the keyboard hint when there are no findings", () => { - const tip = formatStatusBarTooltip({ - CRITICAL: 0, - HIGH: 0, - MEDIUM: 0, - LOW: 0, - INFO: 0, - }); - expect(tip).not.toContain("Alt+F8"); - }); -}); - describe("pickBackgroundColor", () => { - // The stub vscode module returns `{ id }` as the ThemeColor — the - // tests check the colour by id rather than relying on identity. - // We need ThemeColor to be available in the stub for this test; - // statusBar.test.ts's existing minimal stub doesn't include it. - // Below we re-import the function through the same minimal stub - // (vi.mock at the top of this file maps `vscode` to the inline - // object), so we read .id off whatever shape it returns. - - // Pull the function lazily so the vi.mock at the top is already in - // place when it resolves. - async function pick(c: import("./statusBar").SeverityCounts) { - const mod = await import("./statusBar"); - return mod.pickBackgroundColor(c); - } - - it("returns the error-background token when CRITICAL is present", async () => { - const bg = (await pick({ + it("returns the error-background token when CRITICAL is present", () => { + const bg = pickBackgroundColor({ CRITICAL: 1, HIGH: 0, MEDIUM: 0, LOW: 0, INFO: 0, - })) as { id: string } | undefined; + }) as { id: string } | undefined; expect(bg?.id).toBe("statusBarItem.errorBackground"); }); - it("CRITICAL outranks HIGH for the colour choice", async () => { - const bg = (await pick({ + it("CRITICAL outranks HIGH for the colour choice", () => { + const bg = pickBackgroundColor({ CRITICAL: 1, HIGH: 5, MEDIUM: 0, LOW: 0, INFO: 0, - })) as { id: string } | undefined; + }) as { id: string } | undefined; expect(bg?.id).toBe("statusBarItem.errorBackground"); }); - it("returns the warning-background token when HIGH (but no CRITICAL) is present", async () => { - const bg = (await pick({ + it("returns the warning-background token when HIGH (but no CRITICAL) is present", () => { + const bg = pickBackgroundColor({ CRITICAL: 0, HIGH: 3, MEDIUM: 0, LOW: 0, INFO: 0, - })) as { id: string } | undefined; + }) as { id: string } | undefined; expect(bg?.id).toBe("statusBarItem.warningBackground"); }); - it("returns undefined when only MEDIUM / LOW / INFO are present", async () => { + it("returns undefined when only MEDIUM / LOW / INFO are present", () => { expect( - await pick({ + pickBackgroundColor({ CRITICAL: 0, HIGH: 0, MEDIUM: 4, @@ -288,9 +275,9 @@ describe("pickBackgroundColor", () => { ).toBeUndefined(); }); - it("returns undefined on a clean workspace", async () => { + it("returns undefined on a clean workspace", () => { expect( - await pick({ + pickBackgroundColor({ CRITICAL: 0, HIGH: 0, MEDIUM: 0, @@ -300,3 +287,113 @@ describe("pickBackgroundColor", () => { ).toBeUndefined(); }); }); + +describe("registerStatusBar — visibility latch", () => { + // The latch keeps the item hidden until the workspace looks + // CI-relevant (at least one matching file OR at least one published + // diagnostic). Once "seen" as relevant, the item stays visible even + // through clean (zero) publishes — the "clean" signal earns its + // keep. These tests pin both halves of that policy. + + const ctx = { + subscriptions: [] as Array<{ dispose?: () => void }>, + } as unknown as import("vscode").ExtensionContext; + + // Helper: read the most recently created stub status bar item. + function lastItem() { + const items = globalThis.__stubCalls?.statusBarItems ?? []; + return items[items.length - 1]; + } + + // Helper: yield to the microtask queue so the deferred findFiles + // promise inside registerStatusBar settles before we assert. + const tick = () => new Promise((resolve) => setImmediate(resolve)); + + it("starts hidden when the workspace has neither CI files nor diagnostics", async () => { + globalThis.__stubFindFiles = []; + globalThis.__stubDiagnostics = []; + registerStatusBar(ctx); + await tick(); + expect(lastItem().shown).toBe(false); + }); + + it("shows itself once findFiles reports at least one CI candidate", async () => { + globalThis.__stubFindFiles = [uri("/repo/.github/workflows/ci.yml")]; + globalThis.__stubDiagnostics = []; + registerStatusBar(ctx); + await tick(); + expect(lastItem().shown).toBe(true); + }); + + it("shows itself when at least one pipeline-check diagnostic is already published", async () => { + // Workspace has no candidate file (e.g. an untitled buffer that + // somehow carries findings); a published diagnostic still + // qualifies us as relevant. + globalThis.__stubFindFiles = []; + globalThis.__stubDiagnostics = [[uri("/r/a.yml"), [make("HIGH")]]]; + registerStatusBar(ctx); + await tick(); + expect(lastItem().shown).toBe(true); + }); + + it("ignores non-pipeline-check diagnostics for the latch (eslint shouldn't show us)", async () => { + globalThis.__stubFindFiles = []; + globalThis.__stubDiagnostics = [ + [ + uri("/r/a.yml"), + [{ ...make("HIGH"), source: "eslint" }], + ], + ]; + registerStatusBar(ctx); + await tick(); + expect(lastItem().shown).toBe(false); + }); + + it("paints text/tooltip/accessibility/background colour from the initial counts", async () => { + globalThis.__stubFindFiles = [uri("/repo/Dockerfile")]; + globalThis.__stubDiagnostics = [ + [uri("/r/a.yml"), [make("CRITICAL"), make("HIGH")]], + ]; + registerStatusBar(ctx); + await tick(); + const item = lastItem(); + expect(item.text).toContain("$(shield)"); + expect(item.text).toContain("1C"); + expect(item.text).toContain("1H"); + expect((item.tooltip as string).toLowerCase()).toContain("pipeline-check"); + expect(item.accessibilityInformation?.label).toContain("1 critical"); + expect((item.backgroundColor as { id: string })?.id).toBe( + "statusBarItem.errorBackground", + ); + }); + + it("wires the click target to the Findings panel focus command", async () => { + globalThis.__stubFindFiles = []; + globalThis.__stubDiagnostics = []; + registerStatusBar(ctx); + await tick(); + expect(lastItem().command).toBe("pipelineCheck.findings.focus"); + }); + + it("uses the documented 'Pipeline-Check' menu name", async () => { + globalThis.__stubFindFiles = []; + globalThis.__stubDiagnostics = []; + registerStatusBar(ctx); + await tick(); + expect(lastItem().name).toBe("Pipeline-Check"); + }); + + it("pushes the status bar item onto context.subscriptions for cleanup", async () => { + globalThis.__stubFindFiles = []; + globalThis.__stubDiagnostics = []; + const subs: Array<{ dispose?: () => void }> = []; + const localCtx = { + subscriptions: subs, + } as unknown as import("vscode").ExtensionContext; + registerStatusBar(localCtx); + await tick(); + // The returned item itself + the onDidChangeDiagnostics disposable + // both land here. The item should be one of them. + expect(subs.length).toBeGreaterThanOrEqual(1); + }); +}); diff --git a/src/test/integration/activation.test.ts b/src/test/integration/activation.test.ts index 711808d..533750b 100644 --- a/src/test/integration/activation.test.ts +++ b/src/test/integration/activation.test.ts @@ -108,4 +108,84 @@ suite("Pipeline-Check — activation", () => { "virtualWorkspaces should be false (extension spawns a child process)", ); }); + + test("scanWorkspace finds the fixture's workflow file in a real workspace", async function () { + // Regression fence for the nested-brace findFiles bug that + // produced "no scannable files found" even with workflows present. + // The unit suite pins findScannableFiles' shape; this test runs + // the whole command against a real VS Code findFiles to confirm + // the actual glob resolution finds the fixture. + this.timeout(15000); + const ext = vscode.extensions.getExtension(EXTENSION_ID); + assert(ext); + await ext.activate(); + + // The workspace is `test-fixtures/sample-workflow/` (set in + // .vscode-test.mjs). It contains `.github/workflows/release.yml`. + // Verify findFiles independently — same call shape scanWorkspace + // uses internally — and confirm at least one workflow shows up. + const found = await vscode.workspace.findFiles( + "**/.github/workflows/*.{yml,yaml}", + "**/{node_modules,.git}/**", + ); + assert.ok( + found.some((u) => u.fsPath.endsWith("release.yml")), + `findFiles missed the fixture workflow: ${found.map((u) => u.fsPath).join(", ")}`, + ); + + // The command itself should resolve without throwing. The LSP + // child may not be running in CI (Python is not necessarily on + // PATH); the scan walk is independent of the LSP — it just opens + // documents so the (running or not) server sees them. The + // assertion is the negative one: no exception. + await vscode.commands.executeCommand("pipelineCheck.scanWorkspace"); + }); + + test("workspace-trust capability blocks workspace-overridable settings without an explicit prompt", () => { + // serverCommand and serverArgs spawn a child process; the manifest + // declares both as `machine-overridable` so a workspace cannot + // silently override them. Pinned here so a future schema edit + // that demotes their scope can't slip through review. + const ext = vscode.extensions.getExtension(EXTENSION_ID); + assert(ext); + const props = ext.packageJSON.contributes?.configuration?.properties as + | Record + | undefined; + assert.ok(props, "configuration.properties missing from package.json"); + assert.strictEqual( + props["pipelineCheck.serverCommand"]?.scope, + "machine-overridable", + "pipelineCheck.serverCommand must stay machine-overridable", + ); + assert.strictEqual( + props["pipelineCheck.serverArgs"]?.scope, + "machine-overridable", + "pipelineCheck.serverArgs must stay machine-overridable", + ); + }); + + test("viewsWelcome contributes both install-prompt and scan-ready entries", () => { + const ext = vscode.extensions.getExtension(EXTENSION_ID); + assert(ext); + const welcome = ext.packageJSON.contributes?.viewsWelcome as + | Array<{ view: string; when?: string }> + | undefined; + assert.ok(welcome, "viewsWelcome block missing from package.json"); + const onFindings = welcome.filter( + (w) => w.view === "pipelineCheck.findings", + ); + assert.strictEqual( + onFindings.length, + 2, + "findings view should have two viewsWelcome entries (ready + not-ready)", + ); + assert.ok( + onFindings.some((w) => w.when === "pipelineCheck.lspReady"), + "missing ready-state welcome entry", + ); + assert.ok( + onFindings.some((w) => w.when === "!pipelineCheck.lspReady"), + "missing install-prompt welcome entry", + ); + }); }); diff --git a/src/workspaceScan.test.ts b/src/workspaceScan.test.ts index 4da1f9b..ccf0a8d 100644 --- a/src/workspaceScan.test.ts +++ b/src/workspaceScan.test.ts @@ -1,16 +1,30 @@ -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("vscode", async () => { const { vscodeStub } = await import("./__testStubs__/vscode"); return vscodeStub(); }); -import { formatSummary } from "./workspaceScan"; +import { resetStubState } from "./__testStubs__/vscode"; +import { + findScannableFiles, + formatSummary, + scanWorkspace, +} from "./workspaceScan"; -// Pure-logic surface of workspaceScan.ts. The async scan itself is -// hard to unit-test without the VS Code host (findFiles + progress + -// openTextDocument). Integration coverage of the file-discovery path -// lives in src/test/integration/activation.test.ts. +beforeEach(() => { + resetStubState(); +}); + +// Small URI factory matching the shape findFiles returns and +// scanWorkspace consumes — `toString()` for dedupe, `fsPath` for +// progress messages. +function fakeUri(path: string) { + return { + toString: () => `file://${path}`, + fsPath: path, + }; +} describe("formatSummary", () => { it("clean run: 'scanned N files'", () => { @@ -43,3 +57,208 @@ describe("formatSummary", () => { ); }); }); + +describe("findScannableFiles", () => { + // This is the regression fence for the nested-brace findFiles bug. + // The "no scannable files found" symptom returned when a single + // combined glob with nested braces silently matched nothing. + + it("queries findFiles once per pattern (NOT one nested-brace glob)", async () => { + // If a future refactor goes back to `{a,b,c}` we get zero matches + // again. This assertion locks the per-pattern shape. + globalThis.__stubFindFiles = []; + await findScannableFiles( + ["**/.github/workflows/*.{yml,yaml}", "**/.gitlab-ci.yml", "**/Dockerfile"], + "**/node_modules/**", + ); + const calls = globalThis.__stubCalls?.findFiles ?? []; + expect(calls.map((c) => c.include)).toEqual([ + "**/.github/workflows/*.{yml,yaml}", + "**/.gitlab-ci.yml", + "**/Dockerfile", + ]); + }); + + it("passes the exclude glob through verbatim on every call", async () => { + globalThis.__stubFindFiles = []; + const exclude = "**/{node_modules,.git}/**"; + await findScannableFiles(["**/a", "**/b"], exclude); + const calls = globalThis.__stubCalls?.findFiles ?? []; + expect(calls).toHaveLength(2); + for (const c of calls) { + expect(c.exclude).toBe(exclude); + } + }); + + it("returns the union of per-pattern matches", async () => { + globalThis.__stubFindFilesByPattern = { + "**/.github/workflows/*.{yml,yaml}": [fakeUri("/r/.github/workflows/ci.yml")], + "**/.gitlab-ci.yml": [fakeUri("/r/.gitlab-ci.yml")], + "**/Dockerfile": [fakeUri("/r/Dockerfile")], + }; + const out = await findScannableFiles( + [ + "**/.github/workflows/*.{yml,yaml}", + "**/.gitlab-ci.yml", + "**/Dockerfile", + ], + "", + ); + expect(out.map((u) => u.fsPath).sort()).toEqual([ + "/r/.github/workflows/ci.yml", + "/r/.gitlab-ci.yml", + "/r/Dockerfile", + ]); + }); + + it("dedupes URIs that match more than one pattern", async () => { + // A Containerfile pattern could plausibly overlap with a future + // glob; the dedupe contract has to hold or progress reporting + // double-counts and openTextDocument fires twice on the same file. + const dup = fakeUri("/r/Dockerfile"); + globalThis.__stubFindFilesByPattern = { + "**/Dockerfile": [dup], + "**/Containerfile": [dup], + }; + const out = await findScannableFiles( + ["**/Dockerfile", "**/Containerfile"], + "", + ); + expect(out).toHaveLength(1); + expect(out[0].fsPath).toBe("/r/Dockerfile"); + }); + + it("preserves first-seen order across deduped patterns", async () => { + const a = fakeUri("/r/a.yml"); + const b = fakeUri("/r/b.yml"); + const c = fakeUri("/r/c.yml"); + globalThis.__stubFindFilesByPattern = { + "**/p1": [a, b], + "**/p2": [b, c], // b overlaps; should NOT move to position after c. + }; + const out = await findScannableFiles(["**/p1", "**/p2"], ""); + expect(out.map((u) => u.fsPath)).toEqual(["/r/a.yml", "/r/b.yml", "/r/c.yml"]); + }); + + it("returns empty when every pattern misses", async () => { + globalThis.__stubFindFiles = []; + const out = await findScannableFiles(["**/.gitlab-ci.yml"], ""); + expect(out).toEqual([]); + }); + + it("handles an empty pattern list (zero findFiles calls, empty result)", async () => { + const out = await findScannableFiles([], ""); + expect(out).toEqual([]); + expect(globalThis.__stubCalls?.findFiles).toEqual([]); + }); +}); + +describe("scanWorkspace — no-workspace path", () => { + it("returns zero counts and a friendly toast when no folder is open", async () => { + // workspaceFolders defaults to undefined after resetStubState. + const result = await scanWorkspace(); + expect(result).toEqual({ scanned: 0, failed: 0, cancelled: false }); + const info = globalThis.__stubCalls?.infoMessages ?? []; + expect(info).toHaveLength(1); + expect(info[0]).toContain("open a workspace folder"); + }); + + it("quiet mode suppresses the no-workspace toast", async () => { + const result = await scanWorkspace({ quiet: true }); + expect(result).toEqual({ scanned: 0, failed: 0, cancelled: false }); + expect(globalThis.__stubCalls?.infoMessages ?? []).toEqual([]); + }); +}); + +describe("scanWorkspace — no-files path", () => { + beforeEach(() => { + globalThis.__stubWorkspaceFolders = [ + { uri: { toString: () => "file:///r", fsPath: "/r" } }, + ]; + globalThis.__stubFindFiles = []; + }); + + it("returns zero counts and surfaces 'no scannable files' to the user", async () => { + const result = await scanWorkspace(); + expect(result).toEqual({ scanned: 0, failed: 0, cancelled: false }); + const info = globalThis.__stubCalls?.infoMessages ?? []; + expect(info).toHaveLength(1); + expect(info[0].toLowerCase()).toContain("no scannable files"); + }); + + it("quiet mode still scans but emits no toast", async () => { + const result = await scanWorkspace({ quiet: true }); + expect(result).toEqual({ scanned: 0, failed: 0, cancelled: false }); + expect(globalThis.__stubCalls?.infoMessages ?? []).toEqual([]); + }); +}); + +describe("scanWorkspace — scanning path", () => { + beforeEach(() => { + globalThis.__stubWorkspaceFolders = [ + { uri: { toString: () => "file:///r", fsPath: "/r" } }, + ]; + }); + + it("opens every matching file and counts each as scanned", async () => { + globalThis.__stubFindFiles = [ + fakeUri("/r/.github/workflows/a.yml"), + fakeUri("/r/.github/workflows/b.yml"), + fakeUri("/r/Dockerfile"), + ]; + const result = await scanWorkspace(); + // findScannableFiles fires once per pattern (10 patterns), but + // since __stubFindFiles is the same for every include, dedupe + // collapses it back to the three URIs. + expect(result.scanned).toBe(3); + expect(result.failed).toBe(0); + expect(result.cancelled).toBe(false); + }); + + it("emits a success toast in noisy mode", async () => { + globalThis.__stubFindFiles = [fakeUri("/r/Dockerfile")]; + await scanWorkspace(); + const info = globalThis.__stubCalls?.infoMessages ?? []; + expect(info.some((m) => m.toLowerCase().includes("scanned"))).toBe(true); + }); + + it("emits no toast in quiet mode even when files were scanned", async () => { + globalThis.__stubFindFiles = [fakeUri("/r/Dockerfile")]; + await scanWorkspace({ quiet: true }); + expect(globalThis.__stubCalls?.infoMessages ?? []).toEqual([]); + expect(globalThis.__stubCalls?.warningMessages ?? []).toEqual([]); + }); + + it("counts an openTextDocument rejection as failed without aborting the run", async () => { + const ok1 = fakeUri("/r/a.yml"); + const bad = fakeUri("/r/bad.yml"); + const ok2 = fakeUri("/r/b.yml"); + globalThis.__stubFindFiles = [ok1, bad, ok2]; + globalThis.__stubOpenTextDocumentFailures = new Set([bad.toString()]); + + const result = await scanWorkspace(); + expect(result.scanned).toBe(2); + expect(result.failed).toBe(1); + expect(result.cancelled).toBe(false); + // A run with failures surfaces a WARNING (not info) so the + // partial-success state is visible without being noisy. + const warnings = globalThis.__stubCalls?.warningMessages ?? []; + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("1 failed"); + }); + + it("reports cancellation with the partial scanned count", async () => { + globalThis.__stubFindFiles = [ + fakeUri("/r/a.yml"), + fakeUri("/r/b.yml"), + fakeUri("/r/c.yml"), + ]; + // Cancel before the loop body runs so every file is skipped. + globalThis.__stubProgressCancelled = true; + const result = await scanWorkspace(); + expect(result.cancelled).toBe(true); + expect(result.scanned).toBe(0); + // Cancelled runs route the summary to a warning. + expect(globalThis.__stubCalls?.warningMessages ?? []).toHaveLength(1); + }); +}); From a3ababe81e3a9d68aed55e6df6fc1912910d937e Mon Sep 17 00:00:00 2001 From: Daniel Martin <56157528+dmartinochoa@users.noreply.github.com> Date: Tue, 19 May 2026 19:51:04 +0200 Subject: [PATCH 3/4] docs(roadmap): reconcile with v1.0.0 release + R17 integration tests landed - status snapshot: mark v0.1.1, v0.2.0, v1.0.0 as shipped (was 'in flight'); add Post-1.0.0 row pointing at scan-workspace fix, welcome-panel rework, serialize-javascript override, and open PRs - Tests section: integration tests bullet flipped to done with back-ref to R17/PR #14 and the src/test/integration/activation.test.ts surface - C1 / H4 / maintainer-item-4 manual smokes: marked done; v1.0.0 has been live on the marketplace without a regression report, so the hypothesis is moot --- ROADMAP.md | 65 +++++++++++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index e902d5c..c0fd284 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,17 +1,20 @@ # Roadmap Production-readiness work for the Pipeline-Check VS Code extension. The -pre-marketplace security and packaging review (C/H/M/L items below) is -fully landed in v0.1.1. The in-depth code review of 2026-05-19 (R items -at the bottom) is two-thirds landed across PRs #11–14. +pre-marketplace security and packaging review (C/H/M/L items below) +landed in v0.1.1; the in-depth code review of 2026-05-19 (R items at +the bottom) landed across PRs #11–14, #18–22; v1.0.0 shipped +2026-05-19. Everything still open is either blocked on an upstream +input or out-of-scope. ### Status snapshot | Layer | State | |---|---| | **v0.1.0 → v0.1.1** | Shipped 2026-05-19. C1–C2, H1–H4, M1–M5, L1–L6 all closed. | -| **v0.1.1 → v0.2.0 (in flight)** | R1–R9, R12, R14, R16–R18, R20, R21, R24–R26 landed on stacked PRs #11–#14; merge them in order, then tag. | -| **v0.2.0 → 1.0 (in flight)** | R10/R15 (scan-workspace), R22 (eslint-flat-config), R29 (scan-on-save) landed; PVR + Discussions enabled on the repo. | +| **v0.1.1 → v0.2.0** | Shipped. R1–R9, R12, R14, R16–R18, R20, R21, R24–R26 landed on stacked PRs #11–#14. | +| **v0.2.0 → 1.0.0** | Shipped 2026-05-19 ([a202496](https://github.com/greylag-ci/pipeline-check-vscode/commit/a202496)). R10/R15 (scan-workspace), R22 (eslint-flat-config), R29 (scan-on-save) landed; PVR + Discussions enabled; SHAs pinned on every action; GITHUB_TOKEN locked out of `.git/config`. | +| **Post-1.0.0** | Scan-workspace nested-brace fix ([1a2d58f](https://github.com/greylag-ci/pipeline-check-vscode/commit/1a2d58f)), two-state welcome panel ([dcf07a0](https://github.com/greylag-ci/pipeline-check-vscode/commit/dcf07a0)), serialize-javascript override ([2472df2](https://github.com/greylag-ci/pipeline-check-vscode/commit/2472df2)). PR #28 (test coverage 134→187) and PR #27 (SBOM/provenance) open. | | **Blocked** | R11 (need suppression-comment syntax), R13/R27 (server-side change), R19 (interactive screenshot session), R23 (CodeQL setup). | | **Decided against** | R28 (no telemetry — see SECURITY.md). | @@ -19,7 +22,7 @@ at the bottom) is two-thirds landed across PRs #11–14. These cannot land from a branch and have been queued since the production-readiness pass. Each one's failure mode is small enough -that v0.2.0 can ship without them, but the listing improves once +that v1.0.0 has shipped without them, but the listing improves once they're done. 1. **Resolve the CodeQL default-setup conflict.** The advanced @@ -35,10 +38,12 @@ they're done. 3. **Enable Discussions.** ✅ Enabled 2026-05-19 via the GitHub API; the `qna` link in [package.json](package.json) now resolves on the marketplace listing. -4. **Manual H4 smoke** — F5 with the sample-workflow profile, open - each provider's trigger file, confirm diagnostics still appear. - The activation narrowing drops custom workflow paths intentionally - but the regression risk is non-zero. +4. **Manual H4 smoke** — ✅ Effectively cleared by v1.0.0 shipping + on the marketplace without a regression report. The historical + item asked the maintainer to F5 each provider's trigger file + after the activation narrowing; v1.0.0 has been live since + 2026-05-19 with no Discussions or issues filed against + provider-activation regressions. 5. **Capture marketplace screenshots** ([R19](#review-pass-2026-05-19--improvements-from-in-depth-code-review)). Highest-leverage conversion improvement still pending. @@ -67,14 +72,12 @@ is almost certainly broken in a clean install. CI only verifies that the **Plan** -- [ ] **Manual smoke** the maintainer should run: install the - published v0.1.0 in a clean VS Code that doesn't have a sibling - `pipeline-check-vscode` checkout and confirm it fails to - activate. Either: (a) confirms the hypothesis and we cut a 0.1.1 - hotfix from this branch, or (b) reveals a vsce behavior I don't - know about (e.g. it auto-includes prod deps regardless of - `.vscodeignore`) — in which case C1's CI smoke step still has - value as defense-in-depth. +- [x] **Manual smoke** — superseded by shipping v0.1.1 from this + branch with the bundle work below. v0.1.0 → v1.0.0 path has + since published cleanly via the marketplace, so the + missing-runtime-dep hypothesis is moot; `npm run smoke` + ([scripts/smoke.js](scripts/smoke.js)) prevents the regression + in CI. - [x] Add an esbuild bundle: `bundle:dev` (sourcemap) and `bundle:prod` (minified). `vscode:prepublish` runs `typecheck && bundle:prod`. `compile` runs `typecheck && bundle:dev` so F5 stays @@ -185,13 +188,12 @@ A tag created on an arbitrary commit or a force-moved tag would still ship. reliance on the server's content filter as a first line of defence, and no dependency on which language extension owns the `github-actions-workflow` language ID. -- [ ] **Manual smoke** the maintainer should run before merging this - branch: open each provider's fixture (GHA, GitLab, Azure, - Bitbucket, CircleCI, Cloud Build, Buildkite, Drone, Jenkins, - Dockerfile) and confirm diagnostics still appear. Custom - workflow paths (e.g. `pipelines/build.yml`) will no longer - activate the extension — that's the intent, but worth knowing - before users surface it as a bug. +- [x] **Manual smoke** — effectively cleared by v1.0.0 shipping + on the marketplace without a provider-activation regression + report. Custom workflow paths (e.g. `pipelines/build.yml`) + intentionally no longer activate the extension; nobody has + filed against this in Discussions or Issues since the change + landed. --- @@ -242,13 +244,12 @@ it as more pure-logic modules are extracted. → uppercase, unknown → INFO fallback), and the no-refresh-storm contract on a same-mode `setGroupMode` call. Uses `vi.mock("vscode", ...)` to stub the editor namespace. -- [ ] **VS Code integration tests** with `@vscode/test-electron` once - the surface stabilises. Useful for: real diagnostic publishing - end-to-end, the tree view actually rendering in a VS Code host, - and the workspace-trust prompt path. Held back because the - payoff per test is high but the marginal cost of each test is - also high (boot a real Electron + extension host), so the unit - suite earns its keep first. +- [x] **VS Code integration tests** with `@vscode/test-electron` — + landed via [R17](#testing) (PR #14, [3e8370b](https://github.com/greylag-ci/pipeline-check-vscode/commit/3e8370b)) + and extended in PR #28. See + [src/test/integration/activation.test.ts](src/test/integration/activation.test.ts): + activation, command registration, view registration, settings + schema, workspace-trust capability. `npm test` runs the suite (configured in [vitest.config.ts](vitest.config.ts)); both ci.yml and publish.yml run From 47d5862e9c5faebc76bfc22355dcca864fb108d9 Mon Sep 17 00:00:00 2001 From: Daniel Martin <56157528+dmartinochoa@users.noreply.github.com> Date: Tue, 19 May 2026 19:51:18 +0200 Subject: [PATCH 4/4] fix: harden four high-severity activation / scan edge cases 1. LSP crash mid-session no longer leaves the welcome panel showing 'Scan workspace' against a dead server. Subscribe to onDidChangeState so State.Stopped flips pipelineCheck.lspReady back to false; the install-prompt welcome state returns automatically. 2. providerForPath now matches case-insensitively, so a workspace with lowercase 'dockerfile' or 'jenkinsfile' is classified correctly and the disabledProviders middleware can actually silence it. Affects Windows case-preserving filesystems and any user who lowercased the file. Globs themselves stay POSIX-shaped. 3. client.start() is raced against a 30-second timeout. Previously an empty pipelineCheck.serverArgs (or any hung interpreter) left activation pending forever and the welcome panel stuck on the install prompt. On timeout we fire-and-forget client.stop() and surface the standard 'Install in terminal / Open server log' toast. 4. scanWorkspace gates on isLspReady() (new synchronous mirror of the context key) and routes the user toward Install / Restart / Show log via a warning toast when the LSP is down. Previously the scan would openTextDocument every candidate file and complete with a misleading 'scanned N files' toast even though no LSP was alive to produce findings. Quiet mode (scan-on-save) stays silent. Tests: +7 (194 total). Adds coverage for the isLspReady readback, the lowercase/mixed-case provider match, and the LSP-not-ready scan gate (zero counts, no findFiles call, warning toast in noisy mode, silence in quiet mode). Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 29 +++++++++++++++ src/extension.ts | 78 ++++++++++++++++++++++++++++++++++++++- src/lspState.test.ts | 23 +++++++++++- src/lspState.ts | 26 ++++++++++++- src/providers.test.ts | 24 ++++++++++++ src/providers.ts | 18 ++++++--- src/workspaceScan.test.ts | 50 +++++++++++++++++++++++++ src/workspaceScan.ts | 32 ++++++++++++++++ 8 files changed, 271 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7841f00..23d09af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,35 @@ versions follow [SemVer](https://semver.org/). ## [Unreleased] +### Fixed + +- **Welcome panel stops lying after an LSP crash.** Subscribe to + `client.onDidChangeState` so a mid-session server crash (or the + LanguageClient's auto-restart exhausting) flips `pipelineCheck.lspReady` + back to `false`. Previously the panel kept showing "Scan workspace" + after the LSP died — the button still worked, it just opened files + against a dead server. +- **`disabledProviders` now silences lowercase `dockerfile` / + `jenkinsfile`.** The internal glob matcher in `providerForPath` was + case-sensitive against `**/Dockerfile`, so files written in lowercase + (common on Windows case-preserving filesystems) classified as + `undefined` and slipped past the user's disable filter. +- **Activation no longer hangs on a misconfigured `serverArgs`.** + `client.start()` is now raced against a 30-second timeout. An empty + `pipelineCheck.serverArgs` used to drop the Python child into the + REPL where it waited on stdin forever; activation would stay + half-pending and the welcome panel would never leave the install + prompt. On timeout we kill the stranded subprocess and surface the + same "Install in terminal / Open server log" toast the LSP-failure + path uses. +- **`Scan workspace` no longer claims success against a dead LSP.** + The scan command now gates on `isLspReady()` and surfaces a warning + toast with **Install in terminal / Restart language server / Open + server log** when the LSP is down. Quiet mode (scan-on-save) stays + silent. Previously the scan would `openTextDocument` every candidate + file, publish no diagnostics, and finish with a "scanned N files" + toast even though no LSP was alive to produce findings. + ## [1.0.0] — 2026-05-19 First stable release. Closes the v0.x line: the Findings tree has its diff --git a/src/extension.ts b/src/extension.ts index 95829e5..728b38a 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -16,6 +16,7 @@ import { LanguageClient, LanguageClientOptions, ServerOptions, + State, TransportKind, } from "vscode-languageclient/node"; import { FindingsCodeLensProvider } from "./codeLens"; @@ -109,6 +110,22 @@ type LeafLike = { }; let client: LanguageClient | undefined; +// Disposable for the onDidChangeState listener registered against the +// current `client`. We hang on to it so `stopClient` can dispose it +// before the next `startClient` builds a fresh listener — otherwise a +// crash on the previous client would still fire into our handler and +// flip `lspReady` against the live client. +let clientStateChangeDisposable: vscode.Disposable | undefined; +// Hard ceiling on how long `client.start()` is allowed to run before +// we treat the LSP as broken. Without this, a `serverArgs: []` +// (configured Python interpreter drops into the REPL waiting on +// stdin), or any Python interpreter that hangs during module import, +// leaves `activate()` pending forever — the install-prompt welcome +// panel stays up and the user has no way to know the difference +// between "LSP is slow" and "LSP will never come up". 30 s is well +// above the cold-start budget on Windows (where pyc compilation can +// add several seconds the first time pipeline_check.lsp imports). +const START_TIMEOUT_MS = 30_000; function buildClient(): LanguageClient { const config = vscode.workspace.getConfiguration("pipelineCheck"); @@ -193,9 +210,25 @@ async function startClient(): Promise { clientLog.setLogChannel(outputChannel); try { clientLog.info("language server: starting"); - await client.start(); + await startWithTimeout(client, START_TIMEOUT_MS); clientLog.info("language server: started"); setLspReady(true); + // Watch the post-start lifecycle so a mid-session crash (server + // process exits, LanguageClient's auto-restart exhausts) flips the + // welcome panel back to the install-prompt state. Without this, + // `lspReady` only ever transitions back to false on an explicit + // stop/restart — a crashed server would leave the panel saying + // "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) => { + if (event.newState === State.Stopped) { + clientLog.warn("language server: state transitioned to stopped"); + setLspReady(false); + } else if (event.newState === State.Running) { + setLspReady(true); + } + }); } catch (err) { // The most common cause is `python -m pipeline_check.lsp` failing: // either Python is not on PATH or the [lsp] extra is not installed. @@ -245,6 +278,14 @@ async function stopClient(): Promise { const local = client; client = undefined; setLspReady(false); + // Drop the state-change listener BEFORE awaiting stop(). Otherwise + // the Stopped transition that stop() triggers re-fires our handler + // against the now-detached client and calls setLspReady(false) a + // second time. Cheap, but the second flip is misleading in the log. + if (clientStateChangeDisposable) { + clientStateChangeDisposable.dispose(); + clientStateChangeDisposable = undefined; + } let timer: NodeJS.Timeout | undefined; try { await Promise.race([ @@ -263,6 +304,41 @@ async function stopClient(): Promise { } } +/** + * 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 { + let timer: NodeJS.Timeout | undefined; + const timeout = new Promise((_, 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 { diff --git a/src/lspState.test.ts b/src/lspState.test.ts index ed936e3..68132f4 100644 --- a/src/lspState.test.ts +++ b/src/lspState.test.ts @@ -6,7 +6,7 @@ vi.mock("vscode", async () => { }); import { resetStubState } from "./__testStubs__/vscode"; -import { LSP_READY_CONTEXT_KEY, setLspReady } from "./lspState"; +import { LSP_READY_CONTEXT_KEY, isLspReady, setLspReady } from "./lspState"; beforeEach(() => { resetStubState(); @@ -67,3 +67,24 @@ describe("setLspReady", () => { } }); }); + +describe("isLspReady", () => { + // Synchronous mirror of what setLspReady last pushed. The + // scan-workspace gate reads through this rather than through a + // setContext readback (VS Code has no synchronous getter) — if the + // two ever drift, the welcome panel and the scan gate disagree. + + it("reports the value of the last setLspReady call", () => { + setLspReady(true); + expect(isLspReady()).toBe(true); + setLspReady(false); + expect(isLspReady()).toBe(false); + }); + + it("returns true while ready even if other commands have fired", () => { + setLspReady(true); + // A future caller might call executeCommand on other surfaces in + // between; the readback must stay tied to setLspReady alone. + expect(isLspReady()).toBe(true); + }); +}); diff --git a/src/lspState.ts b/src/lspState.ts index a685d89..7e1b1db 100644 --- a/src/lspState.ts +++ b/src/lspState.ts @@ -8,6 +8,15 @@ import * as vscode from "vscode"; export const LSP_READY_CONTEXT_KEY = "pipelineCheck.lspReady"; +// Module-level mirror of what we last pushed to the VS Code context +// key. setContext is fire-and-forget and there is no synchronous +// reader for context keys, so anything that needs to gate behavior on +// LSP readiness (e.g. the scan-workspace command) reads through +// isLspReady() instead. Kept in lockstep with setContext below — a +// missed update here means both the welcome panel and the gate +// disagree, which is worse than either alone. +let ready = false; + /** * Set the `pipelineCheck.lspReady` context key. The two viewsWelcome * entries — install-prompt vs scan-workspace — read from this key, so @@ -19,10 +28,23 @@ export const LSP_READY_CONTEXT_KEY = "pipelineCheck.lspReady"; * returned promise's rejection handling (VS Code's setContext does * not reject in practice). */ -export function setLspReady(ready: boolean): void { +export function setLspReady(value: boolean): void { + ready = value; void vscode.commands.executeCommand( "setContext", LSP_READY_CONTEXT_KEY, - ready, + value, ); } + +/** + * Synchronous readback for code paths that must decide whether the + * LSP is alive RIGHT NOW (e.g. scan-workspace, which would otherwise + * happily open every candidate document against a dead server and + * report "scanned N files" with no findings). The welcome panel + * still reads off the VS Code context key — this is for code that + * runs outside a `when:` clause. + */ +export function isLspReady(): boolean { + return ready; +} diff --git a/src/providers.test.ts b/src/providers.test.ts index 4cdb625..f7a04be 100644 --- a/src/providers.test.ts +++ b/src/providers.test.ts @@ -113,4 +113,28 @@ describe("providerForPath", () => { expect(providerForPath("/repo/mkdocs.yml")).toBeUndefined(); expect(providerForPath("/repo/values.yaml")).toBeUndefined(); }); + + it("matches Dockerfile and Jenkinsfile case-insensitively", () => { + // Windows file systems are case-insensitive but preserve the + // on-disk case in `fsPath`. A user with `dockerfile` (lowercase) + // or `DOCKERFILE` on disk would otherwise slip through + // providerForPath as `undefined`, and the disabledProviders + // middleware filter could not silence them. Same risk on + // case-insensitive macOS APFS volumes and for users who simply + // happen to lowercase build files. The match has to follow. + expect(providerForPath("/repo/dockerfile")).toBe("dockerfile"); + expect(providerForPath("/repo/DOCKERFILE")).toBe("dockerfile"); + expect(providerForPath("/repo/Dockerfile")).toBe("dockerfile"); + expect(providerForPath("/repo/jenkinsfile")).toBe("jenkins"); + expect(providerForPath("/repo/JENKINSFILE")).toBe("jenkins"); + expect(providerForPath("/repo/containerfile")).toBe("dockerfile"); + expect(providerForPath("/repo/CONTAINERFILE")).toBe("dockerfile"); + }); + + it("matches workflow / config filenames regardless of case", () => { + expect(providerForPath("/repo/.gitlab-ci.YML")).toBe("gitlab"); + expect(providerForPath("/repo/.GITHUB/workflows/ci.YAML")).toBe( + "github-actions", + ); + }); }); diff --git a/src/providers.ts b/src/providers.ts index cede47f..00f9a02 100644 --- a/src/providers.ts +++ b/src/providers.ts @@ -84,9 +84,13 @@ export const TRIGGER_DOCUMENT_SELECTOR: readonly TriggerSelector[] = * diagnostics for files whose provider has been disabled in settings. * * Matching is the same minimatch dialect VS Code's `findFiles` and - * `documentSelector` use. Implemented locally with a small glob - * matcher so the function works in both the editor and the unit - * test environment. + * `documentSelector` use, but case-insensitive: Windows file systems + * happily report `dockerfile` or `JENKINSFILE` even though our globs + * spell them `Dockerfile` / `Jenkinsfile`. A case-sensitive match + * would silently drop the provider classification, and the + * `disabledProviders` middleware filter would then fail to silence + * those files. Linux users naming a file `dockerfile` would hit the + * same bug. Globs themselves are still POSIX-shaped (slashes only). */ export function providerForPath(path: string): ProviderId | undefined { // Normalise Windows backslashes — globs are POSIX-shaped. @@ -106,6 +110,8 @@ export function providerForPath(path: string): ProviderId | undefined { * `**` (any number of path segments), `*` (anything but `/`), and * brace alternatives `{a,b}`. Sufficient for `**\/.github/workflows/*.{yml,yaml}` * and similar; not a general-purpose minimatch replacement. + * + * Matching is case-insensitive — see providerForPath for the why. */ function globMatch(pattern: string, path: string): boolean { // Expand brace alternatives into a list of plain globs. @@ -128,7 +134,9 @@ function expandBraces(pattern: string): string[] { function toRegex(pattern: string): RegExp { // Walk the pattern char by char to translate `**`, `*`, and // everything else (escaped). `**` matches zero-or-more path - // segments; `*` matches anything but `/`. + // segments; `*` matches anything but `/`. The `i` flag carries + // case-insensitive matching so `Dockerfile` and `dockerfile` map + // to the same provider. let re = ""; for (let i = 0; i < pattern.length; i++) { if (pattern[i] === "*" && pattern[i + 1] === "*") { @@ -144,5 +152,5 @@ function toRegex(pattern: string): RegExp { re += pattern[i]; } } - return new RegExp(`^${re}$`); + return new RegExp(`^${re}$`, "i"); } diff --git a/src/workspaceScan.test.ts b/src/workspaceScan.test.ts index ccf0a8d..0a70ba9 100644 --- a/src/workspaceScan.test.ts +++ b/src/workspaceScan.test.ts @@ -6,6 +6,7 @@ vi.mock("vscode", async () => { }); import { resetStubState } from "./__testStubs__/vscode"; +import { setLspReady } from "./lspState"; import { findScannableFiles, formatSummary, @@ -14,6 +15,11 @@ import { beforeEach(() => { resetStubState(); + // The scan command bails when the LSP is not ready (otherwise it + // would openTextDocument every file with no didOpen recipient and + // mislead the user with a "scanned N files" toast). Default tests + // to the ready state; the not-ready behaviour gets its own block. + setLspReady(true); }); // Small URI factory matching the shape findFiles returns and @@ -262,3 +268,47 @@ describe("scanWorkspace — scanning path", () => { expect(globalThis.__stubCalls?.warningMessages ?? []).toHaveLength(1); }); }); + +describe("scanWorkspace — LSP-not-ready gate", () => { + // Without a live LSP, opening every candidate document is wasted + // work (no didOpen recipient → no diagnostics → empty Findings) and + // the completion toast would actively mislead the user. The gate + // bails early and routes the user toward the install / restart + // flow instead. + + beforeEach(() => { + globalThis.__stubWorkspaceFolders = [ + { uri: { toString: () => "file:///r", fsPath: "/r" } }, + ]; + globalThis.__stubFindFiles = [ + { toString: () => "file:///r/Dockerfile", fsPath: "/r/Dockerfile" }, + ]; + setLspReady(false); + }); + + it("returns zero counts without touching findFiles or openTextDocument", async () => { + const result = await scanWorkspace(); + expect(result).toEqual({ scanned: 0, failed: 0, cancelled: false }); + // The gate must short-circuit BEFORE the candidate enumeration — + // otherwise a 50k-file workspace pays the findFiles cost just to + // be told no. + expect(globalThis.__stubCalls?.findFiles ?? []).toEqual([]); + }); + + it("surfaces a warning toast with actionable buttons in noisy mode", async () => { + await scanWorkspace(); + const warnings = globalThis.__stubCalls?.warningMessages ?? []; + expect(warnings).toHaveLength(1); + expect(warnings[0].toLowerCase()).toContain("language server"); + expect(warnings[0].toLowerCase()).toContain("not running"); + }); + + it("suppresses the toast in quiet mode", async () => { + // scan-on-save uses quiet mode; a toast on every save would be + // unbearable when the LSP is intermittently down. + const result = await scanWorkspace({ quiet: true }); + expect(result).toEqual({ scanned: 0, failed: 0, cancelled: false }); + expect(globalThis.__stubCalls?.warningMessages ?? []).toEqual([]); + expect(globalThis.__stubCalls?.infoMessages ?? []).toEqual([]); + }); +}); diff --git a/src/workspaceScan.ts b/src/workspaceScan.ts index 4caee92..deb5af1 100644 --- a/src/workspaceScan.ts +++ b/src/workspaceScan.ts @@ -12,6 +12,7 @@ import * as vscode from "vscode"; +import { isLspReady } from "./lspState"; import { TRIGGER_PATTERNS } from "./providers"; // Common heavy directories that should never carry a real workflow file @@ -92,6 +93,37 @@ export async function scanWorkspace( return { scanned: 0, failed: 0, cancelled: false }; } + // Without a live LSP the scan would `openTextDocument` every + // candidate file and then... nothing — no didOpen recipient, no + // diagnostics published, no Findings update. The completion toast + // would still claim "scanned N files", which is a worse signal than + // a clear "LSP isn't running" cue with an actionable button. Skip + // the scan in that case and route the user toward the install / + // restart path. + if (!isLspReady()) { + if (!quiet) { + void vscode.window + .showWarningMessage( + "Pipeline-Check: language server is not running. Install it (or restart) before scanning.", + "Install in terminal", + "Restart language server", + "Open server log", + ) + .then((choice) => { + if (choice === "Install in terminal") { + void vscode.commands.executeCommand( + "pipelineCheck.installInTerminal", + ); + } else if (choice === "Restart language server") { + void vscode.commands.executeCommand("pipelineCheck.restart"); + } else if (choice === "Open server log") { + void vscode.commands.executeCommand("pipelineCheck.showLog"); + } + }); + } + return { scanned: 0, failed: 0, cancelled: false }; + } + const uris = await findScannableFiles(TRIGGER_PATTERNS, EXCLUDE_GLOB); if (uris.length === 0) {