Add CI validation for OSS assets (fonts and licenses)#8828
Merged
christian-byrne merged 10 commits intomainfrom Feb 23, 2026
Merged
Add CI validation for OSS assets (fonts and licenses)#8828christian-byrne merged 10 commits intomainfrom
christian-byrne merged 10 commits intomainfrom
Conversation
dosubot
bot
added
the
size:L
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/22/2026, 05:35:21 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a CI workflow that validates builds for proprietary ABCROM fonts and enforces a license allowlist, introduces a Vite/Rollup plugin to exclude ABCROM fonts from non-cloud builds, and updates knip ignore configuration to include the new workflow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions (workflow)
participant Runner as CI Runner
participant Setup as Setup (checkout, pnpm, node)
participant Build as Vite/Rollup build
participant Dist as Dist filesystem
participant LicChk as license-checker
GH->>Runner: trigger workflow
Runner->>Setup: checkout + pnpm + node setup
par Validate Fonts
Setup->>Build: run build (DISTRIBUTION=localhost)
Build->>Dist: emit bundle files
Dist->>Runner: scan `dist` for ABCROM fonts
alt ABCROM found or dist missing
Runner->>GH: fail job with remediation message
else
Runner->>GH: pass font validation
end
and Validate Licenses
Setup->>Runner: install dependencies
Runner->>LicChk: run license-checker --allowOnly
alt disallowed licenses found
LicChk->>GH: non-zero exit -> fail with remediation
else
LicChk->>GH: pass license validation
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Playwright: ✅ 524 passed, 0 failed · 5 flaky 📊 Browser Reports
|
📦 Bundle: 4.37 MB gzip 🔴 +1.19 kBDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 21.5 kB) • 🟢 -3.62 kBMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 942 kB (baseline 942 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.8 kB (baseline 68.8 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 10 added / 10 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed Editors & Dialogs — 738 B (baseline 738 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 43.2 kB (baseline 43.2 kB) • ⚪ 0 BReusable component library chunks
Status: 8 added / 8 removed Data & Services — 2.51 MB (baseline 2.51 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 58 kB (baseline 58 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 8.83 MB (baseline 8.83 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.61 MB (baseline 7.61 MB) • 🔴 +5.41 kBBundles that do not match a named category
Status: 59 added / 58 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/ci-oss-assets-validation.yaml:
- Around line 93-132: The script currently enables strict exit-on-error with
"set -euo pipefail" so a non-zero exit from the npx license-checker command will
cause the shell to exit before the custom error/ remediation messages run;
modify the logic so the npx license-checker invocation is executed as the
conditional of the if (e.g., use "if npx license-checker ...; then ... else ...
fi") or temporarily disable "set -e" only around the license-checker call
(save/restore errexit) so the exit status can be tested and the remediation
block after the else (the messages that run when license-checker fails) is
reachable; target the npx license-checker invocation and the set -euo pipefail
line when making the change.
- Line 20: The pnpm/action-setup action is pinned to the wrong commit SHA
(pnpm/action-setup@41ff72655975bd51cab0327fa583b6e92b6d3061) while the intended
version is v4.2.0; update the workflow entry that references pnpm/action-setup
so the pin matches the v4.2.0 commit (9fd676a19091d4595eefd76e4bd31c97133911f1)
or replace the pinned SHA with the v4.2.0 tag itself, ensuring the action
reference and SHA are consistent (look for the string pnpm/action-setup and the
v4.2.0 reference to locate and fix the line).
🧹 Nitpick comments (1)
.github/workflows/ci-oss-assets-validation.yaml (1)
52-54: Nit: redundant-inamepatterns.
-inameis already case-insensitive, so-iname '*ABCROM*' -o -iname '*abcrom*'is equivalent to just-iname '*abcrom*'. Same on line 58.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new GitHub Actions workflow to enforce OSS distribution compliance by validating built dist/ assets (proprietary font detection) and production dependency licenses, and updates Knip config to avoid false positives from the new workflow file.
Changes:
- Add
.github/workflows/ci-oss-assets-validation.yamlwith two jobs: proprietary font scan indist/and production license validation vialicense-checker. - Update
knip.config.tsto ignore the new workflow file (avoids Knip misclassification).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
knip.config.ts |
Ignores the new workflow file to prevent Knip false positives. |
.github/workflows/ci-oss-assets-validation.yaml |
Introduces CI checks for proprietary font artifacts and dependency license compliance. |
Comment on lines
117
to
132
| if [ $? -eq 0 ]; then | ||
| echo '' | ||
| echo '✅ All production dependency licenses are approved!' | ||
| else | ||
| echo '' | ||
| echo '❌ ERROR: Found dependencies with non-approved licenses!' | ||
| echo '' | ||
| echo 'To fix this:' | ||
| echo '1. Check the license of the problematic package' | ||
| echo '2. Find an alternative package with an approved license' | ||
| echo '3. If the license is safe and OSI-approved, add it to the --onlyAllow list' | ||
| echo '' | ||
| echo 'For more info on OSI-approved licenses:' | ||
| echo 'https://opensource.org/licenses' | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
With set -euo pipefail, a non-zero exit from npx license-checker ... will terminate the step immediately, so the custom remediation output in the else branch will never run. If you want the friendly error messaging, run the checker inside the if condition (e.g., if ...; then ... else ... fi) or temporarily disable -e while capturing the exit code.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/platform/cloud/onboarding/components/CloudTemplate.vue`:
- Around line 80-84: The dynamic import in CloudTemplate.vue inside onMounted is
a floating promise; update the onMounted handler to handle errors from
import('../assets/css/fonts.css') by either making the onMounted callback async
and awaiting the import with try/catch or by appending a .catch(...) to the
returned promise; ensure the catch logs a clear error (use the app logger or
console.error) and does not swallow the failure so ESLint no-floating-promises
is satisfied and load failures are observable.
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
- New workflow: ci-oss-assets-validation.yaml - Two jobs: validate-fonts and validate-licenses - Font check: Scans dist for proprietary ABCROM fonts - License check: Validates production dependencies against OSI-approved licenses - Uses simple bash/grep approach following #8623 pattern Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace static CSS @import with dynamic import() to ensure fonts are only loaded at runtime in cloud builds, not bundled at build time. This prevents proprietary ABCROM fonts from being included in the dist output when DISTRIBUTION=localhost. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add the following license types to the validation allowlist: - MIT* (for packages reporting MIT with variations) - (MPL-2.0 OR Apache-2.0) (for dompurify security package) - GPL-3.0-only (for @comfyorg/comfyui-electron-types) - UNLICENSED and UNKNOWN (for internal @comfyorg workspace packages) All added licenses are either OSS-approved or internal workspace packages. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The previous commit had leading spaces in the license strings which caused license-checker to fail matching. This commit puts all licenses on a single line to avoid whitespace issues. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add a generateBundle hook that removes ABCROM font files from the output bundle when DISTRIBUTION is not 'cloud'. This approach works correctly with Vite's asset processing pipeline, unlike the previous dynamic import approach which still caused fonts to be bundled. The CloudTemplate component is reverted to use static CSS @import, as the Vite plugin handles the exclusion at build time. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1. Correct pnpm/action-setup SHA pin to match v4.2.0 - Was: 41ff72655975bd51cab0327fa583b6e92b6d3061 (incorrect) - Now: 9fd676a19091d4595eefd76e4bd31c97133911f1 (correct) 2. Fix error handling with set -e - Move license-checker command into if condition directly - This allows error message to be displayed before exit Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…e-checker - Replace license-checker@25.0.1 (7 years old) with @onebeyond/license-checker@6 - Modern, actively maintained (v6.0.0) - SPDX-compliant license expressions - Cleaner API with --allowOnly flag - Includes --ignoreRootPackageLicense flag Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…upport - Replace @onebeyond/license-checker with license-checker-rseidelsohn@4 - license-checker-rseidelsohn better handles monorepos and non-SPDX licenses - Exclude internal @comfyorg packages from license validation - Allow MIT* for packages like wwobjloader2 - Tested locally: 60 packages validated successfully Licenses approved: - MIT (50), Apache-2.0 (4), ISC (3) - (MPL-2.0 OR Apache-2.0) (1), BSD-2-Clause (1), MIT* (1) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
christian-byrne
force-pushed
the
feat/oss-assets-validation
branch
from
606960e to
6ae2036
Compare
Replace static import of cloudOnboardingRoutes with a conditional dynamic import gated on isCloud. This prevents the entire cloud onboarding module graph (including proprietary ABCROM fonts and CSS) from being included in non-cloud builds at the bundler level. The Vite generateBundle plugin is retained as defense-in-depth.
christian-byrne
previously approved these changes
Feb 22, 2026
Contributor
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
Approval — verified OSS font exclusion is valid and necessary
Verification performed
Quality gates — all pass:
pnpm typecheck✅pnpm lint✅pnpm format:check✅pnpm knip✅pnpm vitest run— 373 files, 4809 tests pass ✅
Font exclusion verified by build:
DISTRIBUTION=localhostbuild on main (no PR changes): ABCROM fonts present indist/(ABCROMExtended-BlackItalic-*.woff,.woff2)DISTRIBUTION=localhostbuild on this branch: ABCROM fonts absent fromdist/✅DISTRIBUTION=cloudbuild on this branch: ABCROM fonts present indist/(correctly included for cloud) ✅
Font usage audit: ABCROM is used exclusively in src/platform/cloud/onboarding/ — zero non-cloud consumers. Only CloudTemplate.vue references font-abcrom / ABC ROM Extended, and only fonts.css declares the @font-face. No other component in the codebase uses this font.
Why the font filtering is genuinely needed
Tree-shaking and DCE do not work on font assets. The root cause: router.ts had a static top-level import { cloudOnboardingRoutes } which forced Vite to follow the full module graph (onboardingCloudRoutes.ts → CloudLayoutView.vue → CloudTemplate.vue → <style> @import fonts.css → url(ABCROMExtended-*.woff2)). CSS @font-face url() references are asset pipeline operations — the bundler copies them into the output unconditionally, regardless of whether the JS code paths that render the component are reachable.
Improvement pushed
Pushed a commit that converts the static import to a conditional dynamic import:
const cloudOnboardingRoutes = isCloud
? (await import("./platform/cloud/onboarding/onboardingCloudRoutes"))
.cloudOnboardingRoutes
: []This fixes the root cause — the entire cloud onboarding module graph (including CSS/fonts) is now excluded from non-cloud builds at the bundler level. Verified: fonts are absent from dist/ with this change alone. The Vite generateBundle plugin is retained as defense-in-depth, and the CI check serves as a regression safety net.
-iname is already case-insensitive, so the duplicate patterns are unnecessary. Addresses CodeRabbit review nitpick. Amp-Thread-ID: https://ampcode.com/threads/T-019c7db3-f255-763a-b8be-646d0796accc
christian-byrne
added
the
preview-cpu
|
Related Documentation Checked 0 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
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
2 participants
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
Adds CI workflow to validate OSS build compliance by checking for proprietary fonts and non-approved dependency licenses.
Context
Implementation
Font Validation
License Validation
license-checkernpm packageWorkflow Details
DISTRIBUTION=localhostfor OSS modeTesting
Related
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
┆Issue is synchronized with this Notion page by Unito