Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/actions/comment-release-links/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ runs:
MARKER='desktop-release-summary'
LINKS_VALUE='npm desktop UI|https://www.npmjs.com/package/@comfyorg/desktop-ui/v/{{version}}'
;;
packages/design-system/package.json)
MARKER='design-system-release-summary'
LINKS_VALUE='npm design-system|https://www.npmjs.com/package/@comfyorg/design-system/v/{{version}}'
;;
esac

DIFF_PREFIX='v'
Expand Down
54 changes: 54 additions & 0 deletions .github/workflows/ci-design-system-pack.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
name: 'CI: Design System Pack Check'

on:
pull_request:
branches-ignore: [wip/*, draft/*, temp/*]
paths:
- 'packages/design-system/**'

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
contents: read

jobs:
verify-pack-contents:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6

- name: Install pnpm
uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0

- name: Setup Node.js
uses: actions/setup-node@v6
with:
node-version-file: '.nvmrc'
cache: 'pnpm'

- name: Install dependencies
run: pnpm install --frozen-lockfile --ignore-scripts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❓ q: This Node/pnpm/install block is, like, basically what .github/actions/setup-frontend already does. If --ignore-scripts is not intentional, replace lines 22–32 with that composite action; otherwise keep this explicit because the shared action changes install behavior.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Intentional — setup-frontend's install step doesn't support --ignore-scripts, and this check only needs typecheck + pack, not a full build, so skipping scripts keeps it fast. Keeping the explicit block here.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How much faster is it, with concrete measurements. @claude

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, speed isn't as important as simplicity for infrequent workflows like this. Don't optimize prematurely.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Retracting the "faster" claim — it was never actually measured, so it wasn't a real justification. Checked what --ignore-scripts was actually skipping here: the only lifecycle script is root prepare (husky install + a git config, both || true), which has no effect on typecheck or pack correctness — so there was no real correctness reason for it either. Switched this job to ./.github/actions/setup-frontend in ea88f49, dropping the duplicated manual block.


Generated by Claude Code


- name: Typecheck
run: pnpm -C packages/design-system typecheck

- name: Verify packed tarball only contains src/css and src/icons
shell: bash
run: |
set -euo pipefail
UNEXPECTED=$(pnpm -C packages/design-system pack --dry-run --json 2>/dev/null \
| node -pe "

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Node script inside a bash script in the yaml...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not uncommon

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mosquitoes aren't either.

const pkg = JSON.parse(require('fs').readFileSync(0, 'utf8'))
pkg.files
.map(f => f.path)
.filter(p => p !== 'package.json' && p !== 'LICENSE' && !p.startsWith('src/css/') && !p.startsWith('src/icons/'))
.join('\n')
")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Like, can we please not nest JavaScript inside Bash inside YAML? jq already ships on the Ubuntu runner and says exactly which paths we reject. Dropping 2>/dev/null also keeps pack failures visible. This is, honestly, way easier to read:

Suggested change
UNEXPECTED=$(pnpm -C packages/design-system pack --dry-run --json 2>/dev/null \
| node -pe "
const pkg = JSON.parse(require('fs').readFileSync(0, 'utf8'))
pkg.files
.map(f => f.path)
.filter(p => p !== 'package.json' && p !== 'LICENSE' && !p.startsWith('src/css/') && !p.startsWith('src/icons/'))
.join('\n')
")
UNEXPECTED=$(
pnpm -C packages/design-system pack --dry-run --json |
jq -r '
.files[].path
| select(
. != "package.json"
and . != "LICENSE"
and (startswith("src/css/") | not)
and (startswith("src/icons/") | not)
)
'
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed — replaced the node -pe JSON parsing with the suggested jq -r pipeline and dropped 2>/dev/null so pack failures stay visible, in ef2aec6.


Generated by Claude Code

if [ -n "$UNEXPECTED" ]; then
echo "::error title=Unexpected files in tarball::Packed tarball contains files outside src/css and src/icons:" >&2
echo "$UNEXPECTED" >&2
exit 1
fi
echo 'Packed tarball contains only expected files.'
113 changes: 113 additions & 0 deletions .github/workflows/publish-design-system-on-merge.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
name: Publish Design System on PR Merge

on:
pull_request:
types: ['closed']
branches: [main, core/*]
paths:
- 'packages/design-system/package.json'

jobs:
resolve:
name: Resolve Version and Dist Tag
runs-on: ubuntu-latest
if: >
github.event.pull_request.merged == true &&
contains(github.event.pull_request.labels.*.name, 'Release')
outputs:
version: ${{ steps.get_version.outputs.version }}
dist_tag: ${{ steps.dist.outputs.dist_tag }}
steps:
- name: Checkout code
uses: actions/checkout@v6
with:
ref: ${{ github.event.pull_request.merge_commit_sha }}
persist-credentials: false

- name: Read design-system version
id: get_version
shell: bash
run: |
set -euo pipefail
VERSION=$(node -p "require('./packages/design-system/package.json').version")
echo "version=$VERSION" >> "$GITHUB_OUTPUT"

- name: Determine dist-tag
id: dist
env:
VERSION: ${{ steps.get_version.outputs.version }}
shell: bash
run: |
set -euo pipefail
if [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+- ]]; then
echo "dist_tag=next" >> "$GITHUB_OUTPUT"
else
echo "dist_tag=latest" >> "$GITHUB_OUTPUT"
fi

publish:
name: Publish Design System to npm
needs: resolve
uses: ./.github/workflows/publish-design-system.yaml
with:
version: ${{ needs.resolve.outputs.version }}
dist_tag: ${{ needs.resolve.outputs.dist_tag }}
ref: ${{ github.event.pull_request.merge_commit_sha }}
secrets:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}

comment_design_system_publish:
name: Comment Design System Publish Summary
needs:
- resolve
- publish
if: success()
runs-on: ubuntu-latest
permissions:
contents: read
issues: write
pull-requests: write
steps:
- name: Checkout merge commit
uses: actions/checkout@v6
with:
ref: ${{ github.event.pull_request.merge_commit_sha }}
fetch-depth: 2

- name: Post design-system release summary comment
id: comment
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
uses: ./.github/actions/comment-release-links
with:
issue-number: ${{ github.event.pull_request.number }}
version_file: packages/design-system/package.json

notify_slack:
name: Notify Slack
needs:
- resolve
- publish
if: success()
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- name: Post to Slack
continue-on-error: true
env:
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
# TODO: replace with the real #product-design channel ID before merging.
SLACK_CHANNEL_ID: 'TODO_PRODUCT_DESIGN_CHANNEL_ID'
VERSION: ${{ needs.resolve.outputs.version }}
PR_URL: ${{ github.event.pull_request.html_url }}
PR_NUMBER: ${{ github.event.pull_request.number }}
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
run: |
set -euo pipefail
TEXT=":package: *@comfyorg/design-system@${VERSION}* published to npm — <${PR_URL}|#${PR_NUMBER}> by ${PR_AUTHOR}. <https://www.npmjs.com/package/@comfyorg/design-system/v/${VERSION}|View on npm>"
BODY=$(jq -n --arg ch "$SLACK_CHANNEL_ID" --arg text "$TEXT" '{channel: $ch, text: $text}')
curl -sf -X POST \
-H "Authorization: Bearer $SLACK_BOT_TOKEN" \
-H "Content-Type: application/json" \
-d "$BODY" \
-o /dev/null \
https://slack.com/api/chat.postMessage
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
161 changes: 161 additions & 0 deletions .github/workflows/publish-design-system.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
name: Publish Design System

on:
workflow_dispatch:
inputs:
version:
description: 'Version to publish (e.g., 1.0.1)'
required: true
type: string
dist_tag:
description: 'npm dist-tag to use'
required: true
default: latest
type: string
ref:
description: 'Git ref to checkout (commit SHA, tag, or branch)'
required: false
type: string
workflow_call:
inputs:
version:
required: true
type: string
dist_tag:
required: false
type: string
default: latest
ref:
required: false
type: string
secrets:
NPM_TOKEN:
required: true

concurrency:
group: publish-design-system-${{ inputs.version }}
cancel-in-progress: false
Comment thread
coderabbitai[bot] marked this conversation as resolved.

jobs:
publish_design_system:
name: Publish @comfyorg/design-system
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- name: Validate version input
env:
VERSION: ${{ inputs.version }}
shell: bash
run: |
set -euo pipefail
SEMVER_REGEX='^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-((0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*))?(\+([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*))?$'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

my brother...

if [[ ! "$VERSION" =~ $SEMVER_REGEX ]]; then
echo "::error title=Invalid version::Version '$VERSION' must follow semantic versioning (x.y.z[-suffix][+build])" >&2
exit 1
fi

- name: Determine ref to checkout
id: resolve_ref
env:
REF: ${{ inputs.ref }}
DEFAULT_REF: ${{ github.ref_name }}
shell: bash
run: |
set -euo pipefail
if [ -z "$REF" ]; then
REF="$DEFAULT_REF"
fi
if ! git check-ref-format --allow-onelevel "$REF"; then
echo "::error title=Invalid ref::Ref '$REF' fails git check-ref-format validation." >&2
exit 1
fi
echo "ref=$REF" >> "$GITHUB_OUTPUT"

- name: Checkout repository
uses: actions/checkout@v6
with:
ref: ${{ steps.resolve_ref.outputs.ref }}
fetch-depth: 1
persist-credentials: false

- name: Install pnpm
uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0

- name: Setup Node.js
uses: actions/setup-node@v6
with:
node-version-file: '.nvmrc'
cache: 'pnpm'
registry-url: https://registry.npmjs.org

- name: Install dependencies
run: pnpm install --frozen-lockfile --ignore-scripts
env:
PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: '1'

- name: Validate dist-tag
env:
DIST_TAG: ${{ inputs.dist_tag }}
shell: bash
run: |
set -euo pipefail
if [ -z "$DIST_TAG" ]; then
echo "::error title=Invalid dist-tag::dist_tag must not be empty" >&2
exit 1
fi
if ! node -e "process.exit(require('semver').validRange(process.argv[1]) ? 1 : 0)" "$DIST_TAG"; then
echo "::error title=Invalid dist-tag::Tag '$DIST_TAG' must not parse as a valid SemVer version or range (npm rejects such tags)" >&2
exit 1
fi

- name: Verify package
id: pkg
env:
INPUT_VERSION: ${{ inputs.version }}
shell: bash
run: |
set -euo pipefail
PACKAGE_JSON=packages/design-system/package.json
NAME=$(node -p "require('./${PACKAGE_JSON}').name")
VERSION=$(node -p "require('./${PACKAGE_JSON}').version")
if [ "$NAME" != "@comfyorg/design-system" ]; then
echo "::error title=Package name mismatch::${PACKAGE_JSON} name '$NAME' is not '@comfyorg/design-system'" >&2
exit 1
fi
if [ "$VERSION" != "$INPUT_VERSION" ]; then
echo "::error title=Version mismatch::${PACKAGE_JSON} version $VERSION does not match input $INPUT_VERSION" >&2
exit 1
fi
echo "name=$NAME" >> "$GITHUB_OUTPUT"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
echo "version=$VERSION" >> "$GITHUB_OUTPUT"

- name: Check if version already on npm
id: check_npm
env:
NAME: ${{ steps.pkg.outputs.name }}
VER: ${{ steps.pkg.outputs.version }}
shell: bash
run: |
set -euo pipefail
STATUS=0
OUTPUT=$(npm view "${NAME}@${VER}" --json 2>&1) || STATUS=$?
if [ "$STATUS" -eq 0 ]; then
echo "exists=true" >> "$GITHUB_OUTPUT"
echo "::warning title=Already published::${NAME}@${VER} already exists on npm. Skipping publish."
else
if echo "$OUTPUT" | grep -q "E404"; then
echo "exists=false" >> "$GITHUB_OUTPUT"
else
echo "::error title=Registry lookup failed::$OUTPUT" >&2
exit "$STATUS"
fi
fi

- name: Publish package
if: steps.check_npm.outputs.exists == 'false'
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
DIST_TAG: ${{ inputs.dist_tag }}
run: pnpm publish --access public --tag "$DIST_TAG" --no-git-checks --ignore-scripts
working-directory: packages/design-system
Loading
Loading