fix(ci): trigger on scripts/, and actually run its 71 orphaned tests (#1263)

[alfredo1996]

Closes #1263 — and corrects its premise.

Two gaps; the second was the real one

CI's path filter omitted scripts/, so a change confined to it skipped CI entirely. That directory holds the plugin and connector import generators that run as predev/prebuild — a break there stops every dev and every build.

More seriously: scripts/__tests__ was run by no workflow at all. 71 tests, including coverage of those generators, had never gated a PR.

Why one command didn't already exist

The directory mixes two runners — three files use vitest, two use Node's built-in node:test. A single vitest run scripts/__tests__ fails on the latter with No test suite found in file.

I chose not to convert them. Those two files currently pass (36 assertions), and converting 59 assert.* calls — 11 of them multi-line — via regex risks quiet damage to green tests for no functional gain. Instead the two runners' file sets are now self-maintaining by naming:

• *.test.mjs → vitest
• *.node-test.mjs → node --test

Adding a file to either set needs no script change. Standardising on one runner is worth doing, but not as a drive-by rewrite.

Correction to my own issue

I wrote that eslint . covers scripts/. It does not. eslint.config.js has a config block only for **/*.{ts,tsx}, so root-level JS matches no block and is silently unlinted — enabling it surfaces 33 problems across 8 files. That is a separate change, filed rather than smuggled in here.

Adding scripts/** to the paths still matters, because it's what makes the new test:scripts step run.

Also fixed

Vitest was discovering stale agent worktrees under .claude/worktrees/ as duplicate test files; now excluded.

Verification

• npm run test:scripts — 3 vitest files / 35 tests, plus 36 node:test assertions
• npm run verify now includes it: 248 + 115 + 28 + 3 vitest files, all green
• Attempted to prove lint catches a scripts/ error and it did not — which is exactly how the eslint-coverage gap above was discovered

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests

  • Expanded automated coverage for plugin import generation, manifest validation, and enterprise setup workflows.
  • Added checks for command-line help, dry-run behavior, error handling, and generated output consistency.

• Chores

  • Updated verification and continuous integration checks to include script-related tests.
  • Ensured changes to scripts and lint configuration are appropriately validated in CI.

[coderabbitai]

Walkthrough

Adds Node-based coverage for plugin generation and enterprise setup scripts, exposes it through npm run test:scripts, includes it in verification, and runs it in CI when scripts or ESLint configuration change.

Changes

Script CI coverage

Layer / File(s)	Summary
Script test command and CI wiring .github/workflows/ci.yml, package.json	The new script test command runs Vitest and Node tests; verification and the lint workflow invoke it, while CI paths include scripts/** and eslint.config.js.
Plugin import generator tests scripts/__tests__/generate-plugin-imports.node-test.mjs	Tests validate plugin entries and manifests, generated imports and mappings, idempotent output, and generator error handling.
Enterprise setup script tests scripts/__tests__/setup-enterprise.node-test.mjs	Tests cover script availability, dry-run and help output, environment guidance, and invalid-flag handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• chore(devex): validate locally before pushing — SonarCloud scanner CLI, CI mirror, CodeRabbit #1253 — Both changes extend verification coverage for repository tooling scripts.

Possibly related PRs

• alfredo1996/neoboard#594 — Covers closely related plugin code-generation validation and behavior.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately reflects the CI/scripts test coverage changes, though it omits the separate ESLint gap.
Linked Issues check	✅ Passed	The PR adds scripts/** and eslint.config.js triggers and runs scripts/tests in verification, covering the linked issue's main CI gaps.
Out of Scope Changes check	✅ Passed	The changes stay focused on CI trigger scope and scripts test execution, with no clear unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-1263-ci-path-filter

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[alfredo1996]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (1)

scripts/__tests__/setup-enterprise.node-test.mjs (1)

11-16: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider adding a timeout to spawnSync calls.

None of the run() invocations specify a timeout, so a future regression that makes the script block (e.g. an interactive prompt) would hang the test until the CI job's overall timeout instead of failing fast with a clear error.

♻️ Suggested tweak

 function run(args = [], env = {}) {
   return spawnSync("bash", [SCRIPT, ...args], {
     encoding: "utf8",
     env: { ...process.env, ...env },
+    timeout: 10_000,
   });
 }

🤖 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 `@scripts/__tests__/setup-enterprise.node-test.mjs` around lines 11 - 16, Add a
finite timeout option to the spawnSync configuration inside run, using the
existing call path for all invocations so blocked scripts fail promptly rather
than hanging CI.

🤖 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 `@scripts/__tests__/setup-enterprise.node-test.mjs`:
- Around line 11-16: Add a finite timeout option to the spawnSync configuration
inside run, using the existing call path for all invocations so blocked scripts
fail promptly rather than hanging CI.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0368a98a-0c6e-4878-834f-08d208ceecbd

📥 Commits

Reviewing files that changed from the base of the PR and between 6cafc74 and 7db1624.

📒 Files selected for processing (4)

• .github/workflows/ci.yml
• package.json
• scripts/__tests__/generate-plugin-imports.node-test.mjs
• scripts/__tests__/setup-enterprise.node-test.mjs