feat: pipelineCheck.scanWorkspace + refresh-as-scan (R10, R15)#19
Merged
Conversation
The pre-v0.2.0 `scan-workspace` branch was stranded with merge conflicts in package.json and src/extension.ts. This commit re-applies the work cleanly on top of current main, taking advantage of the v0.2.0 infrastructure (providers.ts as the single source of truth, the shared test stub, the title-case command convention). What it does - New `pipelineCheck.scanWorkspace` command walks every CI/config file in the workspace (matching the same patterns the LSP's documentSelector uses), opens each via openTextDocument so the LSP's didOpen pipeline produces diagnostics, and lets the Findings panel re-render as publishes arrive. Progress toast with cancellation; per-file failures are counted but never abort the whole scan. - `Refresh Findings` (the title-bar button) now triggers a real scan instead of re-rendering the tree from already-published diagnostics. Matches the user's mental model: a refresh icon should fetch fresh data, not re-paint stale data. (R10) - `onCommand:pipelineCheck.scanWorkspace` activation event is auto-generated by VS Code from the contributes.commands entry — no explicit declaration needed in activationEvents. (R15 closed by VS Code's own behaviour.) What it ties together - workspaceScan.ts imports TRIGGER_PATTERNS from providers.ts, so the documentSelector, the activationEvents, and the workspace scan all read from one list. No drift. - The scan command appears at three surfaces: a `$(play)` button on the Findings view title bar (group navigation@0, leftmost), the Command Palette, and a link in the Findings welcome state. Files - src/workspaceScan.ts: re-applied with TRIGGER_PATTERNS import. - src/workspaceScan.test.ts: 8 tests for buildScanGlob + formatSummary (the pattern-list assertion was redundant with providers.test.ts — dropped). - src/extension.ts: registerCommand for both scanWorkspace and the re-pointed findings.refresh. - package.json: command, view/title button (group 0, leftmost), Command Palette entry, welcome-state link. - CHANGELOG.md: new Unreleased block with R10 + R15 entries. - src/test/integration/activation.test.ts: command-registration expected list grows by pipelineCheck.scanWorkspace. Total: 107 unit tests pass (was 99). Lint, compile, smoke, integration-compile all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds a new Scan Workspace command that discovers CI/config files across the workspace and triggers diagnostics via LSP didOpen. Refresh Findings now performs a real scan. It also adds a one-time “what’s new” upgrade toast and status-bar background color selection; tests and manifest/welcome text are updated. ChangesWorkspace Scan Feature
Sequence DiagramsequenceDiagram
participant User
participant FindingsView as Findings View
participant Command as pipelineCheck.scanWorkspace
participant Scan as scanWorkspace
participant Workspace as vscode.workspace
participant Window as vscode.window
participant LSP as Language Server
User->>FindingsView: Click Scan workspace (view title)
FindingsView->>Command: trigger command
Command->>Scan: scanWorkspace()
Scan->>Workspace: findFiles(glob, exclude)
Workspace-->>Scan: candidate URIs
Scan->>Window: withProgress(cancellableToken)
loop for each candidate
Scan->>Workspace: openTextDocument(uri)
Workspace->>LSP: didOpen (diagnostic request)
LSP-->>Workspace: diagnostics published
end
Scan-->>Window: showInformationMessage / showWarningMessage with counts
Window-->>User: toast summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/workspaceScan.test.ts (1)
11-17: ⚡ Quick winAdd targeted tests for
scanWorkspacecancellation/failure accounting.This suite is strong on pure logic, but the scan loop itself is the highest-risk path. A couple mocked tests for cancel + partial failures would lock down behavior regressions.
🤖 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/workspaceScan.test.ts` around lines 11 - 17, The tests are missing coverage for scan loop cancellation and partial failure accounting in scanWorkspace; add targeted unit tests that mock the async scan loop to simulate a user cancellation and a mix of file-processing failures, then assert that scanWorkspace returns/throws the expected cancellation signal and that the final summary counts (failed/cancelled/processed) and user-facing summary text reflect those outcomes. Locate and exercise the scanWorkspace function (and reuse buildScanGlob for input shape) while stubbing the file iteration/openTextDocument/findFiles behavior (or the provider used by providers.test.ts) to produce controlled errors and an early cancellation, and assert the summary string and any returned result object reports the correct failed and cancelled counts. Ensure tests are isolated and deterministic (use jest mocks/fakes for async timing) and add them alongside the existing pure-logic suite.src/workspaceScan.ts (1)
92-94: ⚡ Quick winAdd per-file error context in the failure path.
Right now failures are only counted, so users/devs can’t tell which file failed to open. Logging the URI + error message here makes scan failures actionable.
Suggested diff
- } catch { + } catch (error) { failed += 1; + console.warn( + `[Pipeline-Check] scanWorkspace: failed to open ${uri.fsPath}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); }🤖 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/workspaceScan.ts` around lines 92 - 94, The catch block that only increments failed (in src/workspaceScan.ts around the code that opens files—look for the loop/handler using the failed variable, e.g., the function that scans files or processes a single file) should log per-file error context: capture the file URI/identifier (the same variable used to open/read the file, e.g., fileUri, uri, or path) and the caught error and emit a clear error message (using the module's logger or console if no logger exists) instead of silently incrementing failed; update the catch to accept the error (catch (err)) and call processLogger.error or console.error with a message like "Failed to process <fileUri>: <err.message>" and increment failed afterward.
🤖 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.
Nitpick comments:
In `@src/workspaceScan.test.ts`:
- Around line 11-17: The tests are missing coverage for scan loop cancellation
and partial failure accounting in scanWorkspace; add targeted unit tests that
mock the async scan loop to simulate a user cancellation and a mix of
file-processing failures, then assert that scanWorkspace returns/throws the
expected cancellation signal and that the final summary counts
(failed/cancelled/processed) and user-facing summary text reflect those
outcomes. Locate and exercise the scanWorkspace function (and reuse
buildScanGlob for input shape) while stubbing the file
iteration/openTextDocument/findFiles behavior (or the provider used by
providers.test.ts) to produce controlled errors and an early cancellation, and
assert the summary string and any returned result object reports the correct
failed and cancelled counts. Ensure tests are isolated and deterministic (use
jest mocks/fakes for async timing) and add them alongside the existing
pure-logic suite.
In `@src/workspaceScan.ts`:
- Around line 92-94: The catch block that only increments failed (in
src/workspaceScan.ts around the code that opens files—look for the loop/handler
using the failed variable, e.g., the function that scans files or processes a
single file) should log per-file error context: capture the file URI/identifier
(the same variable used to open/read the file, e.g., fileUri, uri, or path) and
the caught error and emit a clear error message (using the module's logger or
console if no logger exists) instead of silently incrementing failed; update the
catch to accept the error (catch (err)) and call processLogger.error or
console.error with a message like "Failed to process <fileUri>: <err.message>"
and increment failed afterward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e5789e9-d7fa-41b3-b165-6023d3ab4ee0
📒 Files selected for processing (6)
CHANGELOG.mdpackage.jsonsrc/extension.tssrc/test/integration/activation.test.tssrc/workspaceScan.test.tssrc/workspaceScan.ts
Stacked on scan-workspace-v2 (#19). Two UX improvements from the post-v0.2.0 review. 1. Status bar severity color - pickBackgroundColor returns statusBarItem.errorBackground (red) when CRITICAL is present, statusBarItem.warningBackground (yellow) for HIGH-without-CRITICAL, undefined for MEDIUM/LOW/INFO. Same ThemeColor tokens ESLint and Error Lens use, so the visual language reads correctly across themes. - The default fg colour for medium/low/info keeps a "1 medium" workspace from shouting. 2. What's-new notification (src/whatsNew.ts) - First activation after a version bump shows a one-time toast. "See release notes" opens the matching GitHub release URL via vscode.env.openExternal. - isUpgrade compares the manifest version against the value stashed in globalState; strips a leading 'v' and any pre-release suffix (-rc.1) so a stable release after an rc doesn't re-trigger. - The seen-version persists BEFORE the notification fires, so a missed dismissal doesn't loop next launch. - composeMessage and isUpgrade are pure helpers; the test file pins every documented invariant. Also riding along (user-intentional edits already in the working tree) - package.json: dropped onStartupFinished from activationEvents — the activity-bar slot only appears in CI-relevant workspaces, matching the status bar's already-quieter visibility policy. - package.json: expanded untrustedWorkspaces.description to explain the machine-overridable scope on serverCommand/serverArgs. Better copy for the trust prompt VS Code shows on first open. - README.md: Socket supply-chain badge added. Tests - statusBar.test.ts: 5 new tests for pickBackgroundColor (critical / high-without-critical / medium-only / clean / mixed). Updated the inline vscode stub to expose ThemeColor. - whatsNew.test.ts (new): 17 tests across isUpgrade (first install, major/minor/patch upgrades, downgrade, equal, pre-release strip, leading-v strip, malformed prev), composeMessage (version interpolation, mentions every surface), and showWhatsNewIfUpgraded (shows on first install, skips on match, persists before showing, opens URL on click, doesn't open on dismiss, supports custom openExternal for tests). Total: 129 unit tests (was 107 on PR #19; +5 status bar, +17 whatsNew). Lint, compile, smoke all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
ux: status bar severity color + what's-new notification
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.
Summary
Re-applies the stranded `scan-workspace` work cleanly on top of current main. Closes R10 (refresh-as-scan) and R15 (`onCommand:scanWorkspace` activation — handled implicitly by VS Code's auto-generated activation events from `contributes.commands`).
What lands
New `pipelineCheck.scanWorkspace` command
"Refresh Findings" repurposed (R10)
Used to be a tree-only re-render. Now triggers a real scan, matching the user's mental model: a refresh icon should fetch fresh data, not re-render stale data.
Drift eliminated
`workspaceScan.ts` reads `TRIGGER_PATTERNS` from `providers.ts`. Combined with the existing `documentSelector` and `activationEvents`, all three surfaces now share one list.
Files
Test plan
Notes
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Enhancements
Documentation
Tests