Skip to content

release: v1.0.2 — review-batch bug fixes#33

Merged
dmartinochoa merged 2 commits into
mainfrom
fix/review-batch
May 20, 2026
Merged

release: v1.0.2 — review-batch bug fixes#33
dmartinochoa merged 2 commits into
mainfrom
fix/review-batch

Conversation

@dmartinochoa
Copy link
Copy Markdown
Member

Summary

Three review rounds against the working tree turned up five real bugs and a handful of housekeeping items. All under one PR per request.

Bugs

  • Concurrent-restart race in extension.ts. startClient referenced the module-level client after await startWithTimeout; a parallel stopClient could clear or swap the slot, causing client.onDidChangeState to throw TypeError. Now captures the client in a local and verifies identity after the await.
  • whatsNew lexicographic prerelease compare misranks rc.10 below rc.2 (so an rc.2 user would miss the rc.10 upgrade toast — the exact scenario the existing comment in whatsNew.ts:24-29 says is "the worst possible time to skip it"). Replaced with semver §11.4 per-identifier compare.
  • providers.ts ** glob crosses segment boundaries. **/Dockerfile translated to .*Dockerfile and matched myDockerfile. **/ now emits (?:.*/)? so the prefix must end on a real /.
  • scanOnSave rejection leaks as an unhandled promise. onDidSaveTextDocument is fire-and-forget; a scan failure (workspace closed mid-call, fs error) propagated as a generic extension-host error. Added optional onError hook on ScanOnSaveDeps, wired in extension.ts to clientLog.error.
  • scanWorkspace command rejection lands as "Command 'X' failed" toast. Wrapped both pipelineCheck.scanWorkspace and pipelineCheck.findings.refresh in a runScanCommand helper that logs + shows a Pipeline-Check-branded showErrorMessage.

Housekeeping

  • log.setLogChannel signature widened to OutputChannel | undefined (drops the as unknown as test cast).
  • manifest.test.ts welcome-link regex now captures dotted command IDs (pipelineCheck.findings.refresh etc.), closing a regression-fence gap.
  • workspaceScan.test.ts: renamed the misleading "withProgress throws" test (which actually tested the per-file caught-failure case) and added a sibling that exercises the real pre-loop propagation path via a new __stubFindFilesError stub knob.
  • navigate.test.ts: rewrote the "strict comparison" test to use two findings so it actually verifies advancement (the old single-element setup only proved wrap-around).
  • codeLens.test.ts / findingsView.test.ts: use full resetStubState() in beforeEach for consistency with the other test files — closes a future-trip-hazard where a new test asserting on __stubCalls would inherit stale state.
  • extension.ts: two void-prefix consistency fixes on showInformationMessage.
  • codeql.yml: drops the template scaffolding (same three languages, same pinned action SHAs).

Stats

  • 16 files changed, +321 / −145
  • Unit tests: 245 → 254 (+9 new, covering each bug plus the test gaps)
  • Typecheck, ESLint, and the dogfood pipeline-check action all clean locally

Test plan

  • CI matrix green on ubuntu / windows / macOS
  • Integration suite green on Linux (xvfb-run npm run test:integration)
  • npm audit --omit=dev --audit-level=high clean
  • Dogfood scan finds no new HIGH+ on our own .github/workflows/
  • Spot-check: run Pipeline-Check: Restart Language Server twice in quick succession, confirm no TypeError in the extension-host log
  • Spot-check: in a workspace with dockerfile in disabledProviders, confirm a file literally named myDockerfile is no longer silenced (now correctly classified as not-a-Dockerfile)

🤖 Generated with Claude Code

…an rejection paths

Three review rounds turned up these:

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

Housekeeping:

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

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

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

coderabbitai Bot commented May 20, 2026

Warning

Rate limit exceeded

@dmartinochoa has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 55 minutes and 38 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 69a4b3bf-8940-4a8a-8b09-af35d938b956

📥 Commits

Reviewing files that changed from the base of the PR and between 9bda4b2 and 77d7293.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (18)
  • .github/workflows/codeql.yml
  • CHANGELOG.md
  • package.json
  • src/__testStubs__/vscode.ts
  • src/codeLens.test.ts
  • src/extension.ts
  • src/findingsView.test.ts
  • src/log.test.ts
  • src/log.ts
  • src/manifest.test.ts
  • src/navigate.test.ts
  • src/providers.test.ts
  • src/providers.ts
  • src/scanOnSave.test.ts
  • src/scanOnSave.ts
  • src/whatsNew.test.ts
  • src/whatsNew.ts
  • src/workspaceScan.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/review-batch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Bug-fix batch on top of v1.0.1 — five real defects plus housekeeping
covered by 9 new unit tests (245 → 254). See CHANGELOG for the
detailed entry; the source changes themselves landed earlier in this
branch.

Also bumps package-lock.json's top-level "version" to match
package.json (had drifted at 1.0.0 since the v1.0.1 cut).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dmartinochoa dmartinochoa changed the title fix: review-batch — restart race, semver, glob over-match, scan rejection paths release: v1.0.2 — review-batch bug fixes May 20, 2026
@dmartinochoa dmartinochoa merged commit 8378f73 into main May 20, 2026
12 checks passed
@dmartinochoa dmartinochoa deleted the fix/review-batch branch May 20, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant