build: Flatten scoped shipped plugin paths - #790
Conversation
There was a problem hiding this comment.
Pull request overview
Flattens the on-disk directory layout for shipped plugins under headlamp/.plugins so that scoped package names (e.g. @headlamp-k8s/ai-assistant) are packaged as direct children (e.g. .plugins/ai-assistant), matching Headlamp’s plugin discovery behavior and fixing the missing AI Assistant shipped plugin regression.
Changes:
- Adds a
copyShippedPlugin()helper to package built plugin artifacts into a flattened shipped-plugin directory (and remove the legacy scoped path). - Adds build-script regression tests for the plugin packaging behavior and wires them into CI.
- Adds a root
test:buildscript to run build script tests viatsx.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds test:build script to run build-script unit tests. |
| build/setup-plugins.ts | Switches plugin copying logic to the new helper to flatten shipped plugin paths. |
| build/plugin-packaging.ts | Introduces copyShippedPlugin() to flatten scoped plugin output paths and clean up legacy directories. |
| build/plugin-packaging.test.ts | Adds regression tests ensuring scoped plugins are flattened and unscoped plugins keep their directory name. |
| .github/workflows/ci.yml | Adds a CI job to run the new build-script tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
build/plugin-packaging.ts:23
copyShippedPluginrecreates the target directory but never removes existing contents. If a previous build left files that are no longer emitted bydist/, those stale artifacts will remain in the shipped plugin directory and can cause runtime issues or confusing debugging.
fs.mkdirSync(targetDir, { recursive: true });
package.json:16
test:buildrelies on a shell glob (build/*.test.ts). On Windows (where npm scripts run viacmd.exeby default), this glob typically won’t expand, causing the script to fail to find any test files.
"test:build": "tsx --test build/*.test.ts",
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
build/plugin-packaging.ts:3
- New build/ TypeScript files in this repo typically include the standard copyright/license header and use single quotes for imports/strings (see build/setup-plugins.ts, build/setup-external-tools.ts). Adding the header here and switching the imports to single quotes will keep build tooling consistent.
import * as fs from "fs";
import * as path from "path";
Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
sniok
left a comment
There was a problem hiding this comment.
left a small suggestion, not a blocker though
| return { pluginDir, pluginsDir }; | ||
| } | ||
|
|
||
| test('copies a scoped plugin to a direct shipped-plugin directory', () => { |
There was a problem hiding this comment.
I think instead of separate test file for copyShippedPlugin function it would be better to validate that all the expected plugins got bundled properly in an extra validation step inside setup-plugins.ts, similar to how it's done with external tools in verify-bundled-tools.ts, what do you think?
Scoped plugin names created nested directories under
.plugins, but Headlamp only discovers direct children. This flattens scoped plugin paths while preserving the canonical package identity.Fixes: #787
Changes
.plugins/ai-assistant@headlamp-k8s/ai-assistantinpackage.jsonTesting
npm run test:buildnpm run plugin:setupScreenshot