release: v0.2.0 — Findings panel + status bar + CodeLens + navigation + per-provider toggles#16
Merged
Conversation
In-depth review queued 29 follow-up items grouped by category: code fixes, performance, UX gaps, architecture, testing, marketplace, CI, and strategic. Items already covered elsewhere in this roadmap are not repeated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lands the cheap-and-high-value subset of the post-v0.1.1 review pass. R1 — extension.ts: move the filterByThreshold import up to the rest of the import block. Was sitting after a function declaration as a merge artefact. R2 — extension.ts: "Restart language server" toast no longer fires when startClient failed. The catch path already surfaces its own error notification; the success toast now gates on `if (client)`. R3 — extension.ts: stopClient races local.stop() against a 2-second timer (STOP_TIMEOUT_MS) and dispose()s the client on timeout. A deadlocked LSP child used to hold the deactivate path indefinitely and VS Code reported "Window not responding". R4 — findingsView.ts: groupByFile drops the Uri.parse round-trip. Bucket value now carries the original Uri alongside the items array. R5 — findingsView.ts: compareByLocation sorts on fsPath instead of the full URI string. Cross-scheme entries (file:// vs untitled://) no longer bunch at one end of the tree. R6 — findingsView.ts: collectFindings memoised behind a per-refresh cache. buildRoot and updateBadge used to walk the workspace diagnostic store twice per refresh; now both read from the same walk. cachedFindings is invalidated in refresh(). R7 — findingsView.ts: onDidChangeDiagnostics handler now early-outs when the batch's URIs neither carry a pipeline-check diagnostic nor appeared in the last finding set. ESLint / mypy / redhat.yaml keystroke chatter no longer wakes up the tree rebuild. The lastFindingUris set carries the URIs we last had findings for so *clears* are detected too (a stale leaf can't outlive a cleared file). R8 — findingsView.ts: leaf tooltip now appends a "$(book) RULE documentation" link when the server publishes Diagnostic.code.target. MarkdownString gets isTrusted=true so the link is clickable inside a TreeItem tooltip. composeLeafTooltip extracted as a function for unit testing. R9 — statusBar.ts: new module. Adds a left-side status bar item showing per-severity counts (e.g. "$(shield) 3C 1H"). Click reveals the Findings panel via the auto-generated pipelineCheck.findings.focus command. Updates on every onDidChangeDiagnostics; pure helpers (formatStatusBarText, formatStatusBarTooltip, countDiagnostics) live on the module so the tests pin the copy without booting VS Code. R21 — ci.yml: matrix over [ubuntu-latest, windows-latest, macos-latest]. npm audit and the vsix upload stay pinned to Linux (network-bound / would otherwise collide on identical names). fail-fast: false so a Windows-only failure doesn't kill the macOS and Linux runs. Tests: - findingsView.test.ts: MarkdownString stub gains appendMarkdown / isTrusted / supportThemeIcons. getDiagnostics stub handles both the zero-arg (all pairs) and uri-arg (just that URI) forms used by R7's batch-touches-us check. - findingsView.test.ts: +3 tests covering R8 (link appended when docsUrl present, tooltip clean when absent) and R6 (refresh() picks up newly-published diagnostics). - statusBar.test.ts: new file. 13 tests across formatStatusBarText (clean / critical-first / high-pair / collapse-to-total), formatStatusBarTooltip (no findings / breakdown / singular), and countDiagnostics (source filter / tally / INFO fallback / lowercase normalise). Total: 53 tests pass; lint + smoke clean. Three-OS CI verifies on each push. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stacked on review-followups (#11). Lands the next batch of items from the post-v0.1.1 review: R12, R14, R16, R18, R20. R12 — next/previous finding navigation: - src/navigate.ts: collectFindingLocations enumerates every pipeline-check diagnostic across the workspace, sorted by fsPath then by line. pickNextIndex picks the neighbouring finding relative to the editor cursor, wrapping at both ends. goToFinding pulls the active editor's location, calls pickNextIndex, and reveals the target via showTextDocument. - src/extension.ts: registers pipelineCheck.goToNextFinding and pipelineCheck.goToPreviousFinding. - package.json: declares both commands and binds Alt+F8 / Shift+Alt+F8 (the F8 family is VS Code's "next problem" muscle memory; Alt+F8 stays out of conflict with the global binding). - src/navigate.test.ts: 11 tests pinning the sort order, the wrap behaviour at both ends, the no-cursor case, and the strict-after / strict-before semantics that keep "next" from pinning when the cursor sits exactly on a finding. R14 — single source of truth for trigger patterns: - src/providers.ts: TRIGGER_PATTERNS + TRIGGER_DOCUMENT_SELECTOR. The activationEvents in package.json cannot import this module (VS Code reads the manifest before any code runs), so the events list duplicates the patterns intentionally. - src/providers.test.ts: locks the documentSelector list to the package.json activationEvents — every workspaceContains: event must correspond to a pattern, and vice versa, with brace-glob expansion. Catches drift before it ships. - src/extension.ts: documentSelector now reads from providers.ts. R16 — client-side structured logging: - src/log.ts: appendLine into the LanguageClient's outputChannel with a [client] HH:MM:SS.mmm <level> prefix. info/warn/error helpers and a withTiming wrapper that brackets a thunk with start/ok/failed lines. Silent no-op until setLogChannel is called, so activation-order edge cases don't throw. - src/extension.ts: pointed setLogChannel at outputChannel after the client is constructed; logs "starting" / "started" / "failed to start — <reason>" around the LSP boot. - src/log.test.ts: 7 tests covering timestamp formatting, level preservation, the pre-setLogChannel no-op path, and withTiming's success / failure branches. R18 — shared vscode test stub: - src/__testStubs__/vscode.ts: extracted the 93-line vi.mock factory out of findingsView.test.ts. Each test file now reads: vi.mock("vscode", async () => { const { vscodeStub } = await import("./__testStubs__/vscode"); return vscodeStub(); }); vi.mock's hoisting forbids referencing outer-scope bindings, so the async-import pattern is the only way to share. Returns a fresh object per call so tests don't leak state into each other. - src/findingsView.test.ts: collapsed to the shared stub. R20 — marketplace description length CI gate: - .github/workflows/ci.yml: a "Marketplace description length" step (Linux only) fails the build if package.json#description exceeds 145 characters — the marketplace's search-results truncation point. Today's description is 141 chars; the gate keeps future edits honest. Total: 75 tests pass (was 53 on review-followups). Lint, compile, smoke all green. Three-OS CI matrix on every push. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stacked on review-followups-batch-2 (#12). Lands R26, R25, R24. R26 — inline CodeLens summary: - src/codeLens.ts: FindingsCodeLensProvider pins a "Pipeline-Check: 2 critical · 1 high" lens at the top of every scanned file. Click navigates to the Findings panel. Re-emits on every onDidChangeDiagnostics; the lens text tracks the latest LSP publish. - summariseCounts and composeLensTitle are pure; tests pin the per-severity tally, severity-order rendering, and the "null when empty so the lens is omitted" contract. - src/extension.ts: registers the provider with TRIGGER_DOCUMENT_SELECTOR. R25 — per-provider toggles: - src/providers.ts: PROVIDERS map keyed by ProviderId ('github-actions', 'gitlab', 'azure', 'bitbucket', 'circleci', 'cloud-build', 'buildkite', 'drone', 'jenkins', 'dockerfile'). Dockerfile + Containerfile collapse to one 'dockerfile' id since they share syntax. TRIGGER_PATTERNS is now derived from the map so the two can't drift. - providerForPath(): glob-matches a path to a provider id. Tiny local glob matcher covers the dialect our patterns use (**, *, brace alternatives) — sufficient for the actual patterns and side-steps a runtime dependency. - package.json: new pipelineCheck.disabledProviders setting (array of provider ids, enum-constrained). The middleware drops every diagnostic for a URI whose provider sits in the disabled set, so the gutter / Problems / Findings / status bar / CodeLens all respect the toggle through one filter. - providers.test.ts: tests for every provider's path match, Dockerfile/Containerfile aliasing, Windows-backslash normalisation, and the "no match" return. R24 — pre-release channel via tag naming: - .github/workflows/publish.yml: new "Detect pre-release tag" step. Tags with a `-` after the semver core (e.g. v0.2.0-rc.1) ship as pre-release; stable tags (v0.2.0) ship to the stable channel. Detection sets PRERELEASE_FLAG (passed to vsce publish, vsce package, and ovsx publish) and GH_PRERELEASE (passed to gh release create). Header comment documents the convention. Total: 90 tests pass (was 75 on #12). +4 codeLens + +11 providers. Lint, compile, smoke all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stacked on review-followups-batch-3 (#13). Boots a real VS Code extension host in CI and verifies the contracts that unit tests can only approximate. Scope deliberately bounded to client-side activation contracts so the suite doesn't depend on Python + pipeline-check[lsp] being installed in CI. Future tests can add an LSP stub if end-to-end diagnostic publishing is worth verifying. Wiring: - .vscode-test.mjs: workspace-folder is test-fixtures/sample-workflow, so `workspaceContains:` activation events fire. - tsconfig.integration.json: compiles src/test/integration/ to out-test/. Inherits the main tsconfig's strict flags; overrides noEmit (the production build is the only no-emit path) and pulls in the mocha + node global types. - tsconfig.json: excludes src/test/integration/ so the main build doesn't try to type-check the mocha suite without mocha globals. - vitest.config.ts: same exclusion so the unit-test suite doesn't pick up mocha-style describe/test files. - package.json: npm run test:integration:compile + test:integration. - ci.yml: Linux-only step `xvfb-run -a npm run test:integration`. Windows / macOS in the matrix still exercise the bundle + smoke + unit suite; the integration suite adds the genuine extension-host contract on top. - .vscodeignore: excludes out-test/, .vscode-test.mjs, tsconfig.integration.json, vitest.config.ts. Test infra never ships in the .vsix. - .gitignore: adds out-test/. (.vscode-test/ was already ignored.) Tests (src/test/integration/activation.test.ts, 5 cases): - Extension is installed and activates. - All six declared commands register (pipelineCheck.restart / showLog / findings.refresh / findings.changeGrouping / goToNextFinding / goToPreviousFinding). - Findings view registers under the activity-bar container (proxied by the auto-generated pipelineCheck.findings.focus command). - Configuration schema exposes every documented setting (serverCommand, serverArgs, severityThreshold, disabledProviders, trace.server). - untrustedWorkspaces capability is "limited" and virtualWorkspaces is false. DevDeps: - @vscode/test-cli, @vscode/test-electron, mocha@^11, @types/mocha. - mocha pinned to ^11 (npm chose mocha@12-beta by default; the beta has unfixed advisories in `diff` and `serialize-javascript`). CI's `npm audit --omit=dev` still returns clean — the remaining dev-only advisories don't reach users. Total: 90 unit tests (unchanged), 5 integration tests. Lint, compile, unit, integration-compile, smoke all green locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Documentation pass consolidating everything across PRs #11-14 into a coherent story for v0.2.0. No code changes; lint, compile, tests, smoke all green. README.md: - New "Features" section above "What it scans" highlighting the surfaces that have landed: inline diagnostics, Findings panel, status bar item, CodeLens summary, keyboard navigation, tunable signal (severityThreshold + disabledProviders). - "Configuration" table gains pipelineCheck.disabledProviders and notes the machine-overridable scope on serverCommand/serverArgs. - New "Commands and keybindings" reference table. - New "Workspace trust" section documenting the limited-trust declaration and the machine-overridable rationale. - "Development" script list expanded to cover test, test:integration, smoke, lint. - "Releasing" section gains the vX.Y.Z-rc.N pre-release convention, the `production` GitHub Environment gate, and the three-OS CI matrix description. - Screenshot HTML comment updated to call out the four shots we want (inline / findings-panel / hover / status-bar). - New "Security" pointer to SECURITY.md. CHANGELOG.md: - Restored ## [Unreleased] block at the top. - Documented all the in-flight work for v0.2.0 under Added / Changed / Fixed, with R-tag cross-references to the roadmap. - Added a callout for the awk release-note extractor's first-`## [`-to-second behaviour, so future release commits don't accidentally ship the Unreleased boilerplate. ROADMAP.md: - "Status snapshot" table at the top mapping v0.1.0/v0.1.1/v0.2.0 state to closed item counts. - 19 of 29 R items ticked with PR cross-references (#11-#14). - Outstanding maintainer action items reorganised: stale v0.1.0-era items removed; current blockers (CodeQL setup, PVR/Discussions toggles, H4 manual smoke, screenshots) called out explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stacked on docs-polish (#15). Lands the five UX changes from the usability review: 1. First-run discovery — `onStartupFinished` activation - package.json: adds `onStartupFinished` to activationEvents so the activity-bar slot appears in every workspace, not just ones with a `workspaceContains:` match. The LSP child still only spawns when the documentSelector matches an open document, so there's no idle-Python-process cost. - viewsWelcome rewritten as teaching content with progressive disclosure: what the extension does → Python install hint with a one-click Copy command → editor onboarding → Alt+F8 keyboard hint → `---` separator → recovery actions (Restart, Open Log) demoted. 2. Right-click context menu on Findings tree leaves - New commands pipelineCheck.findings.copyRuleId and pipelineCheck.findings.openRuleDocs. Both gated to the leaf context (viewItem == pipelineCheck.finding); commandPalette `when: false` so they don't show up palette-wide without a node. - New top-level pipelineCheck.copyInstallCommand exposed for the welcome-state link and the palette — re-findable after the first-run error toast is dismissed. - extension.ts: registers all three. LeafLike structural type declared locally so extension.ts stays independent of findingsView's internal LeafNode definition. 3. Status bar — accessibility + conditional show - formatStatusBarAccessibilityLabel: full-word per-severity tally ("3 critical, 1 high") read by screen readers instead of the codicon shortcode + letter-by-letter abbreviation. - registerStatusBar: hides the item until the workspace shows evidence of being CI-relevant (either a `findFiles` match for the trigger globs or an actual diagnostic publish). The `relevant` flag latches so a "clean" workspace that USED to have findings keeps the positive signal — it just doesn't shout at frontend projects that happen to have Pipeline-Check installed. - formatStatusBarTooltip: appends "Alt+F8 / Shift+Alt+F8 to step through findings" as the trailing line — primary discovery surface for the navigation keybindings (omitted when there are zero findings). 4. Command title-case consistency - "Restart language server" → "Restart Language Server" - "Show language server output" → "Show Language Server Output" - "Refresh" → "Refresh Findings" - Existing title-case commands (Go to Next/Previous Finding, Change Grouping) unchanged. Command IDs unchanged; settings, keybindings, automation keep working. 5. pipelineCheck.codeLens.enabled setting - Defaults true. Hides the line-1 file-summary CodeLens without disabling CodeLens globally. - codeLens.ts: reads the setting on every provideCodeLenses call so a flip takes effect on the next render. The provider also subscribes to onDidChangeConfiguration and re-fires the change event when the setting flips, so the editor drops the lens immediately. Test stub - src/__testStubs__/vscode.ts gains getConfiguration backed by globalThis.__stubConfig (same pattern as __stubDiagnostics), plus Range and CodeLens classes so the toggle test can construct the provider end-to-end. onDidChangeConfiguration stub added. Tests - statusBar.test.ts: 4 new tests for formatStatusBarAccessibilityLabel (clean / per-severity / single-bucket / no-codicons), 2 new for formatStatusBarTooltip (Alt+F8 keyboard hint present / absent). - codeLens.test.ts: 3 new tests for the FindingsCodeLensProvider toggle behaviour (enabled-default emits, false drops to [], no diagnostics drops to [] regardless). - Integration test activation.test.ts: command-registration list grows by the three new commands. Total: 99 unit tests pass (was 90). Lint, compile, smoke, integration- compile all clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughPipeline-Check v0.2.0 introduces user-facing diagnostics summaries in the status bar and editor CodeLens, next/previous finding keyboard navigation, rule documentation links in tree tooltips, findings view performance optimizations through caching, a centralized provider detection system for LSP routing, structured client logging, multi-platform CI test matrix with pre-release detection, and comprehensive documentation updates. ChangesPipeline-Check v0.2.0 Release
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bumps package.json from 0.1.1 → 0.2.0 and folds the CHANGELOG [Unreleased] block into [0.2.0] — 2026-05-19 so publish.yml's awk release-note extractor picks the right section. v0.2.0 closes 24 of 29 items from the 2026-05-19 review. Headline features: status bar item, inline CodeLens summary, keyboard navigation (Alt+F8 / Shift+Alt+F8), right-click context menus on Findings tree leaves, per-provider toggles, pre-release publish channel, @vscode/test-electron integration tests. One borderline-breaking change called out in the release notes: the extension's activationEvents are narrower than v0.1.x. Files outside the standard CI paths no longer auto-scan; onStartupFinished was added so the activity-bar slot stays visible regardless. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dmartinochoa
changed the title
This was referenced May 19, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/providers.test.ts (1)
98-115: ⚡ Quick winAdd a regression case for basename suffix false-positives.
Please add a negative assertion that
providerForPath("/repo/MyDockerfile")isundefined(and similarly forMyJenkinsfile) to lock exact basename matching behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers.test.ts` around lines 98 - 115, Add regression assertions to ensure basename suffixes don't match: update the tests around providerForPath (in src/providers.test.ts)—specifically add expect(providerForPath("/repo/MyDockerfile")).toBeUndefined() and expect(providerForPath("/repo/MyJenkinsfile")).toBeUndefined() (place them with or next to the "returns undefined for unmatched paths" case) so providerForPath enforces exact basename matching for Dockerfile and Jenkinsfile.src/navigate.test.ts (1)
61-85: ⚡ Quick winAdd a same-line different-column ordering test.
Please add a case with two diagnostics on the same file+line but different
start.characterand assertcollectFindingLocationsreturns ascending column order.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/navigate.test.ts` around lines 61 - 85, Add a new assertion in the same "sorts cross-file by fsPath then by line" test that includes two diagnostics in the same file and same start.line but different start.character values (create them using diag(...) and uri(...)), call collectFindingLocations(iter) and assert the resulting order uses ascending start.character (check f.range.start.character) so entries with the same fsPath and line are ordered by column. Ensure you reference the same diag/uri entries and verify using out.map(f => [f.uri.fsPath, f.range.start.line, f.range.start.character]) or equivalent to validate column ordering.src/test/integration/activation.test.ts (1)
74-80: ⚡ Quick winInclude
pipelineCheck.codeLens.enabledin schema smoke coverage.This release adds a CodeLens toggle, but the schema assertion list doesn’t check it. Add it so integration tests lock the manifest contract.
✅ Suggested test update
for (const key of [ "serverCommand", "serverArgs", "severityThreshold", "disabledProviders", + "codeLens.enabled", "trace.server", ]) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/integration/activation.test.ts` around lines 74 - 80, Add "pipelineCheck.codeLens.enabled" to the array of schema keys being asserted in the integration test loop so the manifest contract includes the new CodeLens toggle; locate the for loop that iterates over the keys array (the array containing "serverCommand", "serverArgs", "severityThreshold", "disabledProviders", "trace.server") in activation.test.ts and append "pipelineCheck.codeLens.enabled" to that list so the test covers the new schema property.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ROADMAP.md`:
- Line 8: Change the "### Status snapshot" heading to an h2 to maintain proper
markdown hierarchy: replace the "### Status snapshot" string with "## Status
snapshot" so the heading level increments correctly after "# Roadmap" in the
document.
In `@src/__testStubs__/vscode.ts`:
- Around line 33-37: The disposable returned by the get event accessor currently
does nothing, causing listeners to remain registered; update the returned object
from the get event function so its dispose method removes the added listener
from this.listeners (e.g., find and splice/remove the listener pushed in the
accessor) instead of returning a no-op, ensuring the EventEmitter stub
unregisters callbacks when disposed.
In `@src/findingsView.ts`:
- Around line 308-323: composeLeafTooltip currently constructs a trusted
MarkdownString from unescaped diagnostic text and enables full command
execution; change it to create an empty vscode.MarkdownString and call
appendText(f.diagnostic.message) to avoid markdown injection, then append the
docs link only after validating the URL via readDocsUrl; update readDocsUrl to
parse the incoming docsUrl and allow only http or https schemes (reject or
return undefined otherwise); set md.isTrusted to { enabledCommands: [] } (or the
equivalent API object) so command URIs cannot be executed while keeping normal
web links clickable.
In `@src/navigate.ts`:
- Around line 34-39: The sort comparator for "out" only compares file path and
start.line, so same-line items can be ordered inconsistently with pickNextIndex;
update the comparator in the out.sort callback to, after comparing
a.range.start.line and b.range.start.line, also compare a.range.start.character
and b.range.start.character (i.e., return a.range.start.character -
b.range.start.character) so the ordering matches pickNextIndex's character
comparison and yields deterministic same-line navigation.
In `@src/providers.ts`:
- Around line 134-139: The current glob-to-regex translation in providers.ts
that handles a double-star (the block referencing pattern, re, and i) emits ".*"
which is too permissive for patterns like "**/Dockerfile"; change the
translation to emit a segment-aware optional directory prefix such as
"(?:.*/)?", and keep the existing logic that advances i and eats a following '/'
when present (so when pattern[i] and pattern[i+1] are '*', append "(?:.*/)?",
increment i, and if pattern[i+1] === '/' then increment i to skip the slash) so
"**/x" still matches "x" but avoids matching suffixes like "...MyDockerfile".
---
Nitpick comments:
In `@src/navigate.test.ts`:
- Around line 61-85: Add a new assertion in the same "sorts cross-file by fsPath
then by line" test that includes two diagnostics in the same file and same
start.line but different start.character values (create them using diag(...) and
uri(...)), call collectFindingLocations(iter) and assert the resulting order
uses ascending start.character (check f.range.start.character) so entries with
the same fsPath and line are ordered by column. Ensure you reference the same
diag/uri entries and verify using out.map(f => [f.uri.fsPath,
f.range.start.line, f.range.start.character]) or equivalent to validate column
ordering.
In `@src/providers.test.ts`:
- Around line 98-115: Add regression assertions to ensure basename suffixes
don't match: update the tests around providerForPath (in
src/providers.test.ts)—specifically add
expect(providerForPath("/repo/MyDockerfile")).toBeUndefined() and
expect(providerForPath("/repo/MyJenkinsfile")).toBeUndefined() (place them with
or next to the "returns undefined for unmatched paths" case) so providerForPath
enforces exact basename matching for Dockerfile and Jenkinsfile.
In `@src/test/integration/activation.test.ts`:
- Around line 74-80: Add "pipelineCheck.codeLens.enabled" to the array of schema
keys being asserted in the integration test loop so the manifest contract
includes the new CodeLens toggle; locate the for loop that iterates over the
keys array (the array containing "serverCommand", "serverArgs",
"severityThreshold", "disabledProviders", "trace.server") in activation.test.ts
and append "pipelineCheck.codeLens.enabled" to that list so the test covers the
new schema property.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a5f21f1-197e-43d7-b91a-f2a56ca9792a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
.github/workflows/ci.yml.github/workflows/publish.yml.gitignore.vscode-test.mjs.vscodeignoreCHANGELOG.mdREADME.mdROADMAP.mdpackage.jsonsrc/__testStubs__/vscode.tssrc/codeLens.test.tssrc/codeLens.tssrc/extension.tssrc/findingsView.test.tssrc/findingsView.tssrc/log.test.tssrc/log.tssrc/navigate.test.tssrc/navigate.tssrc/providers.test.tssrc/providers.tssrc/statusBar.test.tssrc/statusBar.tssrc/test/integration/activation.test.tstsconfig.integration.jsontsconfig.jsonvitest.config.ts
| 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. | ||
|
|
||
| ### Status snapshot |
There was a problem hiding this comment.
Fix heading level for proper document structure.
The heading skips from h1 (# Roadmap) to h3 (### Status snapshot). Per markdown conventions, heading levels should increment by one. Change to h2.
📝 Proposed fix
-### Status snapshot
+## Status snapshot📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### Status snapshot | |
| ## Status snapshot |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 8-8: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ROADMAP.md` at line 8, Change the "### Status snapshot" heading to an h2 to
maintain proper markdown hierarchy: replace the "### Status snapshot" string
with "## Status snapshot" so the heading level increments correctly after "#
Roadmap" in the document.
Comment on lines
+33
to
+37
| get event() { | ||
| return (listener: (e: T) => void) => { | ||
| this.listeners.push(listener); | ||
| return { dispose: () => undefined }; | ||
| }; |
There was a problem hiding this comment.
Dispose should unregister listeners in the EventEmitter stub.
The returned disposable currently does nothing, so listeners remain attached after dispose(). That can leak callbacks across test phases and produce flaky assertions.
Proposed fix
get event() {
return (listener: (e: T) => void) => {
this.listeners.push(listener);
- return { dispose: () => undefined };
+ return {
+ dispose: () => {
+ const idx = this.listeners.indexOf(listener);
+ if (idx >= 0) this.listeners.splice(idx, 1);
+ },
+ };
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| get event() { | |
| return (listener: (e: T) => void) => { | |
| this.listeners.push(listener); | |
| return { dispose: () => undefined }; | |
| }; | |
| get event() { | |
| return (listener: (e: T) => void) => { | |
| this.listeners.push(listener); | |
| return { | |
| dispose: () => { | |
| const idx = this.listeners.indexOf(listener); | |
| if (idx >= 0) this.listeners.splice(idx, 1); | |
| }, | |
| }; | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/__testStubs__/vscode.ts` around lines 33 - 37, The disposable returned by
the get event accessor currently does nothing, causing listeners to remain
registered; update the returned object from the get event function so its
dispose method removes the added listener from this.listeners (e.g., find and
splice/remove the listener pushed in the accessor) instead of returning a no-op,
ensuring the EventEmitter stub unregisters callbacks when disposed.
Comment on lines
+308
to
+323
| function composeLeafTooltip(f: Finding): vscode.MarkdownString { | ||
| // The server composes the message as "title\n\ndescription\n\nFix: ...". | ||
| // Markdown renders the paragraph rhythm; we append a docs link | ||
| // below it when the server published one. The MarkdownString must | ||
| // be ``isTrusted = true`` for the link to be clickable inside a | ||
| // TreeItem tooltip. | ||
| const md = new vscode.MarkdownString(f.diagnostic.message); | ||
| if (f.docsUrl) { | ||
| md.appendMarkdown( | ||
| `\n\n[$(book) ${f.ruleId || "Rule"} documentation](${f.docsUrl})`, | ||
| ); | ||
| } | ||
| md.isTrusted = true; | ||
| md.supportThemeIcons = true; | ||
| return md; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
`In VS Code API, for TreeItem tooltips using MarkdownString:
- Does
isTrusted = trueenablecommand:links in markdown? - Are normal
http/httpslinks still clickable whenisTrustedis{ enabledCommands: [] }?`
💡 Result:
In the VS Code API, the MarkdownString class is used to render rich text, including in TreeItem tooltips. Regarding your questions: 1) Does isTrusted = true enable command: links in markdown? Yes, setting isTrusted = true on a MarkdownString enables the execution of command: links [1][2][3]. When isTrusted is true, VS Code treats the content as coming from a trusted source, allowing it to execute commands via the command: URI scheme [2][3]. You can also use a more granular approach by setting isTrusted to an object with enabledCommands (e.g., md.isTrusted = { enabledCommands: ['myCommandId'] }) to restrict execution to specific, allow-listed commands [3][4]. 2) Are normal http/https links still clickable when isTrusted is { enabledCommands: [] }? Yes, standard http and https links remain clickable even if isTrusted is configured with an empty enabledCommands array (or even if isTrusted is false) [2][5]. The isTrusted property specifically governs the security boundary for executing commands and potentially other sensitive operations; it does not disable standard web navigation links [2]. Note that while http/https links are generally clickable, all MarkdownString content is sanitized, and the ability to interact with links can sometimes be subject to specific UI context limitations (such as those occasionally reported in TreeItem tooltips) [5][6].
Citations:
- 1: Motivate extensions to upgrade to MarkdownString microsoft/vscode#33577
- 2: Can we allow
isTrustedconfiguration for MarkdownString? microsoft/vscode-languageserver-node#503 - 3: Allow
vscode://URLs inside tooltips/hovers microsoft/vscode#181320 - 4: https://microsoft.github.io/monaco-editor/typedoc/interfaces/editor_editor_api.MarkdownStringTrustedOptions.html
- 5: https://stackoverflow.com/questions/43709824/vs-code-markdownstring-wont-display-links
- 6:
command:URIs in trustedMarkdownStringvalues in TreeItem tooltips don't execute commands. microsoft/vscode#123723
🏁 Script executed:
# Check the actual implementation of composeLeafTooltip and readDocsUrl
cat -n src/findingsView.ts | sed -n '300,330p'Repository: greylag-ci/pipeline-check-vscode
Length of output: 1486
🏁 Script executed:
# Check readDocsUrl function around line 364-379
cat -n src/findingsView.ts | sed -n '360,385p'Repository: greylag-ci/pipeline-check-vscode
Length of output: 1198
Harden trusted tooltip rendering and docs URL parsing to prevent command injection.
The code exhibits a command injection vulnerability: Line 314 creates trusted markdown from f.diagnostic.message without escaping, Line 320 enables full command execution with isTrusted = true, and Line 377 accepts any URI scheme from the server without validation. If a malicious or compromised language server sends command://... as Diagnostic.code.target, it becomes executable from the tooltip link.
Fixes required:
- Use
appendText()instead of the constructor to prevent markdown injection in the message - Validate that
docsUrlcontains onlyhttporhttpsschemes inreadDocsUrl() - Restrict
isTrustedto{ enabledCommands: [] }to disable command execution while keeping normal web links clickable
🔒 Proposed hardening
function composeLeafTooltip(f: Finding): vscode.MarkdownString {
- const md = new vscode.MarkdownString(f.diagnostic.message);
+ const md = new vscode.MarkdownString();
+ md.appendText(f.diagnostic.message);
if (f.docsUrl) {
md.appendMarkdown(
`\n\n[$(book) ${f.ruleId || "Rule"} documentation](${f.docsUrl})`,
);
}
- md.isTrusted = true;
+ md.isTrusted = { enabledCommands: [] };
md.supportThemeIcons = true;
return md;
}
function readDocsUrl(diag: vscode.Diagnostic): string | undefined {
if (
diag.code &&
typeof diag.code === "object" &&
"target" in diag.code &&
diag.code.target
) {
- return diag.code.target.toString();
+ try {
+ const raw = diag.code.target.toString();
+ const uri = vscode.Uri.parse(raw, true);
+ if (uri.scheme === "https" || uri.scheme === "http") {
+ return uri.toString(true);
+ }
+ } catch {
+ return undefined;
+ }
}
return undefined;
}Also applies to the readDocsUrl function at lines 364-379.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/findingsView.ts` around lines 308 - 323, composeLeafTooltip currently
constructs a trusted MarkdownString from unescaped diagnostic text and enables
full command execution; change it to create an empty vscode.MarkdownString and
call appendText(f.diagnostic.message) to avoid markdown injection, then append
the docs link only after validating the URL via readDocsUrl; update readDocsUrl
to parse the incoming docsUrl and allow only http or https schemes (reject or
return undefined otherwise); set md.isTrusted to { enabledCommands: [] } (or the
equivalent API object) so command URIs cannot be executed while keeping normal
web links clickable.
Comment on lines
+34
to
+39
| out.sort((a, b) => { | ||
| const lhs = a.uri.fsPath; | ||
| const rhs = b.uri.fsPath; | ||
| if (lhs !== rhs) return lhs.localeCompare(rhs); | ||
| return a.range.start.line - b.range.start.line; | ||
| }); |
There was a problem hiding this comment.
Sorting omits column, which breaks deterministic same-line navigation.
At Line 38, findings are only sorted by line; pickNextIndex compares character too. Multiple diagnostics on the same line can therefore navigate in the wrong order.
Suggested fix
out.sort((a, b) => {
const lhs = a.uri.fsPath;
const rhs = b.uri.fsPath;
if (lhs !== rhs) return lhs.localeCompare(rhs);
- return a.range.start.line - b.range.start.line;
+ const lineDiff = a.range.start.line - b.range.start.line;
+ if (lineDiff !== 0) return lineDiff;
+ return a.range.start.character - b.range.start.character;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| out.sort((a, b) => { | |
| const lhs = a.uri.fsPath; | |
| const rhs = b.uri.fsPath; | |
| if (lhs !== rhs) return lhs.localeCompare(rhs); | |
| return a.range.start.line - b.range.start.line; | |
| }); | |
| out.sort((a, b) => { | |
| const lhs = a.uri.fsPath; | |
| const rhs = b.uri.fsPath; | |
| if (lhs !== rhs) return lhs.localeCompare(rhs); | |
| const lineDiff = a.range.start.line - b.range.start.line; | |
| if (lineDiff !== 0) return lineDiff; | |
| return a.range.start.character - b.range.start.character; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/navigate.ts` around lines 34 - 39, The sort comparator for "out" only
compares file path and start.line, so same-line items can be ordered
inconsistently with pickNextIndex; update the comparator in the out.sort
callback to, after comparing a.range.start.line and b.range.start.line, also
compare a.range.start.character and b.range.start.character (i.e., return
a.range.start.character - b.range.start.character) so the ordering matches
pickNextIndex's character comparison and yields deterministic same-line
navigation.
Comment on lines
+134
to
+139
| if (pattern[i] === "*" && pattern[i + 1] === "*") { | ||
| re += ".*"; | ||
| i++; | ||
| // Eat an immediately-following `/` so `**/x` matches `x` too. | ||
| if (pattern[i + 1] === "/") i++; | ||
| } else if (pattern[i] === "*") { |
There was a problem hiding this comment.
**/file glob translation is too permissive and causes false provider matches.
At Line 134, translating **/ to .* allows suffix matches like .../MyDockerfile for the pattern **/Dockerfile. That can route files to the wrong provider and drop diagnostics incorrectly when provider silencing is enabled.
Suggested fix
- if (pattern[i] === "*" && pattern[i + 1] === "*") {
- re += ".*";
- i++;
- // Eat an immediately-following `/` so `**/x` matches `x` too.
- if (pattern[i + 1] === "/") i++;
+ if (pattern[i] === "*" && pattern[i + 1] === "*") {
+ i++;
+ // `**/` => zero-or-more full path segments.
+ if (pattern[i + 1] === "/") {
+ re += "(?:.*/)?";
+ i++;
+ } else {
+ re += ".*";
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (pattern[i] === "*" && pattern[i + 1] === "*") { | |
| re += ".*"; | |
| i++; | |
| // Eat an immediately-following `/` so `**/x` matches `x` too. | |
| if (pattern[i + 1] === "/") i++; | |
| } else if (pattern[i] === "*") { | |
| if (pattern[i] === "*" && pattern[i + 1] === "*") { | |
| i++; | |
| // `**/` => zero-or-more full path segments. | |
| if (pattern[i + 1] === "/") { | |
| re += "(?:.*/)?"; | |
| i++; | |
| } else { | |
| re += ".*"; | |
| } | |
| } else if (pattern[i] === "*") { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers.ts` around lines 134 - 139, The current glob-to-regex
translation in providers.ts that handles a double-star (the block referencing
pattern, re, and i) emits ".*" which is too permissive for patterns like
"**/Dockerfile"; change the translation to emit a segment-aware optional
directory prefix such as "(?:.*/)?", and keep the existing logic that advances i
and eats a following '/' when present (so when pattern[i] and pattern[i+1] are
'*', append "(?:.*/)?", increment i, and if pattern[i+1] === '/' then increment
i to skip the slash) so "**/x" still matches "x" but avoids matching suffixes
like "...MyDockerfile".
Surfaced by the integration tests hanging in CI: when `python -m pipeline_check.lsp` fails to launch (Python missing, [lsp] extra not installed, server crash on import), the catch path used to await the result of `vscode.window.showErrorMessage`. That promise only resolves when the user clicks a button or closes the toast — so any context that doesn't dismiss it (CI, automation, hidden host window, user who minimised VS Code) deadlocked activate() forever. The fix detaches the notification: fire it, route the button choice through a `.then` handler that still does the copy / log-open work, and return from startClient as soon as the error is logged. Activate now completes in a bounded time regardless of whether anyone clicks the toast. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`suite` / `test` are mocha's TDD-interface exports; `describe` / `it` are BDD. `.vscode-test.mjs` was set to `ui: "bdd"` which leaves the TDD globals undefined, so loading activation.test.js threw `ReferenceError: suite is not defined` and the integration job failed before any test ran. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 19, 2026
5 tasks
dmartinochoa
added a commit
that referenced
this pull request
May 19, 2026
…release (#31) #16 — switch PIP_INSTALL_COMMAND to `python -m pip install ...` instead of the bare `pip install ...` form. Two real-world failure modes drove this: a corporate Windows PowerShell ExecutionPolicy that allows python.exe but blocks pip.exe (the shim is a script), and the case where the official Python installer leaves `python` on PATH but not `pip` (the Scripts directory wasn't added). Both are silent today — the user sees "command not found" or a PowerShell policy error and has no way to recover from the install-in-terminal CTA. The `python -m pip` form sidesteps both by running pip via the interpreter, and matches PyPA's own recommendation. README and sample-workflow README copy updated. #13 — status-bar relevance latch releases on folder removal. A multi-root workspace user who removed the last CI folder used to see the bar item pinned to "clean" for the rest of the session — the `relevant` flag was a one-way latch. Now subscribes to onDidChangeWorkspaceFolders and re-sweeps: when neither CI candidate files nor current findings remain, the item hides again. Re-adding a CI folder re-shows. The diagnostic check inside the recheck guards against a momentary "no candidates" state during a rebuild accidentally hiding the bar. Tests: +6 (245 → 251). Install: +2 pinning the `python -m pip` prefix and the [lsp] extra. Status bar: +4 covering the latch-release path (hides on folder removal, holds on stale findings, re-latches on folder add, single-subscription contract). Stub additions: __stubWorkspaceFoldersListeners so tests can fire onDidChangeWorkspaceFolders manually. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v0.2.0 release
Squashed mega-PR. Closes #11, #12, #13, #14, #15 — every commit from those PRs is in this one's history. Adds a
chore(release): v0.2.0commit at the tip: bumpspackage.jsonto0.2.0and folds the CHANGELOG[Unreleased]block into[0.2.0] — 2026-05-19.Closes 24 of 29 items from the 2026-05-19 in-depth UX / code review.
Headline features
pipelineCheck.codeLens.enabled.Alt+F8/Shift+Alt+F8step between findings.pipelineCheck.disabledProvidersfor monorepo noise reduction.vX.Y.Z-rc.Ntag convention.What's in the commit history
Heads-up for users with non-standard workflow paths
activationEventsnow match only theworkspaceContains:patterns shared with the LSPdocumentSelector(plusonStartupFinishedso the activity-bar slot is always visible). Users with CI definitions outside the standard locations (e.g.pipelines/build.ymlinstead of.github/workflows/*.yml) will still see the extension activate, but the LSP won't auto-scan their custom paths. Workaround: symlink into a standard location.Test plan
npm run lintcleannpm run compilecleannpm test— 99 unit tests passnpm run smokecleanAfter merge
git checkout main && git pullgit tag v0.2.0 && git push origin main v0.2.0productionenvironment when the publish job pauses.Outstanding maintainer items (NOT release blockers)
analyzecheck stays red on PRs (R23) — repo-settings change needed.qnalink.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
pipelineCheck.disabledProvidersandpipelineCheck.codeLens.enabled.Documentation
Chores