Skip to content

Conversation

@FAIZ1409
Copy link

Adds a CI validation script to ensure all workspace workers are registered in:

  • vitest.workspace.js
  • tsconfig.json references
  • ci.yml deploy job

This enforces docs/adding-new-worker.md automatically.

Fixes #375

Copy link
Contributor

@bajtos bajtos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start! 👏🏻

I have mixed feelings about using string-based detection instead of parsing the config files into JSON or JS objects, but I think this is good enough for start and we can easily improve it later if needed.

Comment on lines +20 to +27
- name: Validate worker registration
run: npm run validate-new-workers
working-directory: worker

- run: npm run lint
- run: npm test


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run the new script as part of npm run lint, so that we can catch errors before opening a pull request.

Comment on lines +28 to +41
// 2. Validate each worker is registered everywhere
for (const worker of workers) {
if (!vitest.includes(worker)) {
fail(`${worker} missing from vitest.workspace.js`);
}

if (!tsconfig.includes(worker)) {
fail(`${worker} missing from tsconfig.json project references (run npm run update-ts-project)`);
}

if (!ci.includes(worker)) {
fail(`${worker} missing from ci.yml deploy job`);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't abort on the first problem, print all issues encountered. At the end, exit with code 0 if no issue was found or code 1 if some issues were detected.

],
"scripts": {
"build:types": "npm run build:types --workspaces --if-present",
"validate-new-workers": "node scripts/validate-new-workers.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We execute this for every CI run, even for workers that are no longer "new". Let's find a better name, please.

For example: check-worker-registries

if (!ci.includes(worker)) {
fail(`${worker} missing from ci.yml deploy job`);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add one more validation step for this part:

5. **`CLAUDE.md`** - Add the new worker to the "Monorepo Structure" list

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a CI-time validation to ensure every npm workspace “worker” is registered consistently across the monorepo (Vitest workspace config, TS project references, and CI workflow), enforcing the steps in docs/adding-new-worker.md (Fixes #375).

Changes:

  • Add scripts/validate-new-workers.js to validate workspace registration across key config files.
  • Add an npm script (validate-new-workers) to run the validation.
  • Invoke the validation from the CI workflow.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
scripts/validate-new-workers.js New validation script that compares package.json workspaces against vitest.workspace.js, tsconfig.json, and .github/workflows/ci.yml.
package.json Adds a script entry to run the validation.
.github/workflows/ci.yml Runs the new validation step during CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +12 to +16
// Root files (script runs inside worker/)
const rootPkg = JSON.parse(read("../package.json"));
const vitest = read("../vitest.workspace.js");
const tsconfig = read("../tsconfig.json");
const ci = read("../.github/workflows/ci.yml");
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script reads root files via "../..." paths, but npm run validate-new-workers runs with the repo root as the working directory (package.json script), so read("../package.json")/etc will resolve outside the repo and fail with ENOENT. Resolve paths relative to the repo root (e.g., process.cwd()) or relative to the script location (import.meta.url + path.resolve).

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a valid point. PTAL, @FAIZ1409

An idea to consider: you can use dynamic imports for JS & JSON files:

await import('../vitest.workspace.js')
await import('../tsconfig.json', with: { type: "json" })

Then you can inspect the objects to verify that each worker is registered at the correct place.

That does not solve the problem of finding the correct path of the GH workflow file, so maybe it's not worth it at this time.

Comment on lines +38 to +40
if (!ci.includes(worker)) {
fail(`${worker} missing from ci.yml deploy job`);
}
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ci.includes(worker) scans the entire workflow file, but the failure message claims the worker is "missing from ci.yml deploy job". This can produce false positives if the worker name appears elsewhere (or as a substring) while still being absent from the deploy job. Consider narrowing the check to the jobs.deploy section (e.g., extract that block and search within it, or parse the YAML and verify deploy step workingDirectory values).

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think such false positives are very unlikely and we don't need to worry about them right now. We can fix this check after it starts reporting false positives.


- name: Validate worker registration
run: npm run validate-new-workers
working-directory: worker
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

working-directory: worker points to a directory that doesn't exist in this repo (the monorepo root contains package.json, scripts/, etc.). This step will fail to locate package.json and run the script. Run it from the repo root (remove working-directory) or set it to the correct path.

Suggested change
working-directory: worker

Copilot uses AI. Check for mistakes.
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.

Flag when a new package was not registered in all places

2 participants