mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-07-16 16:58:33 +00:00
Compare commits
2 Commits
DynamicGro
...
codex/fron
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b221925b32 | ||
|
|
83d8d13bbc |
@@ -22,6 +22,29 @@ reviews:
|
||||
docstrings:
|
||||
mode: 'off'
|
||||
custom_checks:
|
||||
- name: Reproduction and contribution evidence
|
||||
mode: error
|
||||
instructions: |
|
||||
Use only PR metadata already available in the review context: the PR
|
||||
description, changed-file list relative to the base, and diff content.
|
||||
Do not run shell commands or follow links to infer missing evidence.
|
||||
|
||||
Fail when the PR description does not contain concrete reproduction or
|
||||
validation steps that another reviewer can follow.
|
||||
|
||||
The PR must select exactly one evidence type:
|
||||
1. Visual: require either distinct Before and After screenshot links, or
|
||||
one screencast link that demonstrates the before and after behavior.
|
||||
2. Non-visual: allow this only when the diff has no user-visible behavior
|
||||
or presentation change. Require an `N/A:` rationale plus a concrete
|
||||
test command and result or relevant log evidence.
|
||||
|
||||
Fail placeholders, generic statements such as `done` or `tested`, a
|
||||
screenshot used as both Before and After, and Non-visual selections for
|
||||
changes to rendered UI, styling, interaction, or user-visible states.
|
||||
Pass otherwise. The repository metadata workflow validates field shape;
|
||||
this check validates whether the supplied evidence matches the diff.
|
||||
|
||||
- name: End-to-end regression coverage for fixes
|
||||
mode: error
|
||||
instructions: |
|
||||
|
||||
31
.github/ISSUE_TEMPLATE/bug-report.yaml
vendored
31
.github/ISSUE_TEMPLATE/bug-report.yaml
vendored
@@ -39,6 +39,37 @@ body:
|
||||
validations:
|
||||
required: true
|
||||
|
||||
- type: dropdown
|
||||
id: evidence-type
|
||||
attributes:
|
||||
label: Evidence type
|
||||
description: Choose the evidence that supports this report.
|
||||
options:
|
||||
- Visual
|
||||
- Non-visual
|
||||
validations:
|
||||
required: true
|
||||
|
||||
- type: textarea
|
||||
id: evidence
|
||||
attributes:
|
||||
label: Evidence
|
||||
description: >-
|
||||
Visual reports require distinct before and after links or one screencast.
|
||||
For non-visual reports, explain why visuals do not apply and include test
|
||||
or log output.
|
||||
placeholder: |
|
||||
Visual:
|
||||
Before: Drag in a screenshot.
|
||||
After: Drag in a screenshot.
|
||||
Or Screencast: Drag in one recording that shows both states.
|
||||
|
||||
Non-visual:
|
||||
N/A: Explain why visual evidence does not apply.
|
||||
Test/log evidence: Paste the command and result, logs, or a link.
|
||||
validations:
|
||||
required: true
|
||||
|
||||
- type: dropdown
|
||||
id: severity
|
||||
attributes:
|
||||
|
||||
40
.github/ISSUE_TEMPLATE/feature-request.yaml
vendored
40
.github/ISSUE_TEMPLATE/feature-request.yaml
vendored
@@ -29,6 +29,46 @@ body:
|
||||
- While collaborating with team members...
|
||||
validations:
|
||||
required: true
|
||||
- type: textarea
|
||||
id: example
|
||||
attributes:
|
||||
label: Concrete example or steps
|
||||
description: Show the exact workflow or steps where the problem occurs.
|
||||
placeholder: |
|
||||
1. Open a workflow with...
|
||||
2. Try to...
|
||||
3. Observe...
|
||||
validations:
|
||||
required: true
|
||||
- type: dropdown
|
||||
id: evidence-type
|
||||
attributes:
|
||||
label: Evidence type
|
||||
description: Choose the evidence that supports this request.
|
||||
options:
|
||||
- Visual
|
||||
- Non-visual
|
||||
validations:
|
||||
required: true
|
||||
- type: textarea
|
||||
id: evidence
|
||||
attributes:
|
||||
label: Evidence
|
||||
description: >-
|
||||
Visual requests require distinct before and after links or one screencast.
|
||||
For non-visual requests, explain why visuals do not apply and include test
|
||||
or log output.
|
||||
placeholder: |
|
||||
Visual:
|
||||
Before: Drag in a screenshot.
|
||||
After: Drag in a screenshot.
|
||||
Or Screencast: Drag in one recording that shows both states.
|
||||
|
||||
Non-visual:
|
||||
N/A: Explain why visual evidence does not apply.
|
||||
Test/log evidence: Paste the command and result, logs, or a link.
|
||||
validations:
|
||||
required: true
|
||||
- type: dropdown
|
||||
id: frequency
|
||||
attributes:
|
||||
|
||||
33
.github/pull_request_template.md
vendored
33
.github/pull_request_template.md
vendored
@@ -15,6 +15,35 @@
|
||||
<!-- If this PR fixes an issue, uncomment and update the line below -->
|
||||
<!-- Fixes #ISSUE_NUMBER -->
|
||||
|
||||
## Screenshots (if applicable)
|
||||
## Reproduction or validation steps
|
||||
|
||||
<!-- Add screenshots or video recording to help explain your changes -->
|
||||
<!-- Required. List the exact steps a reviewer can follow. -->
|
||||
|
||||
1.
|
||||
|
||||
## Evidence type
|
||||
|
||||
<!-- Required. Check exactly one. -->
|
||||
|
||||
- [ ] Visual
|
||||
- [ ] Non-visual
|
||||
|
||||
## Before
|
||||
|
||||
<!-- Visual: add a screenshot link. Leave blank when using a screencast. -->
|
||||
|
||||
## After
|
||||
|
||||
<!-- Visual: add a screenshot link. Leave blank when using a screencast. -->
|
||||
|
||||
## Screencast
|
||||
|
||||
<!-- Visual alternative: add one link that demonstrates before and after. -->
|
||||
|
||||
## Non-visual rationale
|
||||
|
||||
<!-- Non-visual only. Start with "N/A:" and explain why visuals do not apply. -->
|
||||
|
||||
## Test or log evidence
|
||||
|
||||
<!-- Non-visual only. Paste the command and result, logs, or a link. -->
|
||||
|
||||
@@ -41,6 +41,20 @@ jobs:
|
||||
title: '[chore] Update electron-types to ${{ steps.get-version.outputs.NEW_VERSION }}'
|
||||
body: |
|
||||
Automated update of desktop API types to version ${{ steps.get-version.outputs.NEW_VERSION }}.
|
||||
|
||||
## Reproduction or validation steps
|
||||
1. Review the package and lockfile changes for the expected version.
|
||||
2. Confirm the desktop API type checks pass in CI.
|
||||
|
||||
## Evidence type
|
||||
- [ ] Visual
|
||||
- [x] Non-visual
|
||||
|
||||
## Non-visual rationale
|
||||
N/A: This automated dependency update has no visual behavior.
|
||||
|
||||
## Test or log evidence
|
||||
`pnpm install --workspace-root @comfyorg/comfyui-electron-types@latest` passed in workflow run `${{ github.run_id }}`.
|
||||
branch: update-electron-types-${{ steps.get-version.outputs.NEW_VERSION }}
|
||||
base: main
|
||||
labels: |
|
||||
|
||||
@@ -98,6 +98,20 @@ jobs:
|
||||
- Generated on: ${{ github.event.repository.updated_at }}
|
||||
|
||||
These types are automatically generated using openapi-typescript.
|
||||
|
||||
## Reproduction or validation steps
|
||||
1. Review the generated type diff against the referenced Manager commit.
|
||||
2. Confirm the generated type lint and repository checks pass in CI.
|
||||
|
||||
## Evidence type
|
||||
- [ ] Visual
|
||||
- [x] Non-visual
|
||||
|
||||
## Non-visual rationale
|
||||
N/A: This automated generated-type update has no visual behavior.
|
||||
|
||||
## Test or log evidence
|
||||
`pnpm lint:fix:no-cache -- ./src/types/generatedManagerTypes.ts` passed in workflow run `${{ github.run_id }}`.
|
||||
branch: update-manager-types-${{ steps.manager-info.outputs.commit }}
|
||||
base: ${{ inputs.target_branch }}
|
||||
labels: Manager
|
||||
|
||||
117
.github/workflows/ci-issue-evidence.yaml
vendored
Normal file
117
.github/workflows/ci-issue-evidence.yaml
vendored
Normal file
@@ -0,0 +1,117 @@
|
||||
# Description: Labels and reports issues blocked by missing evidence or a Comfy human owner.
|
||||
name: 'CI: Issue Evidence'
|
||||
|
||||
on:
|
||||
issues:
|
||||
types: [opened, edited, reopened, assigned, unassigned]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
issues: write
|
||||
|
||||
concurrency:
|
||||
group: issue-evidence-${{ github.event.issue.number }}
|
||||
cancel-in-progress: true
|
||||
|
||||
jobs:
|
||||
issue-evidence:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout default-branch validator
|
||||
uses: actions/checkout@v6
|
||||
with:
|
||||
ref: ${{ github.workflow_sha }}
|
||||
|
||||
- name: Set up Node.js
|
||||
uses: actions/setup-node@v6
|
||||
with:
|
||||
node-version-file: '.nvmrc'
|
||||
|
||||
- name: Validate issue evidence
|
||||
id: evidence
|
||||
continue-on-error: true
|
||||
run: node scripts/cicd/validate-evidence.ts issue "$GITHUB_EVENT_PATH"
|
||||
|
||||
- name: Validate human maintainer assignment
|
||||
id: maintainer
|
||||
continue-on-error: true
|
||||
env:
|
||||
COMFY_ORG_MEMBERS_READ_TOKEN: ${{ secrets.COMFY_ORG_MEMBERS_READ_TOKEN }}
|
||||
run: node scripts/cicd/validate-maintainer.ts "$GITHUB_EVENT_PATH"
|
||||
|
||||
- name: Update blocked labels
|
||||
if: always()
|
||||
uses: actions/github-script@v8
|
||||
env:
|
||||
EVIDENCE_VALID: ${{ steps.evidence.outputs.valid }}
|
||||
MAINTAINER_VALID: ${{ steps.maintainer.outputs.valid }}
|
||||
with:
|
||||
script: |
|
||||
const issue_number = context.issue.number
|
||||
const { owner, repo } = context.repo
|
||||
const policies = [
|
||||
{
|
||||
description: 'Blocked until required reproduction and evidence details are added',
|
||||
name: 'blocked: needs-evidence',
|
||||
valid: process.env.EVIDENCE_VALID === 'true'
|
||||
},
|
||||
{
|
||||
description: 'Blocked until an active human comfy_frontend_devs member is assigned',
|
||||
name: 'blocked: needs-maintainer',
|
||||
valid: process.env.MAINTAINER_VALID === 'true'
|
||||
}
|
||||
]
|
||||
|
||||
for (const policy of policies) {
|
||||
if (policy.valid) {
|
||||
try {
|
||||
await github.rest.issues.removeLabel({
|
||||
owner,
|
||||
repo,
|
||||
issue_number,
|
||||
name: policy.name
|
||||
})
|
||||
} catch (error) {
|
||||
if (error.status !== 404) throw error
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
try {
|
||||
await github.rest.issues.getLabel({
|
||||
owner,
|
||||
repo,
|
||||
name: policy.name
|
||||
})
|
||||
} catch (error) {
|
||||
if (error.status !== 404) throw error
|
||||
await github.rest.issues.createLabel({
|
||||
owner,
|
||||
repo,
|
||||
name: policy.name,
|
||||
color: 'B60205',
|
||||
description: policy.description
|
||||
}).catch((createError) => {
|
||||
const alreadyExists =
|
||||
createError.status === 422 &&
|
||||
createError.response?.data?.errors?.some(
|
||||
(detail) => detail.code === 'already_exists'
|
||||
)
|
||||
if (!alreadyExists) throw createError
|
||||
})
|
||||
}
|
||||
|
||||
await github.rest.issues.addLabels({
|
||||
owner,
|
||||
repo,
|
||||
issue_number,
|
||||
labels: [policy.name]
|
||||
})
|
||||
}
|
||||
|
||||
- name: Fail blocked contribution
|
||||
if: >-
|
||||
always() &&
|
||||
(steps.evidence.outputs.valid != 'true' ||
|
||||
steps.maintainer.outputs.valid != 'true')
|
||||
run: exit 1
|
||||
67
.github/workflows/ci-pr-evidence.yaml
vendored
Normal file
67
.github/workflows/ci-pr-evidence.yaml
vendored
Normal file
@@ -0,0 +1,67 @@
|
||||
# Description: Enforces PR evidence and a Comfy human owner with trusted base-branch code.
|
||||
name: 'CI: PR Evidence'
|
||||
|
||||
on:
|
||||
pull_request_target:
|
||||
types:
|
||||
[
|
||||
opened,
|
||||
edited,
|
||||
synchronize,
|
||||
reopened,
|
||||
ready_for_review,
|
||||
assigned,
|
||||
unassigned
|
||||
]
|
||||
merge_group:
|
||||
types: [checks_requested]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: read
|
||||
|
||||
concurrency:
|
||||
group: pr-evidence-${{ github.event.pull_request.number || github.event.merge_group.head_sha }}
|
||||
cancel-in-progress: true
|
||||
|
||||
jobs:
|
||||
pr-evidence:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout trusted validator
|
||||
uses: actions/checkout@v6
|
||||
with:
|
||||
ref: ${{ github.workflow_sha }}
|
||||
|
||||
- name: Set up Node.js
|
||||
uses: actions/setup-node@v6
|
||||
with:
|
||||
node-version-file: '.nvmrc'
|
||||
|
||||
- name: Validate human maintainer assignment
|
||||
id: maintainer
|
||||
if: github.event_name == 'pull_request_target'
|
||||
continue-on-error: true
|
||||
env:
|
||||
COMFY_ORG_MEMBERS_READ_TOKEN: ${{ secrets.COMFY_ORG_MEMBERS_READ_TOKEN }}
|
||||
run: node scripts/cicd/validate-maintainer.ts "$GITHUB_EVENT_PATH"
|
||||
|
||||
- name: Validate pull request evidence
|
||||
id: evidence
|
||||
if: github.event_name == 'pull_request_target'
|
||||
continue-on-error: true
|
||||
run: node scripts/cicd/validate-evidence.ts pr "$GITHUB_EVENT_PATH"
|
||||
|
||||
- name: Enforce contribution policy
|
||||
if: >-
|
||||
github.event_name == 'pull_request_target' &&
|
||||
(steps.maintainer.outputs.valid != 'true' ||
|
||||
steps.evidence.outputs.valid != 'true')
|
||||
run: exit 1
|
||||
|
||||
- name: Validate current merge group metadata
|
||||
if: github.event_name == 'merge_group'
|
||||
env:
|
||||
COMFY_ORG_MEMBERS_READ_TOKEN: ${{ secrets.COMFY_ORG_MEMBERS_READ_TOKEN }}
|
||||
GITHUB_TOKEN: ${{ github.token }}
|
||||
run: node scripts/cicd/validate-merge-group.ts "$GITHUB_EVENT_PATH"
|
||||
14
.github/workflows/i18n-update-nodes.yaml
vendored
14
.github/workflows/i18n-update-nodes.yaml
vendored
@@ -49,6 +49,20 @@ jobs:
|
||||
Automated PR to update locales for node definitions
|
||||
|
||||
This PR was created automatically by the frontend update workflow.
|
||||
|
||||
## Reproduction or validation steps
|
||||
1. Review the locale diff for the expected node-definition strings.
|
||||
2. Confirm locale generation and repository checks pass in CI.
|
||||
|
||||
## Evidence type
|
||||
- [ ] Visual
|
||||
- [x] Non-visual
|
||||
|
||||
## Non-visual rationale
|
||||
N/A: This automated locale data update has no visual behavior.
|
||||
|
||||
## Test or log evidence
|
||||
`pnpm locale` passed in workflow run `${{ github.run_id }}`.
|
||||
branch: update-locales-node-defs-${{ github.event.inputs.trigger_type }}-${{ github.run_id }}
|
||||
base: main
|
||||
labels: dependencies
|
||||
|
||||
16
.github/workflows/pr-backport.yaml
vendored
16
.github/workflows/pr-backport.yaml
vendored
@@ -348,22 +348,34 @@ jobs:
|
||||
run: |
|
||||
# Get PR data for manual triggers
|
||||
if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
|
||||
PR_DATA=$(gh pr view ${{ inputs.pr_number }} --json title,author)
|
||||
PR_DATA=$(gh pr view ${{ inputs.pr_number }} --json title,author,body)
|
||||
PR_TITLE=$(echo "$PR_DATA" | jq -r '.title')
|
||||
PR_AUTHOR=$(echo "$PR_DATA" | jq -r '.author.login')
|
||||
PR_SOURCE_BODY=$(echo "$PR_DATA" | jq -r '.body // ""')
|
||||
else
|
||||
PR_TITLE=$(jq -r '.pull_request.title' "$GITHUB_EVENT_PATH")
|
||||
PR_AUTHOR=$(jq -r '.pull_request.user.login' "$GITHUB_EVENT_PATH")
|
||||
PR_SOURCE_BODY=$(jq -r '.pull_request.body // ""' "$GITHUB_EVENT_PATH")
|
||||
fi
|
||||
|
||||
for backport in ${{ steps.backport.outputs.success }}; do
|
||||
IFS=':' read -r target branch <<< "${backport}"
|
||||
|
||||
PR_BODY=$(cat <<EOF
|
||||
Backport of #${PR_NUMBER} to \`${target}\`.
|
||||
|
||||
Automatically created by the backport workflow.
|
||||
|
||||
## Source pull request evidence
|
||||
${PR_SOURCE_BODY}
|
||||
EOF
|
||||
)
|
||||
|
||||
if PR_URL=$(gh pr create \
|
||||
--base "${target}" \
|
||||
--head "${branch}" \
|
||||
--title "[backport ${target}] ${PR_TITLE}" \
|
||||
--body "Backport of #${PR_NUMBER} to \`${target}\`"$'\n\n'"Automatically created by backport workflow." \
|
||||
--body "${PR_BODY}" \
|
||||
--label "backport" 2>&1); then
|
||||
|
||||
# Extract PR number from URL
|
||||
|
||||
14
.github/workflows/release-version-bump.yaml
vendored
14
.github/workflows/release-version-bump.yaml
vendored
@@ -202,6 +202,20 @@ jobs:
|
||||
${{ steps.capitalised.outputs.capitalised }} version increment to ${{ steps.bump-version.outputs.NEW_VERSION }}
|
||||
|
||||
**Base branch:** `${{ steps.prepared-inputs.outputs.branch }}`
|
||||
|
||||
## Reproduction or validation steps
|
||||
1. Review the package and lockfile version changes.
|
||||
2. Confirm release checks pass for the target branch in CI.
|
||||
|
||||
## Evidence type
|
||||
- [ ] Visual
|
||||
- [x] Non-visual
|
||||
|
||||
## Non-visual rationale
|
||||
N/A: This automated version bump has no visual behavior.
|
||||
|
||||
## Test or log evidence
|
||||
`pnpm version ${{ steps.prepared-inputs.outputs.version_type }} --no-git-tag-version` passed in workflow run `${{ github.run_id }}`.
|
||||
branch: version-bump-${{ steps.bump-version.outputs.NEW_VERSION }}
|
||||
base: ${{ steps.prepared-inputs.outputs.branch }}
|
||||
labels: |
|
||||
|
||||
14
.github/workflows/release-website.yaml
vendored
14
.github/workflows/release-website.yaml
vendored
@@ -63,6 +63,20 @@ jobs:
|
||||
snapshot (with a warning annotation in CI).
|
||||
|
||||
Triggered by workflow run `${{ github.run_id }}`.
|
||||
|
||||
## Reproduction or validation steps
|
||||
1. Review both regenerated snapshot diffs for expected remote data.
|
||||
2. Confirm the Vercel website preview builds successfully.
|
||||
|
||||
## Evidence type
|
||||
- [ ] Visual
|
||||
- [x] Non-visual
|
||||
|
||||
## Non-visual rationale
|
||||
N/A: This automated snapshot refresh changes data files only.
|
||||
|
||||
## Test or log evidence
|
||||
Status: success for snapshot generation in workflow run `${{ github.run_id }}`.
|
||||
branch: chore/refresh-website-snapshots-${{ github.run_id }}
|
||||
base: main
|
||||
labels: |
|
||||
|
||||
14
.github/workflows/version-bump-desktop-ui.yaml
vendored
14
.github/workflows/version-bump-desktop-ui.yaml
vendored
@@ -86,6 +86,20 @@ jobs:
|
||||
${{ steps.capitalised.outputs.capitalised }} version increment for @comfyorg/desktop-ui to ${{ steps.bump-version.outputs.NEW_VERSION }}
|
||||
|
||||
**Base branch:** `${{ github.event.inputs.branch }}`
|
||||
|
||||
## Reproduction or validation steps
|
||||
1. Review the desktop UI package and lockfile version changes.
|
||||
2. Confirm desktop UI release checks pass for the target branch in CI.
|
||||
|
||||
## Evidence type
|
||||
- [ ] Visual
|
||||
- [x] Non-visual
|
||||
|
||||
## Non-visual rationale
|
||||
N/A: This automated package version bump has no visual behavior.
|
||||
|
||||
## Test or log evidence
|
||||
`pnpm -C apps/desktop-ui version ${{ github.event.inputs.version_type }} --no-git-tag-version` passed in workflow run `${{ github.run_id }}`.
|
||||
branch: desktop-ui-version-bump-${{ steps.bump-version.outputs.NEW_VERSION }}
|
||||
base: ${{ github.event.inputs.branch }}
|
||||
labels: |
|
||||
|
||||
21
.github/workflows/weekly-docs-check.yaml
vendored
21
.github/workflows/weekly-docs-check.yaml
vendored
@@ -84,6 +84,7 @@ jobs:
|
||||
- ## Review Notes section with any important context
|
||||
3. Be specific about which files were updated and why
|
||||
4. If no changes were needed, write a brief message stating documentation is up to date
|
||||
5. Do not add evidence-contract headings; the workflow appends those sections
|
||||
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
|
||||
claude_args: "--max-turns 256 --allowedTools 'Bash(git status),Bash(git diff),Bash(git log),Bash(pnpm:*),Bash(npm:*),Bash(node:*),Bash(tsc:*),Bash(echo:*),Read,Write,Edit,Glob,Grep'"
|
||||
continue-on-error: false
|
||||
@@ -127,6 +128,26 @@ jobs:
|
||||
EOF
|
||||
fi
|
||||
|
||||
- name: Add required PR evidence
|
||||
if: steps.check_changes.outputs.has_changes == 'true'
|
||||
run: |
|
||||
cat >> /tmp/pr-body-${{ github.run_id }}.md <<'EOF'
|
||||
|
||||
## Reproduction or validation steps
|
||||
1. Review each documentation diff against the current implementation.
|
||||
2. Confirm referenced commands and links work as documented.
|
||||
|
||||
## Evidence type
|
||||
- [ ] Visual
|
||||
- [x] Non-visual
|
||||
|
||||
## Non-visual rationale
|
||||
N/A: This automated review changes documentation only.
|
||||
|
||||
## Test or log evidence
|
||||
Status: success. The documentation review completed before this PR was created.
|
||||
EOF
|
||||
|
||||
- name: Create or Update Pull Request
|
||||
if: steps.check_changes.outputs.has_changes == 'true'
|
||||
uses: peter-evans/create-pull-request@c0f553fe549906ede9cf27b5156039d195d2ece0 # v8.1.0
|
||||
|
||||
@@ -258,8 +258,11 @@ The original litegraph repository (https://github.com/Comfy-Org/litegraph.js) is
|
||||
|
||||
1. Ensure your branch is up to date with main
|
||||
2. Run all tests and ensure they pass
|
||||
3. Create a pull request with a clear title and description
|
||||
4. Use conventional commit format for PR titles:
|
||||
3. Create a pull request with a clear title, description, and exact reproduction or validation steps
|
||||
4. Include contribution evidence:
|
||||
- For visual behavior or presentation changes, add distinct Before and After screenshots or one screencast that demonstrates both states.
|
||||
- For a truly non-visual change, select Non-visual, add an `N/A:` rationale, and include the test command and result or relevant logs.
|
||||
5. Use conventional commit format for PR titles:
|
||||
- `feat:` for new features
|
||||
- `fix:` for bug fixes
|
||||
- `docs:` for documentation
|
||||
@@ -270,9 +273,51 @@ The original litegraph repository (https://github.com/Comfy-Org/litegraph.js) is
|
||||
### Review Process
|
||||
|
||||
1. All PRs require at least one review
|
||||
2. Address review feedback promptly
|
||||
3. Keep PRs focused - one feature/fix per PR
|
||||
4. Large features should be discussed in an issue first
|
||||
2. Issues and pull requests under active execution require an active human member of the Comfy-Org `comfy_frontend_devs` team as an assignee. Automation and service accounts are executors, not accountable owners. Choose the person with the most context, using CODEOWNERS and recent changes to the related code path. The policy preserves existing assignees; COMOS performs contextual selection for artifacts it creates.
|
||||
3. Address review feedback promptly
|
||||
4. Keep PRs focused - one feature/fix per PR
|
||||
5. Large features should be discussed in an issue first
|
||||
|
||||
The `pr-evidence` check fails until both the evidence contract and human owner
|
||||
contract are satisfied. Issues receive `blocked: needs-evidence` and
|
||||
`blocked: needs-maintainer` labels as applicable, and the policy workflow fails
|
||||
until both conditions are fixed. Opaque GitHub attachment links in Screencast
|
||||
fields are fetched without credentials and accepted only when GitHub serves
|
||||
video content or an animated GIF. A static screenshot labeled as a screencast
|
||||
does not satisfy the check.
|
||||
|
||||
Repository administrators must configure the
|
||||
`COMFY_ORG_MEMBERS_READ_TOKEN` Actions secret before enabling `pr-evidence` in
|
||||
the ProtectMain or Core release branch rulesets. Use a dedicated fine-grained
|
||||
token with `Comfy-Org` organization `Members: read` permission, including team
|
||||
membership reads, and no repository write permission. The trusted workflows use
|
||||
it to verify the assignee's GitHub user type, active organization membership,
|
||||
and active `comfy_frontend_devs` team membership. They fail closed when the
|
||||
secret or any lookup is unavailable. Merge the workflow to the default branch
|
||||
and verify a successful check before making it required.
|
||||
|
||||
#### Contribution policy rollout
|
||||
|
||||
Do not make `pr-evidence` required when the workflow first lands. Use this
|
||||
sequence so existing open contributions are not blocked without warning:
|
||||
|
||||
1. Configure and verify `COMFY_ORG_MEMBERS_READ_TOKEN`.
|
||||
2. Merge the policy workflows while `pr-evidence` is not a required check. New
|
||||
and updated contributions still report failures and blocked labels, which is
|
||||
the audit signal.
|
||||
3. Observe a small canary across manual, fork, and automated contributions.
|
||||
Confirm assignment edits rerun the check and valid edits clear issue labels.
|
||||
4. Audit and backfill the active pull request and issue queues with evidence and
|
||||
a contextual Comfy-Org human assignee.
|
||||
5. Add `pr-evidence` to ProtectMain ruleset `991238` only after the active main
|
||||
queue is ready.
|
||||
6. Test a canary against both `core/**` and `cloud/**`. Add `pr-evidence` to Core
|
||||
release branches ruleset `7297873` only after both targets are confirmed to
|
||||
trigger the trusted workflow. Confirm the merge-group check refetches and
|
||||
validates current PR bodies and assignees before enabling the ruleset.
|
||||
|
||||
The workflow is intentionally fail closed during the audit phase. It remains
|
||||
non-blocking only because the rulesets do not require its check yet.
|
||||
|
||||
## Questions?
|
||||
|
||||
|
||||
@@ -1,9 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import {
|
||||
validateComfyNodeDef,
|
||||
zDynamicGroupInputSpec
|
||||
} from '../schemas/nodeDefSchema'
|
||||
import { validateComfyNodeDef } from '../schemas/nodeDefSchema'
|
||||
import type { ComfyNodeDef } from '../schemas/nodeDefSchema'
|
||||
|
||||
const EXAMPLE_NODE_DEF: ComfyNodeDef = {
|
||||
@@ -68,42 +65,3 @@ describe('validateNodeDef', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('zDynamicGroupInputSpec', () => {
|
||||
const template = { required: { a: ['STRING', {}] } }
|
||||
|
||||
it('rejects min greater than max', () => {
|
||||
expect(
|
||||
zDynamicGroupInputSpec.safeParse([
|
||||
'COMFY_DYNAMICGROUP_V3',
|
||||
{ template, min: 60, max: 50 }
|
||||
]).success
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts min equal to max', () => {
|
||||
expect(
|
||||
zDynamicGroupInputSpec.safeParse([
|
||||
'COMFY_DYNAMICGROUP_V3',
|
||||
{ template, min: 3, max: 3 }
|
||||
]).success
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('applies default min and max', () => {
|
||||
const parsed = zDynamicGroupInputSpec.parse([
|
||||
'COMFY_DYNAMICGROUP_V3',
|
||||
{ template }
|
||||
])
|
||||
expect(parsed[1].min).toBe(0)
|
||||
expect(parsed[1].max).toBe(50)
|
||||
})
|
||||
|
||||
it('accepts an optional group_name', () => {
|
||||
const parsed = zDynamicGroupInputSpec.parse([
|
||||
'COMFY_DYNAMICGROUP_V3',
|
||||
{ template, group_name: 'Lora' }
|
||||
])
|
||||
expect(parsed[1].group_name).toBe('Lora')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -344,26 +344,6 @@ export const zDynamicComboInputSpec = z.tuple([
|
||||
})
|
||||
])
|
||||
|
||||
export const zDynamicGroupInputSpec = z.tuple([
|
||||
z.literal('COMFY_DYNAMICGROUP_V3'),
|
||||
zBaseInputOptions
|
||||
.extend({
|
||||
template: zComfyInputsSpec,
|
||||
min: z.number().int().nonnegative().optional().default(0),
|
||||
max: z.number().int().positive().max(100).optional().default(50),
|
||||
group_name: z.string().optional()
|
||||
})
|
||||
.superRefine((data, ctx) => {
|
||||
if (data.min > data.max) {
|
||||
ctx.addIssue({
|
||||
code: 'custom',
|
||||
message: 'min must be less than or equal to max',
|
||||
path: ['min']
|
||||
})
|
||||
}
|
||||
})
|
||||
])
|
||||
|
||||
export const zMatchTypeOptions = z.object({
|
||||
...zBaseInputOptions.shape,
|
||||
type: z.literal('COMFY_MATCHTYPE_V3'),
|
||||
|
||||
151
scripts/cicd/evidence-workflows.test.ts
Normal file
151
scripts/cicd/evidence-workflows.test.ts
Normal file
@@ -0,0 +1,151 @@
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { resolve } from 'node:path'
|
||||
import { runInNewContext } from 'node:vm'
|
||||
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
function workflow(name: string): string {
|
||||
return readFileSync(resolve('.github/workflows', name), 'utf8')
|
||||
}
|
||||
|
||||
function issueLabelScript(): string {
|
||||
const source = workflow('ci-issue-evidence.yaml')
|
||||
const marker = ' script: |\n'
|
||||
const scriptAndFollowingSteps = source.slice(
|
||||
source.indexOf(marker) + marker.length
|
||||
)
|
||||
return scriptAndFollowingSteps
|
||||
.split('\n - name:')[0]
|
||||
.replace(/^ {12}/gm, '')
|
||||
}
|
||||
|
||||
const automatedFrontendPrWorkflows = [
|
||||
'api-update-electron-api-types.yaml',
|
||||
'api-update-manager-api-types.yaml',
|
||||
'i18n-update-nodes.yaml',
|
||||
'release-version-bump.yaml',
|
||||
'release-website.yaml',
|
||||
'version-bump-desktop-ui.yaml',
|
||||
'weekly-docs-check.yaml'
|
||||
]
|
||||
|
||||
describe('evidence workflows', () => {
|
||||
it('runs the trusted validator from the workflow commit', () => {
|
||||
const source = workflow('ci-pr-evidence.yaml')
|
||||
expect(source).toContain('ref: ${{ github.workflow_sha }}')
|
||||
expect(source).toContain(' assigned,\n unassigned')
|
||||
expect(source).toContain(
|
||||
'COMFY_ORG_MEMBERS_READ_TOKEN: ${{ secrets.COMFY_ORG_MEMBERS_READ_TOKEN }}'
|
||||
)
|
||||
expect(source).toContain('node scripts/cicd/validate-maintainer.ts')
|
||||
expect(source).toContain('steps.maintainer.outputs.valid')
|
||||
expect(source).toContain("github.event_name == 'merge_group'")
|
||||
expect(source).toContain('node scripts/cicd/validate-merge-group.ts')
|
||||
expect(source).not.toContain('Evidence was validated before')
|
||||
expect(source).toContain('cancel-in-progress: true')
|
||||
})
|
||||
|
||||
it('rechecks issue evidence and ownership when assignments change', () => {
|
||||
const source = workflow('ci-issue-evidence.yaml')
|
||||
expect(source).toContain('assigned, unassigned')
|
||||
expect(source).toContain('ref: ${{ github.workflow_sha }}')
|
||||
expect(source).toContain('blocked: needs-evidence')
|
||||
expect(source).toContain('blocked: needs-maintainer')
|
||||
expect(source).toContain('Fail blocked contribution')
|
||||
expect(source).toContain(
|
||||
'COMFY_ORG_MEMBERS_READ_TOKEN: ${{ secrets.COMFY_ORG_MEMBERS_READ_TOKEN }}'
|
||||
)
|
||||
})
|
||||
|
||||
it.for(automatedFrontendPrWorkflows)(
|
||||
'%s generates a structurally complete evidence body',
|
||||
(name) => {
|
||||
const source = workflow(name)
|
||||
expect(source).toContain('## Reproduction or validation steps')
|
||||
expect(source).toContain('## Evidence type')
|
||||
expect(source).toContain('## Non-visual rationale')
|
||||
expect(source).toContain('## Test or log evidence')
|
||||
}
|
||||
)
|
||||
|
||||
it('backports retain the validated source pull request evidence', () => {
|
||||
const source = workflow('pr-backport.yaml')
|
||||
expect(source).toContain('PR_SOURCE_BODY=')
|
||||
expect(source).toContain('## Source pull request evidence')
|
||||
expect(source).toContain('${PR_SOURCE_BODY}')
|
||||
})
|
||||
|
||||
it('adds the issue label after another run wins the creation race', async () => {
|
||||
const addLabels = vi.fn()
|
||||
const github = {
|
||||
rest: {
|
||||
issues: {
|
||||
addLabels,
|
||||
createLabel: vi.fn().mockRejectedValue({
|
||||
response: { data: { errors: [{ code: 'already_exists' }] } },
|
||||
status: 422
|
||||
}),
|
||||
getLabel: vi.fn().mockRejectedValue({ status: 404 }),
|
||||
removeLabel: vi.fn()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
await runInNewContext(`(async () => {${issueLabelScript()}})()`, {
|
||||
context: {
|
||||
issue: { number: 42 },
|
||||
repo: { owner: 'Comfy-Org', repo: 'ComfyUI_frontend' }
|
||||
},
|
||||
github,
|
||||
process: {
|
||||
env: { EVIDENCE_VALID: 'false', MAINTAINER_VALID: 'true' }
|
||||
}
|
||||
})
|
||||
|
||||
expect(addLabels).toHaveBeenCalledWith({
|
||||
issue_number: 42,
|
||||
labels: ['blocked: needs-evidence'],
|
||||
owner: 'Comfy-Org',
|
||||
repo: 'ComfyUI_frontend'
|
||||
})
|
||||
})
|
||||
|
||||
it('tracks missing maintainers separately from missing evidence', async () => {
|
||||
const addLabels = vi.fn()
|
||||
const removeLabel = vi.fn().mockRejectedValue({ status: 404 })
|
||||
const github = {
|
||||
rest: {
|
||||
issues: {
|
||||
addLabels,
|
||||
createLabel: vi.fn().mockResolvedValue({}),
|
||||
getLabel: vi.fn().mockResolvedValue({}),
|
||||
removeLabel
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
await runInNewContext(`(async () => {${issueLabelScript()}})()`, {
|
||||
context: {
|
||||
issue: { number: 42 },
|
||||
repo: { owner: 'Comfy-Org', repo: 'ComfyUI_frontend' }
|
||||
},
|
||||
github,
|
||||
process: {
|
||||
env: { EVIDENCE_VALID: 'true', MAINTAINER_VALID: 'false' }
|
||||
}
|
||||
})
|
||||
|
||||
expect(removeLabel).toHaveBeenCalledWith({
|
||||
issue_number: 42,
|
||||
name: 'blocked: needs-evidence',
|
||||
owner: 'Comfy-Org',
|
||||
repo: 'ComfyUI_frontend'
|
||||
})
|
||||
expect(addLabels).toHaveBeenCalledWith({
|
||||
issue_number: 42,
|
||||
labels: ['blocked: needs-maintainer'],
|
||||
owner: 'Comfy-Org',
|
||||
repo: 'ComfyUI_frontend'
|
||||
})
|
||||
})
|
||||
})
|
||||
684
scripts/cicd/validate-evidence.test.ts
Normal file
684
scripts/cicd/validate-evidence.test.ts
Normal file
@@ -0,0 +1,684 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import {
|
||||
probeGitHubScreencast,
|
||||
validateIssueEvidence,
|
||||
validateIssueEvidenceWithMedia,
|
||||
validatePullRequestEvidence,
|
||||
validatePullRequestEvidenceWithMedia
|
||||
} from './validate-evidence'
|
||||
import type { MediaResponse } from './validate-evidence'
|
||||
|
||||
function issueBody({
|
||||
evidence,
|
||||
heading = 'Steps to Reproduce',
|
||||
mode = 'Visual',
|
||||
reproduction = '1. Open ComfyUI\n2. Queue the workflow'
|
||||
}: {
|
||||
evidence: string
|
||||
heading?: string
|
||||
mode?: string
|
||||
reproduction?: string
|
||||
}): string {
|
||||
return `## ${heading}
|
||||
${reproduction}
|
||||
|
||||
## Evidence type
|
||||
${mode}
|
||||
|
||||
## Evidence
|
||||
${evidence}`
|
||||
}
|
||||
|
||||
function pullRequestBody({
|
||||
after = '',
|
||||
before = '',
|
||||
mode = 'Visual',
|
||||
rationale = '',
|
||||
screencast = '',
|
||||
steps = '1. Open the queue\n2. Reproduce the change',
|
||||
testEvidence = ''
|
||||
}: {
|
||||
after?: string
|
||||
before?: string
|
||||
mode?: 'Both' | 'Neither' | 'Non-visual' | 'Visual'
|
||||
rationale?: string
|
||||
screencast?: string
|
||||
steps?: string
|
||||
testEvidence?: string
|
||||
}): string {
|
||||
const visual = mode === 'Visual' || mode === 'Both' ? 'x' : ' '
|
||||
const nonVisual = mode === 'Non-visual' || mode === 'Both' ? 'x' : ' '
|
||||
return `## Reproduction or validation steps
|
||||
${steps}
|
||||
|
||||
## Evidence type
|
||||
- [${visual}] Visual
|
||||
- [${nonVisual}] Non-visual
|
||||
|
||||
## Before
|
||||
${before}
|
||||
|
||||
## After
|
||||
${after}
|
||||
|
||||
## Screencast
|
||||
${screencast}
|
||||
|
||||
## Non-visual rationale
|
||||
${rationale}
|
||||
|
||||
## Test or log evidence
|
||||
${testEvidence}`
|
||||
}
|
||||
|
||||
function mediaResponse(
|
||||
status: number,
|
||||
headers: Record<string, string> = {}
|
||||
): MediaResponse {
|
||||
const normalized = new Map(
|
||||
Object.entries(headers).map(([name, value]) => [name.toLowerCase(), value])
|
||||
)
|
||||
return {
|
||||
headers: { get: (name) => normalized.get(name.toLowerCase()) ?? null },
|
||||
ok: status >= 200 && status < 300,
|
||||
status
|
||||
}
|
||||
}
|
||||
|
||||
describe('validateIssueEvidence', () => {
|
||||
it('accepts visual evidence with reproduction steps', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'Before: https://github.com/user-attachments/assets/11111111-1111-4111-8111-111111111111\nAfter: https://github.com/user-attachments/assets/22222222-2222-4222-8222-222222222222'
|
||||
})
|
||||
)
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('accepts standard GitHub markdown images for before and after', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'Before: \nAfter: '
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('accepts one visual screencast', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({ evidence: 'https://example.com/change.webm' })
|
||||
)
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('accepts a content-verified GitHub screencast attachment', async () => {
|
||||
const result = await validateIssueEvidenceWithMedia(
|
||||
issueBody({
|
||||
evidence:
|
||||
'Screencast: https://github.com/user-attachments/assets/77777777-7777-4777-8777-777777777777'
|
||||
}),
|
||||
vi.fn().mockResolvedValue(true)
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects a screenshot attachment labeled as an issue screencast', async () => {
|
||||
const result = await validateIssueEvidenceWithMedia(
|
||||
issueBody({
|
||||
evidence:
|
||||
'Screencast: https://github.com/user-attachments/assets/77777777-7777-4777-8777-777777777777'
|
||||
}),
|
||||
vi.fn().mockResolvedValue(false)
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts a non-visual rationale with test evidence', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'N/A: The failure is an API response.\nTest/log evidence: `pnpm test` reproduces the timeout.',
|
||||
heading: 'Concrete example or steps',
|
||||
mode: 'Non-visual'
|
||||
})
|
||||
)
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects empty numbered reproduction steps', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'https://github.com/user-attachments/assets/33333333-3333-4333-8333-333333333333',
|
||||
reproduction: '1.\n2.\n3.'
|
||||
})
|
||||
)
|
||||
expect(result.errors).toContain(
|
||||
'Add concrete reproduction steps or an example.'
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects visual evidence without a link', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({ evidence: 'The button is visibly broken.' })
|
||||
)
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects a generic URL that is not visual media', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({ evidence: 'https://example.com/evidence' })
|
||||
)
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects one screenshot without before and after states', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'https://github.com/user-attachments/assets/44444444-4444-4444-8444-444444444444'
|
||||
})
|
||||
)
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects two unlabeled screenshots because before and after are ambiguous', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'https://github.com/user-attachments/assets/55555555-5555-4555-8555-555555555555\nhttps://github.com/user-attachments/assets/66666666-6666-4666-8666-666666666666'
|
||||
})
|
||||
)
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects non-visual evidence without tests or logs', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence: 'N/A: This changes internal state only.',
|
||||
mode: 'Non-visual'
|
||||
})
|
||||
)
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects GitHub URLs outside the exact attachment asset path', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'Before: https://github.com/user-attachments/not-assets/before\nAfter: https://github.com/user-attachments/assets/after/extra'
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects encoded path separators in GitHub attachment IDs', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'Before: https://github.com/user-attachments/assets/11111111-1111-4111-8111-111111111111\nAfter: https://github.com/user-attachments/assets/22222222-2222-4222-8222-222222222222%2Fextra'
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects a fabricated non-UUID GitHub attachment asset', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'Before: https://github.com/user-attachments/assets/before\nAfter: https://github.com/user-attachments/assets/after'
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects an image URL labeled as a screencast', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({ evidence: 'Screencast: https://example.com/change.png' })
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('canonicalizes labeled before and after URLs before comparing them', () => {
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence:
|
||||
'Before: https://example.com/change.png?state=before\nAfter: https://EXAMPLE.com/change.png?state=after#result'
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects an HTTP alias for the same GitHub attachment', () => {
|
||||
const attachment =
|
||||
'github.com/user-attachments/assets/11111111-1111-4111-8111-111111111111'
|
||||
const result = validateIssueEvidence(
|
||||
issueBody({
|
||||
evidence: `Before: http://${attachment}\nAfter: https://${attachment}`
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('validatePullRequestEvidence', () => {
|
||||
it('ignores required headings hidden inside an HTML comment', () => {
|
||||
const hidden = `Context 🧪 <!--
|
||||
## Reproduction or validation steps
|
||||
1. Hidden valid step
|
||||
2. Hidden second step
|
||||
|
||||
## Evidence type
|
||||
- [x] Visual
|
||||
- [ ] Non-visual
|
||||
|
||||
## Screencast
|
||||
https://example.com/hidden.webm
|
||||
-->`
|
||||
const visible = pullRequestBody({
|
||||
mode: 'Non-visual',
|
||||
rationale: 'N/A: This changes CI metadata only.',
|
||||
testEvidence: '`pnpm test:unit` passed.'
|
||||
})
|
||||
|
||||
expect(validatePullRequestEvidence(`${hidden}\n${visible}`).valid).toBe(
|
||||
true
|
||||
)
|
||||
expect(validatePullRequestEvidence(hidden).valid).toBe(false)
|
||||
})
|
||||
|
||||
it('ignores required headings inside a fenced code sample', () => {
|
||||
const fenced = `\`\`\`markdown
|
||||
## Reproduction or validation steps
|
||||
1. Hidden valid step
|
||||
2. Hidden second step
|
||||
|
||||
## Evidence type
|
||||
- [x] Visual
|
||||
- [ ] Non-visual
|
||||
|
||||
## Screencast
|
||||
https://example.com/hidden.webm
|
||||
\`\`\``
|
||||
|
||||
expect(validatePullRequestEvidence(fenced).valid).toBe(false)
|
||||
})
|
||||
|
||||
it('preserves visible fenced test and log evidence', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
mode: 'Non-visual',
|
||||
rationale: 'N/A: This changes CI metadata only.',
|
||||
testEvidence: '```text\nStatus: success\n```'
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('accepts before and after links', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
after:
|
||||
'https://github.com/user-attachments/assets/22222222-2222-4222-8222-222222222222',
|
||||
before:
|
||||
'https://github.com/user-attachments/assets/11111111-1111-4111-8111-111111111111'
|
||||
})
|
||||
)
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('accepts one content-verified GitHub screencast', async () => {
|
||||
const result = await validatePullRequestEvidenceWithMedia(
|
||||
pullRequestBody({
|
||||
screencast:
|
||||
'https://github.com/user-attachments/assets/77777777-7777-4777-8777-777777777777'
|
||||
}),
|
||||
vi.fn().mockResolvedValue(true)
|
||||
)
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects a GitHub screencast when the media probe fails', async () => {
|
||||
const result = await validatePullRequestEvidenceWithMedia(
|
||||
pullRequestBody({
|
||||
screencast:
|
||||
'https://github.com/user-attachments/assets/77777777-7777-4777-8777-777777777777'
|
||||
}),
|
||||
vi.fn().mockRejectedValue(new Error('probe unavailable'))
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects an image-markdown attachment in the screencast field', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
screencast:
|
||||
''
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts a non-visual rationale with test evidence', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
mode: 'Non-visual',
|
||||
rationale: 'N/A: This only changes CI metadata.',
|
||||
testEvidence: '`pnpm test:unit` passed.'
|
||||
})
|
||||
)
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects selecting both evidence modes', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({ mode: 'Both' })
|
||||
)
|
||||
expect(result.errors).toContain(
|
||||
'Check exactly one evidence type: Visual or Non-visual.'
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects selecting neither evidence mode', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({ mode: 'Neither' })
|
||||
)
|
||||
expect(result.errors).toContain(
|
||||
'Check exactly one evidence type: Visual or Non-visual.'
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects template placeholders', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
mode: 'Non-visual',
|
||||
rationale: 'N/A:',
|
||||
steps: '1.',
|
||||
testEvidence: 'TBD'
|
||||
})
|
||||
)
|
||||
expect(result.valid).toBe(false)
|
||||
expect(result.errors).toHaveLength(3)
|
||||
})
|
||||
|
||||
it('rejects single-word validation and log placeholders', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
mode: 'Non-visual',
|
||||
rationale: 'N/A: This only changes CI metadata.',
|
||||
steps: 'done',
|
||||
testEvidence: 'test'
|
||||
})
|
||||
)
|
||||
expect(result.errors).toContain('Add reproduction or validation steps.')
|
||||
expect(result.errors).toContain(
|
||||
'Non-visual pull requests require test or log evidence.'
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects a generic all-tests-passed claim without a command or log', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
mode: 'Non-visual',
|
||||
rationale: 'N/A: This only changes CI metadata.',
|
||||
testEvidence: 'All tests passed.'
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.errors).toContain(
|
||||
'Non-visual pull requests require test or log evidence.'
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects generic URLs as before and after visual evidence', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
after: 'https://example.com/after',
|
||||
before: 'https://example.com/before'
|
||||
})
|
||||
)
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('requires distinct before and after visual evidence', () => {
|
||||
const attachment =
|
||||
'https://github.com/user-attachments/assets/88888888-8888-4888-8888-888888888888'
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({ after: attachment, before: attachment })
|
||||
)
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('canonicalizes before and after URLs before comparing them', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
after: 'https://EXAMPLE.com/change.png?state=after#result',
|
||||
before: 'https://example.com/change.png?state=before'
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('treats a trailing slash as the same visual evidence URL', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
after: 'https://www.loom.com/share/same-recording/',
|
||||
before: 'https://www.loom.com/share/same-recording'
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects an HTTP alias for the same before and after attachment', () => {
|
||||
const attachment =
|
||||
'github.com/user-attachments/assets/88888888-8888-4888-8888-888888888888'
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({
|
||||
after: `https://${attachment}`,
|
||||
before: `http://${attachment}`
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects an image-only URL in the screencast field', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({ screencast: 'https://example.com/change.png' })
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts an explicit video URL in the screencast field', () => {
|
||||
const result = validatePullRequestEvidence(
|
||||
pullRequestBody({ screencast: 'https://example.com/change.webm' })
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('probeGitHubScreencast', () => {
|
||||
const attachment =
|
||||
'https://github.com/user-attachments/assets/77777777-7777-4777-8777-777777777777'
|
||||
|
||||
it('rejects non-GitHub attachment URLs without making a request', async () => {
|
||||
const request = vi.fn()
|
||||
|
||||
await expect(
|
||||
probeGitHubScreencast('https://example.com/demo.mp4', request)
|
||||
).resolves.toBe(false)
|
||||
expect(request).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('follows a manual GitHub redirect and accepts video content', async () => {
|
||||
const request = vi.fn().mockResolvedValueOnce(
|
||||
mediaResponse(302, {
|
||||
location:
|
||||
'https://github-production-user-asset-6210df.s3.amazonaws.com/assets/demo.mp4?response-content-type=video%2Fmp4'
|
||||
})
|
||||
)
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(true)
|
||||
expect(request).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
attachment,
|
||||
expect.objectContaining({ method: 'HEAD', redirect: 'manual' })
|
||||
)
|
||||
expect(request).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('accepts an animated GIF content type', async () => {
|
||||
const request = vi
|
||||
.fn()
|
||||
.mockResolvedValue(mediaResponse(200, { 'content-type': 'image/gif' }))
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(true)
|
||||
})
|
||||
|
||||
it('rejects a static screenshot attachment', async () => {
|
||||
const request = vi
|
||||
.fn()
|
||||
.mockResolvedValue(mediaResponse(200, { 'content-type': 'image/png' }))
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects a GitHub redirect that identifies a static PNG', async () => {
|
||||
const request = vi.fn().mockResolvedValueOnce(
|
||||
mediaResponse(302, {
|
||||
location:
|
||||
'https://github-production-user-asset-6210df.s3.amazonaws.com/assets/demo.png?response-content-type=image%2Fpng'
|
||||
})
|
||||
)
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(
|
||||
false
|
||||
)
|
||||
expect(request).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('uses a range GET when an opaque signed redirect rejects HEAD', async () => {
|
||||
const request = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(
|
||||
mediaResponse(302, {
|
||||
location:
|
||||
'https://github-production-user-asset-6210df.s3.amazonaws.com/assets/demo'
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce(mediaResponse(403))
|
||||
.mockResolvedValueOnce(
|
||||
mediaResponse(206, { 'content-type': 'video/mp4' })
|
||||
)
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(true)
|
||||
expect(request).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
expect.stringContaining('github-production-user-asset-6210df'),
|
||||
expect.objectContaining({ method: 'GET', redirect: 'manual' })
|
||||
)
|
||||
})
|
||||
|
||||
it.for([405, 501])(
|
||||
'uses a range GET when the initial HEAD returns %i',
|
||||
async (status) => {
|
||||
const request = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(mediaResponse(status))
|
||||
.mockResolvedValueOnce(
|
||||
mediaResponse(206, { 'content-type': 'video/mp4' })
|
||||
)
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(
|
||||
true
|
||||
)
|
||||
expect(request).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
attachment,
|
||||
expect.objectContaining({ method: 'GET', redirect: 'manual' })
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
it('does not retry an initial 403 response with GET', async () => {
|
||||
const request = vi.fn().mockResolvedValue(mediaResponse(403))
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(
|
||||
false
|
||||
)
|
||||
expect(request).toHaveBeenCalledTimes(1)
|
||||
expect(request).toHaveBeenCalledWith(
|
||||
attachment,
|
||||
expect.objectContaining({ method: 'HEAD', redirect: 'manual' })
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects a redirect without a location header', async () => {
|
||||
const request = vi.fn().mockResolvedValue(mediaResponse(302))
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(
|
||||
false
|
||||
)
|
||||
expect(request).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it.for([
|
||||
'http://github-production-user-asset-6210df.s3.amazonaws.com/assets/demo',
|
||||
'https://user:password@github-production-user-asset-6210df.s3.amazonaws.com/assets/demo'
|
||||
])('rejects an unsafe redirect target: %s', async (location) => {
|
||||
const request = vi.fn().mockResolvedValue(mediaResponse(302, { location }))
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(
|
||||
false
|
||||
)
|
||||
expect(request).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('rejects a redirect chain after five hops', async () => {
|
||||
const request = vi.fn().mockResolvedValue(
|
||||
mediaResponse(302, {
|
||||
location:
|
||||
'https://github-production-user-asset-6210df.s3.amazonaws.com/assets/next'
|
||||
})
|
||||
)
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(
|
||||
false
|
||||
)
|
||||
expect(request).toHaveBeenCalledTimes(5)
|
||||
})
|
||||
|
||||
it('rejects redirects outside GitHub media hosts', async () => {
|
||||
const request = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(
|
||||
mediaResponse(302, { location: 'https://example.com/video.mp4' })
|
||||
)
|
||||
|
||||
await expect(probeGitHubScreencast(attachment, request)).resolves.toBe(
|
||||
false
|
||||
)
|
||||
expect(request).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
709
scripts/cicd/validate-evidence.ts
Normal file
709
scripts/cicd/validate-evidence.ts
Normal file
@@ -0,0 +1,709 @@
|
||||
import { appendFileSync, readFileSync } from 'node:fs'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
|
||||
type EvidenceKind = 'issue' | 'pr'
|
||||
|
||||
type EventPayload = {
|
||||
issue?: { body?: string | null }
|
||||
pull_request?: { body?: string | null }
|
||||
}
|
||||
|
||||
export type PullRequestEvidencePayload = {
|
||||
body?: string | null
|
||||
number?: number | null
|
||||
}
|
||||
|
||||
export type ScreencastProbe = (url: string) => Promise<boolean>
|
||||
|
||||
export type MediaResponse = {
|
||||
headers: { get(name: string): string | null }
|
||||
ok: boolean
|
||||
status: number
|
||||
}
|
||||
|
||||
export type MediaRequest = (
|
||||
url: string,
|
||||
init: {
|
||||
headers?: Record<string, string>
|
||||
method: 'GET' | 'HEAD'
|
||||
redirect: 'manual'
|
||||
}
|
||||
) => Promise<MediaResponse>
|
||||
|
||||
export type EvidenceValidation = {
|
||||
errors: string[]
|
||||
valid: boolean
|
||||
}
|
||||
|
||||
const EMPTY_VALUE = /^(?:-|n\/a|none|not applicable|tbd|todo)$/i
|
||||
|
||||
function clean(value: string): string {
|
||||
return stripHtmlCommentsOutsideFences(value).trim()
|
||||
}
|
||||
|
||||
function spaces(value: string): string {
|
||||
return value.replace(/[^\r\n]/g, ' ')
|
||||
}
|
||||
|
||||
function maskHtmlComments(
|
||||
value: string,
|
||||
startsInsideComment: boolean
|
||||
): { masked: string; remainsInsideComment: boolean } {
|
||||
const masked = value.split('')
|
||||
let inComment = startsInsideComment
|
||||
let offset = 0
|
||||
|
||||
while (offset < value.length) {
|
||||
if (inComment) {
|
||||
const end = value.indexOf('-->', offset)
|
||||
const stop = end === -1 ? value.length : end + 3
|
||||
masked.fill(' ', offset, stop)
|
||||
offset = stop
|
||||
if (end === -1) break
|
||||
inComment = false
|
||||
continue
|
||||
}
|
||||
|
||||
const start = value.indexOf('<!--', offset)
|
||||
if (start === -1) break
|
||||
masked.fill(' ', start, start + 4)
|
||||
offset = start + 4
|
||||
inComment = true
|
||||
}
|
||||
|
||||
return { masked: masked.join(''), remainsInsideComment: inComment }
|
||||
}
|
||||
|
||||
function maskHiddenHeadingRegions(body: string): string {
|
||||
let inComment = false
|
||||
let fence: { character: '`' | '~'; length: number } | null = null
|
||||
|
||||
return body
|
||||
.split(/(?<=\n)/)
|
||||
.map((line) => {
|
||||
const ending = line.endsWith('\r\n')
|
||||
? '\r\n'
|
||||
: line.endsWith('\n')
|
||||
? '\n'
|
||||
: ''
|
||||
const content = ending ? line.slice(0, -ending.length) : line
|
||||
|
||||
if (fence) {
|
||||
const closing = new RegExp(
|
||||
`^ {0,3}\\${fence.character}{${fence.length},}[ \\t]*$`
|
||||
).test(content)
|
||||
if (closing) fence = null
|
||||
return `${spaces(content)}${ending}`
|
||||
}
|
||||
|
||||
const commentResult = maskHtmlComments(content, inComment)
|
||||
inComment = commentResult.remainsInsideComment
|
||||
const opening = commentResult.masked.match(/^ {0,3}(`{3,}|~{3,})/)
|
||||
if (opening) {
|
||||
const marker = opening[1]
|
||||
fence = {
|
||||
character: marker[0] as '`' | '~',
|
||||
length: marker.length
|
||||
}
|
||||
return `${spaces(content)}${ending}`
|
||||
}
|
||||
|
||||
return `${commentResult.masked}${ending}`
|
||||
})
|
||||
.join('')
|
||||
}
|
||||
|
||||
function stripHtmlCommentsOutsideFences(body: string): string {
|
||||
let inComment = false
|
||||
let fence: { character: '`' | '~'; length: number } | null = null
|
||||
|
||||
return body
|
||||
.split(/(?<=\n)/)
|
||||
.map((line) => {
|
||||
const ending = line.endsWith('\r\n')
|
||||
? '\r\n'
|
||||
: line.endsWith('\n')
|
||||
? '\n'
|
||||
: ''
|
||||
const content = ending ? line.slice(0, -ending.length) : line
|
||||
|
||||
if (fence) {
|
||||
const closing = new RegExp(
|
||||
`^ {0,3}\\${fence.character}{${fence.length},}[ \\t]*$`
|
||||
).test(content)
|
||||
if (closing) fence = null
|
||||
return line
|
||||
}
|
||||
|
||||
const commentResult = maskHtmlComments(content, inComment)
|
||||
inComment = commentResult.remainsInsideComment
|
||||
const opening = commentResult.masked.match(/^ {0,3}(`{3,}|~{3,})/)
|
||||
if (opening) {
|
||||
const marker = opening[1]
|
||||
fence = {
|
||||
character: marker[0] as '`' | '~',
|
||||
length: marker.length
|
||||
}
|
||||
}
|
||||
return `${commentResult.masked}${ending}`
|
||||
})
|
||||
.join('')
|
||||
}
|
||||
|
||||
function section(body: string, heading: string): string {
|
||||
const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
|
||||
const headingSearch = maskHiddenHeadingRegions(body)
|
||||
const match = new RegExp(
|
||||
`^ {0,3}#{2,6}[ \\t]+${escapedHeading}[ \\t]*#*[ \\t]*(?:\\r?\\n|$)`,
|
||||
'im'
|
||||
).exec(headingSearch)
|
||||
if (!match) return ''
|
||||
|
||||
const contentStart = match.index + match[0].length
|
||||
const nextHeading = /^ {0,3}#{2,6}[ \t]+/gm
|
||||
nextHeading.lastIndex = contentStart
|
||||
const nextMatch = nextHeading.exec(headingSearch)
|
||||
return clean(body.slice(contentStart, nextMatch?.index ?? body.length))
|
||||
}
|
||||
|
||||
function hasContent(value: string): boolean {
|
||||
const content = clean(value)
|
||||
.replace(/^\s*(?:[-*+]\s*|\d+\.\s*)$/gm, '')
|
||||
.replace(/^\s*```[\w-]*\s*$/gm, '')
|
||||
.trim()
|
||||
|
||||
return content.length >= 4 && !EMPTY_VALUE.test(content)
|
||||
}
|
||||
|
||||
function hasProcedure(value: string): boolean {
|
||||
const content = clean(value)
|
||||
if (!hasContent(content)) return false
|
||||
|
||||
const listedSteps = content
|
||||
.split(/\r?\n/)
|
||||
.filter((line) => /^\s*(?:[-*+]\s+|\d+[.)]\s+)/.test(line))
|
||||
.map((line) => line.replace(/^\s*(?:[-*+]\s+|\d+[.)]\s+)/, '').trim())
|
||||
.filter((line) => line.length >= 4 && !EMPTY_VALUE.test(line))
|
||||
|
||||
if (listedSteps.length >= 2) return true
|
||||
|
||||
const words = content.match(/[\p{L}\p{N}_-]+/gu) ?? []
|
||||
return content.length >= 20 && words.length >= 5
|
||||
}
|
||||
|
||||
function isGitHubAttachmentUrl(host: string, path: string): boolean {
|
||||
return (
|
||||
host === 'github.com' &&
|
||||
/^\/user-attachments\/assets\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/.test(
|
||||
path
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
function isOpaqueGitHubAttachmentUrl(rawUrl: string): boolean {
|
||||
try {
|
||||
const url = new URL(rawUrl)
|
||||
return (
|
||||
url.protocol === 'https:' &&
|
||||
!url.username &&
|
||||
!url.password &&
|
||||
isGitHubAttachmentUrl(
|
||||
url.hostname.toLowerCase(),
|
||||
url.pathname.toLowerCase()
|
||||
)
|
||||
)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
function isLoomUrl(host: string, path: string): boolean {
|
||||
return (
|
||||
(host === 'loom.com' || host === 'www.loom.com') &&
|
||||
path.startsWith('/share/')
|
||||
)
|
||||
}
|
||||
|
||||
function isVisualMediaUrl(rawUrl: string): boolean {
|
||||
try {
|
||||
const url = new URL(rawUrl)
|
||||
if (url.protocol !== 'https:' || url.username || url.password) return false
|
||||
const host = url.hostname.toLowerCase()
|
||||
const path = url.pathname.toLowerCase()
|
||||
|
||||
return (
|
||||
isGitHubAttachmentUrl(host, path) ||
|
||||
host === 'user-images.githubusercontent.com' ||
|
||||
/\.(?:gif|jpe?g|mov|mp4|png|webm|webp)$/.test(path) ||
|
||||
isLoomUrl(host, path)
|
||||
)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
function isExplicitScreencastUrl(rawUrl: string): boolean {
|
||||
try {
|
||||
const url = new URL(rawUrl)
|
||||
if (url.protocol !== 'https:' || url.username || url.password) return false
|
||||
const host = url.hostname.toLowerCase()
|
||||
const path = url.pathname.toLowerCase()
|
||||
return /\.(?:gif|mov|mp4|webm)$/.test(path) || isLoomUrl(host, path)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
function isAllowedGitHubMediaHost(host: string): boolean {
|
||||
return (
|
||||
host === 'github.com' ||
|
||||
host === 'user-images.githubusercontent.com' ||
|
||||
host.endsWith('.githubusercontent.com') ||
|
||||
/^github-production-user-asset-[a-z0-9-]+\.s3\.amazonaws\.com$/.test(host)
|
||||
)
|
||||
}
|
||||
|
||||
function isRedirectStatus(status: number): boolean {
|
||||
return [301, 302, 303, 307, 308].includes(status)
|
||||
}
|
||||
|
||||
function redirectMediaVerdict(url: URL): boolean | null {
|
||||
const hintedContentType =
|
||||
url.searchParams.get('response-content-type')?.toLowerCase() ?? ''
|
||||
if (
|
||||
hintedContentType.startsWith('video/') ||
|
||||
hintedContentType === 'image/gif'
|
||||
) {
|
||||
return true
|
||||
}
|
||||
if (hintedContentType.startsWith('image/')) return false
|
||||
|
||||
const path = url.pathname.toLowerCase()
|
||||
if (/\.(?:gif|mov|mp4|webm)$/.test(path)) return true
|
||||
if (/\.(?:jpe?g|png|webp)$/.test(path)) return false
|
||||
return null
|
||||
}
|
||||
|
||||
function isVerifiedScreencastResponse(
|
||||
url: URL,
|
||||
response: MediaResponse
|
||||
): boolean {
|
||||
const contentType =
|
||||
response.headers
|
||||
.get('content-type')
|
||||
?.split(';', 1)[0]
|
||||
.trim()
|
||||
.toLowerCase() ?? ''
|
||||
if (contentType.startsWith('video/') || contentType === 'image/gif') {
|
||||
return true
|
||||
}
|
||||
|
||||
return (
|
||||
(!contentType || contentType === 'application/octet-stream') &&
|
||||
/\.(?:gif|mov|mp4|webm)$/.test(url.pathname.toLowerCase())
|
||||
)
|
||||
}
|
||||
|
||||
export async function probeGitHubScreencast(
|
||||
rawUrl: string,
|
||||
request: MediaRequest = async (url, init) =>
|
||||
fetch(url, { ...init, signal: AbortSignal.timeout(15_000) })
|
||||
): Promise<boolean> {
|
||||
if (!isOpaqueGitHubAttachmentUrl(rawUrl)) return false
|
||||
|
||||
let current = new URL(rawUrl)
|
||||
for (let redirectCount = 0; redirectCount <= 4; redirectCount += 1) {
|
||||
if (
|
||||
current.protocol !== 'https:' ||
|
||||
current.username ||
|
||||
current.password ||
|
||||
!isAllowedGitHubMediaHost(current.hostname.toLowerCase())
|
||||
) {
|
||||
return false
|
||||
}
|
||||
|
||||
let response = await request(current.href, {
|
||||
headers: { Accept: '*/*' },
|
||||
method: 'HEAD',
|
||||
redirect: 'manual'
|
||||
})
|
||||
if (
|
||||
response.status === 405 ||
|
||||
response.status === 501 ||
|
||||
(response.status === 403 && redirectCount > 0)
|
||||
) {
|
||||
response = await request(current.href, {
|
||||
headers: { Accept: '*/*', Range: 'bytes=0-0' },
|
||||
method: 'GET',
|
||||
redirect: 'manual'
|
||||
})
|
||||
}
|
||||
|
||||
if (isRedirectStatus(response.status)) {
|
||||
const location = response.headers.get('location')
|
||||
if (!location) return false
|
||||
const redirected = new URL(location, current)
|
||||
if (
|
||||
redirected.protocol !== 'https:' ||
|
||||
redirected.username ||
|
||||
redirected.password ||
|
||||
!isAllowedGitHubMediaHost(redirected.hostname.toLowerCase())
|
||||
) {
|
||||
return false
|
||||
}
|
||||
const redirectVerdict = redirectMediaVerdict(redirected)
|
||||
if (redirectVerdict !== null) return redirectVerdict
|
||||
current = redirected
|
||||
continue
|
||||
}
|
||||
|
||||
return response.ok && isVerifiedScreencastResponse(current, response)
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
function isStandaloneScreencastLink(content: string, link: string): boolean {
|
||||
const escaped = link.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
|
||||
return new RegExp(
|
||||
`^\\s*(?:Screencast:\\s*)?(?:${escaped}|\\[[^\\]]+\\]\\(${escaped}\\))\\s*$`,
|
||||
'im'
|
||||
).test(content)
|
||||
}
|
||||
|
||||
function opaqueScreencastUrls(value: string): string[] {
|
||||
const content = clean(value)
|
||||
const links = content.match(/https?:\/\/[^\s)>]+/gi) ?? []
|
||||
return links
|
||||
.map((rawLink) => rawLink.replace(/[),.;]+$/, ''))
|
||||
.filter(
|
||||
(link) =>
|
||||
isOpaqueGitHubAttachmentUrl(link) &&
|
||||
isStandaloneScreencastLink(content, link)
|
||||
)
|
||||
}
|
||||
|
||||
function hasScreencastEvidence(
|
||||
value: string,
|
||||
verifiedOpaqueUrls: ReadonlySet<string> = new Set()
|
||||
): boolean {
|
||||
const content = clean(value)
|
||||
const links = content.match(/https?:\/\/[^\s)>]+/gi) ?? []
|
||||
for (const rawLink of links) {
|
||||
const link = rawLink.replace(/[),.;]+$/, '')
|
||||
if (isExplicitScreencastUrl(link)) return true
|
||||
try {
|
||||
const url = new URL(link)
|
||||
if (
|
||||
!isGitHubAttachmentUrl(
|
||||
url.hostname.toLowerCase(),
|
||||
url.pathname.toLowerCase()
|
||||
)
|
||||
) {
|
||||
continue
|
||||
}
|
||||
if (
|
||||
isStandaloneScreencastLink(content, link) &&
|
||||
verifiedOpaqueUrls.has(canonicalMediaUrl(link))
|
||||
) {
|
||||
return true
|
||||
}
|
||||
} catch {
|
||||
continue
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
function canonicalMediaUrl(rawUrl: string): string {
|
||||
const url = new URL(rawUrl)
|
||||
url.hash = ''
|
||||
url.search = ''
|
||||
url.hostname = url.hostname.toLowerCase()
|
||||
try {
|
||||
url.pathname = decodeURIComponent(url.pathname)
|
||||
} catch {
|
||||
// Keep malformed percent encoding unchanged; URL parsing already succeeded.
|
||||
}
|
||||
if (url.pathname.length > 1) {
|
||||
url.pathname = url.pathname.replace(/\/+$/, '')
|
||||
}
|
||||
return url.href
|
||||
}
|
||||
|
||||
function visualMediaUrls(value: string): string[] {
|
||||
const links = clean(value).match(/https?:\/\/[^\s)>]+/gi) ?? []
|
||||
return links.filter(isVisualMediaUrl)
|
||||
}
|
||||
|
||||
function hasIssueVisualEvidence(
|
||||
value: string,
|
||||
verifiedOpaqueUrls: ReadonlySet<string> = new Set()
|
||||
): boolean {
|
||||
const urls = visualMediaUrls(value)
|
||||
if (urls.some(isExplicitScreencastUrl)) return true
|
||||
|
||||
const labeledUrl = (
|
||||
label: string,
|
||||
accepts: (url: string) => boolean
|
||||
): string => {
|
||||
const match = clean(value).match(
|
||||
new RegExp(`^\\s*(?:Or\\s+)?${label}:\\s*(.+)$`, 'im')
|
||||
)
|
||||
const url = visualMediaUrls(match?.[1] ?? '')[0] ?? ''
|
||||
return accepts(url) ? url : ''
|
||||
}
|
||||
const labeledScreencast = clean(value).match(
|
||||
/^\s*(?:Or\s+)?Screencast:\s*(.+)$/im
|
||||
)?.[1]
|
||||
if (
|
||||
labeledScreencast &&
|
||||
hasScreencastEvidence(labeledScreencast, verifiedOpaqueUrls)
|
||||
) {
|
||||
return true
|
||||
}
|
||||
|
||||
const before = labeledUrl('Before', isVisualMediaUrl)
|
||||
const after = labeledUrl('After', isVisualMediaUrl)
|
||||
return Boolean(
|
||||
before && after && canonicalMediaUrl(before) !== canonicalMediaUrl(after)
|
||||
)
|
||||
}
|
||||
|
||||
function hasTestOrLogEvidence(value: string): boolean {
|
||||
const evidence = clean(value)
|
||||
if (!hasContent(evidence) || evidence.length < 16) return false
|
||||
|
||||
const command =
|
||||
/\b(?:bun|deno|npm|npx|playwright|pnpm|pytest|tox|vitest|yarn)\b/i.test(
|
||||
evidence
|
||||
)
|
||||
const outcome =
|
||||
/\b(?:exit(?:ed)?\s+(?:code\s+)?\d+|fail(?:ed|ure)?|pass(?:ed)?|success(?:ful)?|timeout)\b/i.test(
|
||||
evidence
|
||||
)
|
||||
const logExcerpt =
|
||||
/(?:^|\n)\s*(?:error|exception|failure|status)\s*[:=]|\b(?:stack trace|status\s+[1-5]\d\d)\b/i.test(
|
||||
evidence
|
||||
)
|
||||
const linkedEvidence =
|
||||
/\b(?:ci|log|test)\b/i.test(evidence) && /https?:\/\/\S+/i.test(evidence)
|
||||
|
||||
return (command && outcome) || logExcerpt || linkedEvidence
|
||||
}
|
||||
|
||||
function issueReproduction(body: string): string {
|
||||
return (
|
||||
section(body, 'Steps to Reproduce') ||
|
||||
section(body, 'Concrete example or steps')
|
||||
)
|
||||
}
|
||||
|
||||
function nonVisualIssueEvidence(value: string): boolean {
|
||||
const evidence = clean(value)
|
||||
const reason = evidence.match(/^N\/A:\s*(.+)$/im)?.[1] ?? ''
|
||||
const testOrLog =
|
||||
evidence.match(/^Test\/log evidence:\s*([\s\S]*)$/im)?.[1] ?? ''
|
||||
return hasContent(reason) && hasTestOrLogEvidence(testOrLog)
|
||||
}
|
||||
|
||||
function validateIssueEvidenceWithVerifiedMedia(
|
||||
body: string,
|
||||
verifiedOpaqueUrls: ReadonlySet<string>
|
||||
): EvidenceValidation {
|
||||
const errors: string[] = []
|
||||
const mode = section(body, 'Evidence type').toLowerCase()
|
||||
const evidence = section(body, 'Evidence')
|
||||
|
||||
if (!hasProcedure(issueReproduction(body))) {
|
||||
errors.push('Add concrete reproduction steps or an example.')
|
||||
}
|
||||
|
||||
if (mode !== 'visual' && mode !== 'non-visual') {
|
||||
errors.push('Select exactly one evidence type: Visual or Non-visual.')
|
||||
} else if (
|
||||
mode === 'visual' &&
|
||||
!hasIssueVisualEvidence(evidence, verifiedOpaqueUrls)
|
||||
) {
|
||||
errors.push(
|
||||
'Visual issues require distinct before and after links or a screencast.'
|
||||
)
|
||||
} else if (mode === 'non-visual' && !nonVisualIssueEvidence(evidence)) {
|
||||
errors.push(
|
||||
'Non-visual issues require an N/A rationale and test/log evidence.'
|
||||
)
|
||||
}
|
||||
|
||||
return { errors, valid: errors.length === 0 }
|
||||
}
|
||||
|
||||
export function validateIssueEvidence(body: string): EvidenceValidation {
|
||||
return validateIssueEvidenceWithVerifiedMedia(body, new Set())
|
||||
}
|
||||
|
||||
function selectedPrModes(body: string): string[] {
|
||||
const mode = section(body, 'Evidence type')
|
||||
return ['Visual', 'Non-visual'].filter((value) =>
|
||||
new RegExp(`^\\s*[-*]\\s+\\[[xX]\\]\\s+${value}\\s*$`, 'im').test(mode)
|
||||
)
|
||||
}
|
||||
|
||||
function validatePullRequestEvidenceWithVerifiedMedia(
|
||||
body: string,
|
||||
verifiedOpaqueUrls: ReadonlySet<string>
|
||||
): EvidenceValidation {
|
||||
const errors: string[] = []
|
||||
const selectedModes = selectedPrModes(body)
|
||||
|
||||
if (!hasProcedure(section(body, 'Reproduction or validation steps'))) {
|
||||
errors.push('Add reproduction or validation steps.')
|
||||
}
|
||||
|
||||
if (selectedModes.length !== 1) {
|
||||
errors.push('Check exactly one evidence type: Visual or Non-visual.')
|
||||
} else if (selectedModes[0] === 'Visual') {
|
||||
const before = visualMediaUrls(section(body, 'Before'))[0]
|
||||
const after = visualMediaUrls(section(body, 'After'))[0]
|
||||
const hasBeforeAndAfter = Boolean(
|
||||
before && after && canonicalMediaUrl(before) !== canonicalMediaUrl(after)
|
||||
)
|
||||
const hasScreencast = hasScreencastEvidence(
|
||||
section(body, 'Screencast'),
|
||||
verifiedOpaqueUrls
|
||||
)
|
||||
if (!hasBeforeAndAfter && !hasScreencast) {
|
||||
errors.push(
|
||||
'Visual pull requests require before and after links or a screencast.'
|
||||
)
|
||||
}
|
||||
} else {
|
||||
const rationale = section(body, 'Non-visual rationale')
|
||||
const reason = rationale.match(/^N\/A:\s*([\s\S]*)$/i)?.[1] ?? ''
|
||||
if (!hasContent(reason)) {
|
||||
errors.push('Non-visual pull requests require an N/A rationale.')
|
||||
}
|
||||
if (!hasTestOrLogEvidence(section(body, 'Test or log evidence'))) {
|
||||
errors.push('Non-visual pull requests require test or log evidence.')
|
||||
}
|
||||
}
|
||||
|
||||
return { errors, valid: errors.length === 0 }
|
||||
}
|
||||
|
||||
export function validatePullRequestEvidence(body: string): EvidenceValidation {
|
||||
return validatePullRequestEvidenceWithVerifiedMedia(body, new Set())
|
||||
}
|
||||
|
||||
async function verifiedOpaqueScreencasts(
|
||||
value: string,
|
||||
probe: ScreencastProbe
|
||||
): Promise<Set<string>> {
|
||||
const verified = new Set<string>()
|
||||
for (const url of opaqueScreencastUrls(value)) {
|
||||
try {
|
||||
if (await probe(url)) verified.add(canonicalMediaUrl(url))
|
||||
} catch {
|
||||
// Probe errors fail closed by leaving the attachment unverified.
|
||||
}
|
||||
}
|
||||
return verified
|
||||
}
|
||||
|
||||
export async function validateIssueEvidenceWithMedia(
|
||||
body: string,
|
||||
probe: ScreencastProbe = probeGitHubScreencast
|
||||
): Promise<EvidenceValidation> {
|
||||
const verified = await verifiedOpaqueScreencasts(
|
||||
section(body, 'Evidence'),
|
||||
probe
|
||||
)
|
||||
return validateIssueEvidenceWithVerifiedMedia(body, verified)
|
||||
}
|
||||
|
||||
export async function validatePullRequestEvidenceWithMedia(
|
||||
body: string,
|
||||
probe: ScreencastProbe = probeGitHubScreencast
|
||||
): Promise<EvidenceValidation> {
|
||||
const verified = await verifiedOpaqueScreencasts(
|
||||
section(body, 'Screencast'),
|
||||
probe
|
||||
)
|
||||
return validatePullRequestEvidenceWithVerifiedMedia(body, verified)
|
||||
}
|
||||
|
||||
export async function validatePullRequestEvidenceBatch(
|
||||
pullRequests: PullRequestEvidencePayload[],
|
||||
probe: ScreencastProbe = probeGitHubScreencast
|
||||
): Promise<EvidenceValidation> {
|
||||
if (pullRequests.length === 0) {
|
||||
return {
|
||||
errors: [
|
||||
'No pull request metadata was available for evidence validation.'
|
||||
],
|
||||
valid: false
|
||||
}
|
||||
}
|
||||
|
||||
const errors: string[] = []
|
||||
for (const pullRequest of pullRequests) {
|
||||
const result = await validatePullRequestEvidenceWithMedia(
|
||||
pullRequest.body ?? '',
|
||||
probe
|
||||
)
|
||||
if (!result.valid) {
|
||||
const prefix = pullRequest.number ? `PR #${pullRequest.number}: ` : ''
|
||||
errors.push(...result.errors.map((error) => `${prefix}${error}`))
|
||||
}
|
||||
}
|
||||
return { errors, valid: errors.length === 0 }
|
||||
}
|
||||
|
||||
async function runCli(): Promise<void> {
|
||||
const [kind, eventPath] = process.argv.slice(2)
|
||||
if ((kind !== 'issue' && kind !== 'pr') || !eventPath) {
|
||||
throw new Error('Usage: validate-evidence.ts <issue|pr> <event.json>')
|
||||
}
|
||||
|
||||
const payload = JSON.parse(readFileSync(eventPath, 'utf8')) as EventPayload
|
||||
const body =
|
||||
kind === 'issue' ? payload.issue?.body : payload.pull_request?.body
|
||||
const result = await validateEvidence(kind, body ?? '')
|
||||
|
||||
if (process.env.GITHUB_OUTPUT) {
|
||||
appendFileSync(
|
||||
process.env.GITHUB_OUTPUT,
|
||||
`valid=${result.valid}\nerrors=${JSON.stringify(result.errors)}\n`
|
||||
)
|
||||
}
|
||||
|
||||
if (result.valid) {
|
||||
process.stdout.write('Evidence contract satisfied.\n')
|
||||
return
|
||||
}
|
||||
|
||||
result.errors.forEach((error) => console.error(`::error::${error}`))
|
||||
process.exitCode = 1
|
||||
}
|
||||
|
||||
async function validateEvidence(
|
||||
kind: EvidenceKind,
|
||||
body: string
|
||||
): Promise<EvidenceValidation> {
|
||||
return kind === 'issue'
|
||||
? validateIssueEvidenceWithMedia(body)
|
||||
: validatePullRequestEvidenceWithMedia(body)
|
||||
}
|
||||
|
||||
if (
|
||||
process.argv[1] &&
|
||||
import.meta.url === pathToFileURL(process.argv[1]).href
|
||||
) {
|
||||
runCli().catch((error: unknown) => {
|
||||
if (process.env.GITHUB_OUTPUT) {
|
||||
appendFileSync(process.env.GITHUB_OUTPUT, 'valid=false\nerrors=[]\n')
|
||||
}
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
console.error(`::error::${message}`)
|
||||
process.exitCode = 1
|
||||
})
|
||||
}
|
||||
217
scripts/cicd/validate-maintainer.test.ts
Normal file
217
scripts/cicd/validate-maintainer.test.ts
Normal file
@@ -0,0 +1,217 @@
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { spawnSync } from 'node:child_process'
|
||||
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import {
|
||||
createGitHubMembershipLookup,
|
||||
validateMaintainerAssignees
|
||||
} from './validate-maintainer'
|
||||
|
||||
function activeMember(login = 'maintainer') {
|
||||
return {
|
||||
membership: { state: 'active' },
|
||||
teamMembership: { state: 'active' },
|
||||
user: { login, suspended_at: null, type: 'User' }
|
||||
}
|
||||
}
|
||||
|
||||
describe('validateMaintainerAssignees', () => {
|
||||
it('rejects an empty assignee list', async () => {
|
||||
const membershipFor = vi.fn()
|
||||
const result = await validateMaintainerAssignees([], membershipFor)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(membershipFor).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('accepts an active human Comfy-Org member', async () => {
|
||||
const result = await validateMaintainerAssignees(
|
||||
[{ login: 'maintainer', type: 'User' }],
|
||||
vi.fn().mockResolvedValue(activeMember())
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects a pending organization membership', async () => {
|
||||
const result = await validateMaintainerAssignees(
|
||||
[{ login: 'maintainer', type: 'User' }],
|
||||
vi.fn().mockResolvedValue({
|
||||
membership: { state: 'pending' },
|
||||
teamMembership: { state: 'active' },
|
||||
user: { login: 'maintainer', suspended_at: null, type: 'User' }
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects GitHub bot accounts even when they are organization members', async () => {
|
||||
const membershipFor = vi.fn().mockResolvedValue(activeMember())
|
||||
const result = await validateMaintainerAssignees(
|
||||
[{ login: 'automation[bot]', type: 'Bot' }],
|
||||
membershipFor
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(membershipFor).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects bot-like service users even when GitHub reports User', async () => {
|
||||
const membershipFor = vi
|
||||
.fn()
|
||||
.mockResolvedValue(activeMember('comfy-pr-bot'))
|
||||
const result = await validateMaintainerAssignees(
|
||||
[{ login: 'comfy-pr-bot', type: 'User' }],
|
||||
membershipFor
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(membershipFor).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects a human who is not an organization member', async () => {
|
||||
const result = await validateMaintainerAssignees(
|
||||
[{ login: 'contributor', type: 'User' }],
|
||||
vi.fn().mockResolvedValue({
|
||||
membership: null,
|
||||
teamMembership: null,
|
||||
user: { login: 'contributor', suspended_at: null, type: 'User' }
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(result.errors).toContain(
|
||||
'Assign at least one active human member of the Comfy-Org/comfy_frontend_devs team.'
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects an organization service user outside the human-only team', async () => {
|
||||
const result = await validateMaintainerAssignees(
|
||||
[{ login: 'comfyui-wiki', type: 'User' }],
|
||||
vi.fn().mockResolvedValue({
|
||||
membership: { state: 'active' },
|
||||
teamMembership: null,
|
||||
user: { login: 'comfyui-wiki', suspended_at: null, type: 'User' }
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects a suspended organization member', async () => {
|
||||
const result = await validateMaintainerAssignees(
|
||||
[{ login: 'maintainer', type: 'User' }],
|
||||
vi.fn().mockResolvedValue({
|
||||
membership: { state: 'active' },
|
||||
teamMembership: { state: 'active' },
|
||||
user: {
|
||||
login: 'maintainer',
|
||||
suspended_at: '2026-07-11T00:00:00Z',
|
||||
type: 'User'
|
||||
}
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
})
|
||||
|
||||
it('checks later human assignees after rejecting an ineligible one', async () => {
|
||||
const membershipFor = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({
|
||||
membership: null,
|
||||
teamMembership: null,
|
||||
user: { login: 'contributor', suspended_at: null, type: 'User' }
|
||||
})
|
||||
.mockResolvedValueOnce(activeMember('owner'))
|
||||
const result = await validateMaintainerAssignees(
|
||||
[
|
||||
{ login: 'contributor', type: 'User' },
|
||||
{ login: 'owner', type: 'User' }
|
||||
],
|
||||
membershipFor
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(true)
|
||||
expect(membershipFor).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('propagates a membership API failure so callers fail closed', async () => {
|
||||
await expect(
|
||||
validateMaintainerAssignees(
|
||||
[{ login: 'maintainer', type: 'User' }],
|
||||
vi.fn().mockRejectedValue(new Error('GitHub API unavailable'))
|
||||
)
|
||||
).rejects.toThrow('GitHub API unavailable')
|
||||
})
|
||||
|
||||
it('fails closed when the human-team membership API fails', async () => {
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({
|
||||
json: vi.fn().mockResolvedValue({ login: 'maintainer', type: 'User' }),
|
||||
ok: true,
|
||||
status: 200
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
json: vi.fn().mockResolvedValue({ state: 'active' }),
|
||||
ok: true,
|
||||
status: 200
|
||||
})
|
||||
.mockResolvedValueOnce({ ok: false, status: 503 })
|
||||
vi.stubGlobal('fetch', fetchMock)
|
||||
|
||||
try {
|
||||
await expect(
|
||||
createGitHubMembershipLookup('token')('maintainer')
|
||||
).rejects.toThrow('GitHub human-team membership lookup failed')
|
||||
expect(fetchMock.mock.calls[2][0]).toContain(
|
||||
'/teams/comfy_frontend_devs/memberships/maintainer'
|
||||
)
|
||||
} finally {
|
||||
vi.unstubAllGlobals()
|
||||
}
|
||||
})
|
||||
|
||||
it('writes valid=false and exits nonzero when CLI setup fails', () => {
|
||||
const directory = mkdtempSync(join(tmpdir(), 'maintainer-policy-'))
|
||||
const eventPath = join(directory, 'event.json')
|
||||
const outputPath = join(directory, 'github-output.txt')
|
||||
writeFileSync(
|
||||
eventPath,
|
||||
JSON.stringify({
|
||||
pull_request: {
|
||||
assignees: [{ login: 'maintainer', type: 'User' }]
|
||||
}
|
||||
})
|
||||
)
|
||||
|
||||
try {
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
['scripts/cicd/validate-maintainer.ts', eventPath],
|
||||
{
|
||||
cwd: process.cwd(),
|
||||
encoding: 'utf8',
|
||||
env: {
|
||||
...process.env,
|
||||
COMFY_ORG_MEMBERS_READ_TOKEN: '',
|
||||
GITHUB_OUTPUT: outputPath
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
expect(result.status).toBe(1)
|
||||
expect(result.stderr).toContain(
|
||||
'::error::COMFY_ORG_MEMBERS_READ_TOKEN is required.'
|
||||
)
|
||||
expect(readFileSync(outputPath, 'utf8')).toBe('valid=false\n')
|
||||
} finally {
|
||||
rmSync(directory, { force: true, recursive: true })
|
||||
}
|
||||
})
|
||||
})
|
||||
220
scripts/cicd/validate-maintainer.ts
Normal file
220
scripts/cicd/validate-maintainer.ts
Normal file
@@ -0,0 +1,220 @@
|
||||
import { appendFileSync, readFileSync } from 'node:fs'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
|
||||
type Assignee = {
|
||||
login?: string | null
|
||||
type?: string | null
|
||||
}
|
||||
|
||||
type EventPayload = {
|
||||
issue?: { assignees?: Assignee[] | null }
|
||||
pull_request?: { assignees?: Assignee[] | null }
|
||||
}
|
||||
|
||||
type GitHubUser = {
|
||||
login?: string | null
|
||||
suspended_at?: string | null
|
||||
type?: string | null
|
||||
}
|
||||
|
||||
type OrgMembership = {
|
||||
state?: string | null
|
||||
}
|
||||
|
||||
export type MembershipRecord = {
|
||||
membership: OrgMembership | null
|
||||
teamMembership: OrgMembership | null
|
||||
user: GitHubUser | null
|
||||
}
|
||||
|
||||
export type MembershipLookup = (login: string) => Promise<MembershipRecord>
|
||||
|
||||
export type MaintainerValidation = {
|
||||
errors: string[]
|
||||
valid: boolean
|
||||
}
|
||||
|
||||
export type PullRequestAssigneePayload = {
|
||||
assignees?: Assignee[] | null
|
||||
number?: number | null
|
||||
}
|
||||
|
||||
const BOT_LOGIN = /(?:\[bot\]|[-_]bot)$/i
|
||||
const COMFY_ORG = 'Comfy-Org'
|
||||
const HUMAN_TEAM = 'comfy_frontend_devs'
|
||||
|
||||
function isHumanAccount(assignee: Assignee): assignee is Assignee & {
|
||||
login: string
|
||||
} {
|
||||
const login = assignee.login?.trim() ?? ''
|
||||
return (
|
||||
Boolean(login) &&
|
||||
assignee.type?.toLowerCase() === 'user' &&
|
||||
!BOT_LOGIN.test(login)
|
||||
)
|
||||
}
|
||||
|
||||
function isActiveOrgHuman(record: MembershipRecord, login: string): boolean {
|
||||
const userLogin = record.user?.login?.trim() ?? ''
|
||||
return (
|
||||
userLogin.toLowerCase() === login.toLowerCase() &&
|
||||
record.user?.type?.toLowerCase() === 'user' &&
|
||||
!record.user.suspended_at &&
|
||||
record.membership?.state?.toLowerCase() === 'active' &&
|
||||
record.teamMembership?.state?.toLowerCase() === 'active'
|
||||
)
|
||||
}
|
||||
|
||||
export async function validateMaintainerAssignees(
|
||||
assignees: Assignee[],
|
||||
membershipFor: MembershipLookup
|
||||
): Promise<MaintainerValidation> {
|
||||
for (const assignee of assignees) {
|
||||
if (!isHumanAccount(assignee)) continue
|
||||
if (isActiveOrgHuman(await membershipFor(assignee.login), assignee.login)) {
|
||||
return { errors: [], valid: true }
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
errors: [
|
||||
'Assign at least one active human member of the Comfy-Org/comfy_frontend_devs team.'
|
||||
],
|
||||
valid: false
|
||||
}
|
||||
}
|
||||
|
||||
export async function validatePullRequestMaintainers(
|
||||
pullRequests: PullRequestAssigneePayload[],
|
||||
membershipFor: MembershipLookup
|
||||
): Promise<MaintainerValidation> {
|
||||
if (pullRequests.length === 0) {
|
||||
return {
|
||||
errors: ['No pull request metadata was available for owner validation.'],
|
||||
valid: false
|
||||
}
|
||||
}
|
||||
|
||||
const errors: string[] = []
|
||||
for (const pullRequest of pullRequests) {
|
||||
const result = await validateMaintainerAssignees(
|
||||
pullRequest.assignees ?? [],
|
||||
membershipFor
|
||||
)
|
||||
if (!result.valid) {
|
||||
const prefix = pullRequest.number ? `PR #${pullRequest.number}: ` : ''
|
||||
errors.push(...result.errors.map((error) => `${prefix}${error}`))
|
||||
}
|
||||
}
|
||||
|
||||
return { errors, valid: errors.length === 0 }
|
||||
}
|
||||
|
||||
export function createGitHubMembershipLookup(token: string): MembershipLookup {
|
||||
return async (login): Promise<MembershipRecord> => {
|
||||
const headers = {
|
||||
Accept: 'application/vnd.github+json',
|
||||
Authorization: `Bearer ${token}`,
|
||||
'X-GitHub-Api-Version': '2026-03-10'
|
||||
}
|
||||
const userResponse = await fetch(
|
||||
`https://api.github.com/users/${encodeURIComponent(login)}`,
|
||||
{ headers }
|
||||
)
|
||||
if (userResponse.status === 404) {
|
||||
return { membership: null, teamMembership: null, user: null }
|
||||
}
|
||||
if (!userResponse.ok) {
|
||||
throw new Error(
|
||||
`GitHub user lookup failed for ${login}: ${userResponse.status}`
|
||||
)
|
||||
}
|
||||
|
||||
const membershipResponse = await fetch(
|
||||
`https://api.github.com/orgs/${COMFY_ORG}/memberships/${encodeURIComponent(login)}`,
|
||||
{ headers }
|
||||
)
|
||||
if (membershipResponse.status === 404) {
|
||||
return {
|
||||
membership: null,
|
||||
teamMembership: null,
|
||||
user: (await userResponse.json()) as GitHubUser
|
||||
}
|
||||
}
|
||||
if (!membershipResponse.ok) {
|
||||
throw new Error(
|
||||
`GitHub organization membership lookup failed for ${login}: ${membershipResponse.status}`
|
||||
)
|
||||
}
|
||||
|
||||
const teamResponse = await fetch(
|
||||
`https://api.github.com/orgs/${COMFY_ORG}/teams/${HUMAN_TEAM}/memberships/${encodeURIComponent(login)}`,
|
||||
{ headers }
|
||||
)
|
||||
if (teamResponse.status === 404) {
|
||||
return {
|
||||
membership: (await membershipResponse.json()) as OrgMembership,
|
||||
teamMembership: null,
|
||||
user: (await userResponse.json()) as GitHubUser
|
||||
}
|
||||
}
|
||||
if (!teamResponse.ok) {
|
||||
throw new Error(
|
||||
`GitHub human-team membership lookup failed for ${login}: ${teamResponse.status}`
|
||||
)
|
||||
}
|
||||
|
||||
return {
|
||||
membership: (await membershipResponse.json()) as OrgMembership,
|
||||
teamMembership: (await teamResponse.json()) as OrgMembership,
|
||||
user: (await userResponse.json()) as GitHubUser
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function writeOutput(valid: boolean): void {
|
||||
if (process.env.GITHUB_OUTPUT) {
|
||||
appendFileSync(process.env.GITHUB_OUTPUT, `valid=${valid}\n`)
|
||||
}
|
||||
}
|
||||
|
||||
async function runCli(): Promise<void> {
|
||||
const [eventPath] = process.argv.slice(2)
|
||||
if (!eventPath) {
|
||||
throw new Error('Usage: validate-maintainer.ts <event.json>')
|
||||
}
|
||||
|
||||
const token = process.env.COMFY_ORG_MEMBERS_READ_TOKEN?.trim()
|
||||
if (!token) {
|
||||
throw new Error('COMFY_ORG_MEMBERS_READ_TOKEN is required.')
|
||||
}
|
||||
|
||||
const payload = JSON.parse(readFileSync(eventPath, 'utf8')) as EventPayload
|
||||
const assignees =
|
||||
payload.pull_request?.assignees ?? payload.issue?.assignees ?? []
|
||||
const result = await validateMaintainerAssignees(
|
||||
assignees,
|
||||
createGitHubMembershipLookup(token)
|
||||
)
|
||||
|
||||
writeOutput(result.valid)
|
||||
if (result.valid) {
|
||||
process.stdout.write('Human Comfy maintainer assignment satisfied.\n')
|
||||
return
|
||||
}
|
||||
|
||||
result.errors.forEach((error) => console.error(`::error::${error}`))
|
||||
process.exitCode = 1
|
||||
}
|
||||
|
||||
if (
|
||||
process.argv[1] &&
|
||||
import.meta.url === pathToFileURL(process.argv[1]).href
|
||||
) {
|
||||
runCli().catch((error: unknown) => {
|
||||
writeOutput(false)
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
console.error(`::error::${message}`)
|
||||
process.exitCode = 1
|
||||
})
|
||||
}
|
||||
239
scripts/cicd/validate-merge-group.test.ts
Normal file
239
scripts/cicd/validate-merge-group.test.ts
Normal file
@@ -0,0 +1,239 @@
|
||||
import { spawnSync } from 'node:child_process'
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import {
|
||||
loadMergeGroupPullRequests,
|
||||
validateMergeGroupPolicy
|
||||
} from './validate-merge-group'
|
||||
import type { RepoRequest } from './validate-merge-group'
|
||||
|
||||
const HEAD_SHA = 'a'.repeat(40)
|
||||
|
||||
function validBody(): string {
|
||||
return `## Reproduction or validation steps
|
||||
1. Run the metadata check
|
||||
2. Inspect the generated result
|
||||
|
||||
## Evidence type
|
||||
- [ ] Visual
|
||||
- [x] Non-visual
|
||||
|
||||
## Non-visual rationale
|
||||
N/A: This changes CI metadata only.
|
||||
|
||||
## Test or log evidence
|
||||
\`pnpm test:unit\` passed.`
|
||||
}
|
||||
|
||||
function activeHuman() {
|
||||
return {
|
||||
membership: { state: 'active' },
|
||||
teamMembership: { state: 'active' },
|
||||
user: { login: 'maintainer', suspended_at: null, type: 'User' }
|
||||
}
|
||||
}
|
||||
|
||||
describe('loadMergeGroupPullRequests', () => {
|
||||
it.for([null, 'not-a-sha'])(
|
||||
'fails before requesting metadata when head_sha is %s',
|
||||
async (headSha) => {
|
||||
const request: RepoRequest = vi.fn()
|
||||
|
||||
await expect(
|
||||
loadMergeGroupPullRequests(
|
||||
{ merge_group: { head_ref: 'unused', head_sha: headSha } },
|
||||
request
|
||||
)
|
||||
).rejects.toThrow('Merge group head SHA is missing or malformed.')
|
||||
expect(request).not.toHaveBeenCalled()
|
||||
}
|
||||
)
|
||||
|
||||
it('uses associated PRs and refetches current metadata for every PR', async () => {
|
||||
const request: RepoRequest = vi.fn(async (path) => {
|
||||
if (path === `commits/${HEAD_SHA}/pulls`) {
|
||||
return [
|
||||
{ body: 'stale', number: 41 },
|
||||
{ body: 'stale', number: 42 }
|
||||
]
|
||||
}
|
||||
if (path === 'pulls/41') {
|
||||
return {
|
||||
assignees: [{ login: 'maintainer', type: 'User' }],
|
||||
body: 'current 41',
|
||||
number: 41
|
||||
}
|
||||
}
|
||||
if (path === 'pulls/42') {
|
||||
return {
|
||||
assignees: [{ login: 'owner', type: 'User' }],
|
||||
body: 'current 42',
|
||||
number: 42
|
||||
}
|
||||
}
|
||||
throw new Error(`Unexpected path: ${path}`)
|
||||
})
|
||||
|
||||
const result = await loadMergeGroupPullRequests(
|
||||
{ merge_group: { head_ref: 'unused', head_sha: HEAD_SHA } },
|
||||
request
|
||||
)
|
||||
|
||||
expect(result.map(({ body, number }) => ({ body, number }))).toEqual([
|
||||
{ body: 'current 41', number: 41 },
|
||||
{ body: 'current 42', number: 42 }
|
||||
])
|
||||
})
|
||||
|
||||
it('falls back to the exact queue head_ref PR token when associations are empty', async () => {
|
||||
const request: RepoRequest = vi.fn(async (path) => {
|
||||
if (path === `commits/${HEAD_SHA}/pulls`) return []
|
||||
if (path === 'pulls/13556') {
|
||||
return { assignees: [], body: validBody(), number: 13556 }
|
||||
}
|
||||
throw new Error(`Unexpected path: ${path}`)
|
||||
})
|
||||
|
||||
const result = await loadMergeGroupPullRequests(
|
||||
{
|
||||
merge_group: {
|
||||
head_ref: `refs/heads/gh-readonly-queue/main/pr-13556-${HEAD_SHA}`,
|
||||
head_sha: HEAD_SHA
|
||||
}
|
||||
},
|
||||
request
|
||||
)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].number).toBe(13556)
|
||||
})
|
||||
|
||||
it('fails when neither API associations nor an exact head_ref token exist', async () => {
|
||||
const request: RepoRequest = vi.fn().mockResolvedValue([])
|
||||
|
||||
await expect(
|
||||
loadMergeGroupPullRequests(
|
||||
{
|
||||
merge_group: {
|
||||
head_ref: `refs/heads/gh-readonly-queue/main/not-a-pr-${HEAD_SHA}`,
|
||||
head_sha: HEAD_SHA
|
||||
}
|
||||
},
|
||||
request
|
||||
)
|
||||
).rejects.toThrow('No pull request was associated with the merge group')
|
||||
})
|
||||
|
||||
it('propagates associated-pulls API failures', async () => {
|
||||
await expect(
|
||||
loadMergeGroupPullRequests(
|
||||
{ merge_group: { head_ref: 'unused', head_sha: HEAD_SHA } },
|
||||
vi.fn().mockRejectedValue(new Error('GitHub API unavailable'))
|
||||
)
|
||||
).rejects.toThrow('GitHub API unavailable')
|
||||
})
|
||||
|
||||
it('rejects malformed refetched pull request metadata', async () => {
|
||||
const request: RepoRequest = vi.fn(async (path) => {
|
||||
if (path === `commits/${HEAD_SHA}/pulls`) return [{ number: 41 }]
|
||||
if (path === 'pulls/41') return { body: validBody() }
|
||||
throw new Error(`Unexpected path: ${path}`)
|
||||
})
|
||||
|
||||
await expect(
|
||||
loadMergeGroupPullRequests(
|
||||
{ merge_group: { head_ref: 'unused', head_sha: HEAD_SHA } },
|
||||
request
|
||||
)
|
||||
).rejects.toThrow('GitHub returned malformed pull request metadata.')
|
||||
})
|
||||
})
|
||||
|
||||
describe('validateMergeGroupPolicy', () => {
|
||||
it('validates current evidence and human-team ownership for every PR', async () => {
|
||||
const request: RepoRequest = vi.fn(async (path) => {
|
||||
if (path === `commits/${HEAD_SHA}/pulls`) {
|
||||
return [{ number: 41 }, { number: 42 }]
|
||||
}
|
||||
const number = Number(path.replace('pulls/', ''))
|
||||
return {
|
||||
assignees: [{ login: 'maintainer', type: 'User' }],
|
||||
body: validBody(),
|
||||
number
|
||||
}
|
||||
})
|
||||
|
||||
const result = await validateMergeGroupPolicy(
|
||||
{ merge_group: { head_ref: 'unused', head_sha: HEAD_SHA } },
|
||||
request,
|
||||
vi.fn().mockResolvedValue(activeHuman())
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(true)
|
||||
})
|
||||
|
||||
it('prefixes failures from any current PR in the group', async () => {
|
||||
const request: RepoRequest = vi.fn(async (path) => {
|
||||
if (path === `commits/${HEAD_SHA}/pulls`) {
|
||||
return [{ number: 41 }, { number: 42 }]
|
||||
}
|
||||
const number = Number(path.replace('pulls/', ''))
|
||||
return {
|
||||
assignees: [{ login: 'maintainer', type: 'User' }],
|
||||
body: number === 41 ? validBody() : '',
|
||||
number
|
||||
}
|
||||
})
|
||||
|
||||
const result = await validateMergeGroupPolicy(
|
||||
{ merge_group: { head_ref: 'unused', head_sha: HEAD_SHA } },
|
||||
request,
|
||||
vi.fn().mockResolvedValue(activeHuman())
|
||||
)
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(result.errors.some((error) => error.startsWith('PR #42:'))).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('validate-merge-group CLI', () => {
|
||||
it('loads in Node before failing closed on missing setup', () => {
|
||||
const directory = mkdtempSync(join(tmpdir(), 'merge-group-policy-'))
|
||||
const eventPath = join(directory, 'event.json')
|
||||
const outputPath = join(directory, 'github-output.txt')
|
||||
writeFileSync(eventPath, JSON.stringify({ merge_group: {} }))
|
||||
|
||||
try {
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
['scripts/cicd/validate-merge-group.ts', eventPath],
|
||||
{
|
||||
cwd: process.cwd(),
|
||||
encoding: 'utf8',
|
||||
env: {
|
||||
...process.env,
|
||||
COMFY_ORG_MEMBERS_READ_TOKEN: '',
|
||||
GITHUB_OUTPUT: outputPath,
|
||||
GITHUB_REPOSITORY: '',
|
||||
GITHUB_TOKEN: ''
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
expect(result.status).toBe(1)
|
||||
expect(result.stderr).toContain(
|
||||
'::error::Event path, GITHUB_TOKEN, GITHUB_REPOSITORY, and COMFY_ORG_MEMBERS_READ_TOKEN are required.'
|
||||
)
|
||||
expect(result.stderr).not.toContain('ERR_MODULE_NOT_FOUND')
|
||||
expect(readFileSync(outputPath, 'utf8')).toBe('valid=false\n')
|
||||
} finally {
|
||||
rmSync(directory, { force: true, recursive: true })
|
||||
}
|
||||
})
|
||||
})
|
||||
202
scripts/cicd/validate-merge-group.ts
Normal file
202
scripts/cicd/validate-merge-group.ts
Normal file
@@ -0,0 +1,202 @@
|
||||
import { appendFileSync, readFileSync } from 'node:fs'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
|
||||
import { validatePullRequestEvidenceBatch } from './validate-evidence.ts'
|
||||
import type {
|
||||
EvidenceValidation,
|
||||
PullRequestEvidencePayload,
|
||||
ScreencastProbe
|
||||
} from './validate-evidence.ts'
|
||||
import {
|
||||
createGitHubMembershipLookup,
|
||||
validatePullRequestMaintainers
|
||||
} from './validate-maintainer.ts'
|
||||
import type {
|
||||
MembershipLookup,
|
||||
PullRequestAssigneePayload
|
||||
} from './validate-maintainer.ts'
|
||||
|
||||
type MergeGroupEvent = {
|
||||
merge_group?: {
|
||||
head_ref?: string | null
|
||||
head_sha?: string | null
|
||||
} | null
|
||||
}
|
||||
|
||||
type PullRequestPayload = PullRequestEvidencePayload &
|
||||
PullRequestAssigneePayload & {
|
||||
number: number
|
||||
}
|
||||
|
||||
export type RepoRequest = (path: string) => Promise<unknown>
|
||||
|
||||
function pullRequestNumber(value: unknown): number | null {
|
||||
if (!value || typeof value !== 'object' || !('number' in value)) return null
|
||||
const number = value.number
|
||||
return typeof number === 'number' && Number.isInteger(number) && number > 0
|
||||
? number
|
||||
: null
|
||||
}
|
||||
|
||||
function currentPullRequest(value: unknown): PullRequestPayload {
|
||||
const number = pullRequestNumber(value)
|
||||
if (!number || !value || typeof value !== 'object') {
|
||||
throw new Error('GitHub returned malformed pull request metadata.')
|
||||
}
|
||||
|
||||
const body =
|
||||
'body' in value && typeof value.body === 'string' ? value.body : ''
|
||||
const assignees =
|
||||
'assignees' in value && Array.isArray(value.assignees)
|
||||
? value.assignees
|
||||
.filter(
|
||||
(assignee): assignee is { login?: string; type?: string } =>
|
||||
Boolean(assignee) && typeof assignee === 'object'
|
||||
)
|
||||
.map((assignee) => ({
|
||||
login:
|
||||
typeof assignee.login === 'string' ? assignee.login : undefined,
|
||||
type: typeof assignee.type === 'string' ? assignee.type : undefined
|
||||
}))
|
||||
: []
|
||||
|
||||
return { assignees, body, number }
|
||||
}
|
||||
|
||||
function fallbackPullRequestNumber(headRef: string): number | null {
|
||||
const match = headRef.match(/\/pr-(\d+)-[0-9a-f]{40}$/i)
|
||||
if (!match) return null
|
||||
const number = Number(match[1])
|
||||
return Number.isSafeInteger(number) && number > 0 ? number : null
|
||||
}
|
||||
|
||||
export async function loadMergeGroupPullRequests(
|
||||
event: MergeGroupEvent,
|
||||
request: RepoRequest
|
||||
): Promise<PullRequestPayload[]> {
|
||||
const headSha = event.merge_group?.head_sha?.trim() ?? ''
|
||||
const headRef = event.merge_group?.head_ref?.trim() ?? ''
|
||||
if (!/^[0-9a-f]{40}$/i.test(headSha)) {
|
||||
throw new Error('Merge group head SHA is missing or malformed.')
|
||||
}
|
||||
|
||||
const associated = await request(
|
||||
`commits/${encodeURIComponent(headSha)}/pulls`
|
||||
)
|
||||
if (!Array.isArray(associated)) {
|
||||
throw new Error('GitHub returned malformed associated pull request data.')
|
||||
}
|
||||
|
||||
let numbers: number[]
|
||||
if (associated.length > 0) {
|
||||
numbers = associated.map((pullRequest) => {
|
||||
const number = pullRequestNumber(pullRequest)
|
||||
if (!number) {
|
||||
throw new Error('GitHub returned a malformed associated pull request.')
|
||||
}
|
||||
return number
|
||||
})
|
||||
} else {
|
||||
const fallback = fallbackPullRequestNumber(headRef)
|
||||
if (!fallback) {
|
||||
throw new Error(
|
||||
'No pull request was associated with the merge group and head_ref did not identify one.'
|
||||
)
|
||||
}
|
||||
numbers = [fallback]
|
||||
}
|
||||
|
||||
const uniqueNumbers = [...new Set(numbers)]
|
||||
return Promise.all(
|
||||
uniqueNumbers.map(async (number) =>
|
||||
currentPullRequest(await request(`pulls/${number}`))
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
export async function validateMergeGroupPolicy(
|
||||
event: MergeGroupEvent,
|
||||
request: RepoRequest,
|
||||
membershipFor: MembershipLookup,
|
||||
screencastProbe?: ScreencastProbe
|
||||
): Promise<EvidenceValidation> {
|
||||
const pullRequests = await loadMergeGroupPullRequests(event, request)
|
||||
const [evidence, maintainers] = await Promise.all([
|
||||
validatePullRequestEvidenceBatch(pullRequests, screencastProbe),
|
||||
validatePullRequestMaintainers(pullRequests, membershipFor)
|
||||
])
|
||||
const errors = [...evidence.errors, ...maintainers.errors]
|
||||
return { errors, valid: errors.length === 0 }
|
||||
}
|
||||
|
||||
function createRepoRequest(repository: string, token: string): RepoRequest {
|
||||
const [owner, repo, ...extra] = repository.split('/')
|
||||
if (!owner || !repo || extra.length > 0) {
|
||||
throw new Error('GITHUB_REPOSITORY must be owner/repo.')
|
||||
}
|
||||
|
||||
return async (path): Promise<unknown> => {
|
||||
const response = await fetch(
|
||||
`https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${path}`,
|
||||
{
|
||||
headers: {
|
||||
Accept: 'application/vnd.github+json',
|
||||
Authorization: `Bearer ${token}`,
|
||||
'X-GitHub-Api-Version': '2026-03-10'
|
||||
},
|
||||
signal: AbortSignal.timeout(30_000)
|
||||
}
|
||||
)
|
||||
if (!response.ok) {
|
||||
throw new Error(
|
||||
`GitHub repository metadata lookup failed: ${response.status}`
|
||||
)
|
||||
}
|
||||
return response.json()
|
||||
}
|
||||
}
|
||||
|
||||
function writeOutput(valid: boolean): void {
|
||||
if (process.env.GITHUB_OUTPUT) {
|
||||
appendFileSync(process.env.GITHUB_OUTPUT, `valid=${valid}\n`)
|
||||
}
|
||||
}
|
||||
|
||||
async function runCli(): Promise<void> {
|
||||
const [eventPath] = process.argv.slice(2)
|
||||
const repoToken = process.env.GITHUB_TOKEN?.trim()
|
||||
const membershipToken = process.env.COMFY_ORG_MEMBERS_READ_TOKEN?.trim()
|
||||
const repository = process.env.GITHUB_REPOSITORY?.trim()
|
||||
if (!eventPath || !repoToken || !membershipToken || !repository) {
|
||||
throw new Error(
|
||||
'Event path, GITHUB_TOKEN, GITHUB_REPOSITORY, and COMFY_ORG_MEMBERS_READ_TOKEN are required.'
|
||||
)
|
||||
}
|
||||
|
||||
const event = JSON.parse(readFileSync(eventPath, 'utf8')) as MergeGroupEvent
|
||||
const result = await validateMergeGroupPolicy(
|
||||
event,
|
||||
createRepoRequest(repository, repoToken),
|
||||
createGitHubMembershipLookup(membershipToken)
|
||||
)
|
||||
writeOutput(result.valid)
|
||||
if (result.valid) {
|
||||
process.stdout.write('Merge group contribution policy satisfied.\n')
|
||||
return
|
||||
}
|
||||
|
||||
result.errors.forEach((error) => console.error(`::error::${error}`))
|
||||
process.exitCode = 1
|
||||
}
|
||||
|
||||
if (
|
||||
process.argv[1] &&
|
||||
import.meta.url === pathToFileURL(process.argv[1]).href
|
||||
) {
|
||||
runCli().catch((error: unknown) => {
|
||||
writeOutput(false)
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
console.error(`::error::${message}`)
|
||||
process.exitCode = 1
|
||||
})
|
||||
}
|
||||
@@ -79,7 +79,6 @@ export interface SafeWidgetData {
|
||||
advanced?: boolean
|
||||
hidden?: boolean
|
||||
read_only?: boolean
|
||||
removable?: boolean
|
||||
values?: unknown
|
||||
}
|
||||
/** Input specification from node definition */
|
||||
@@ -207,8 +206,7 @@ function extractWidgetDisplayOptions(
|
||||
canvasOnly: widget.options.canvasOnly,
|
||||
advanced: widget.options?.advanced ?? widget.advanced,
|
||||
hidden: widget.options.hidden,
|
||||
read_only: widget.options.read_only,
|
||||
removable: widget.options.removable
|
||||
read_only: widget.options.read_only
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,54 +0,0 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { transformInputSpecV1ToV2 } from '@/schemas/nodeDef/migration'
|
||||
import type { InputSpec } from '@/schemas/nodeDefSchema'
|
||||
|
||||
import { resolveInputType } from './dynamicTypes'
|
||||
|
||||
describe('resolveInputType', () => {
|
||||
it('resolves field types from a dynamic group template', () => {
|
||||
const spec = transformInputSpecV1ToV2(
|
||||
[
|
||||
'COMFY_DYNAMICGROUP_V3',
|
||||
{
|
||||
template: {
|
||||
required: { image: ['IMAGE', {}] },
|
||||
optional: { text: ['STRING', {}] }
|
||||
}
|
||||
}
|
||||
] as InputSpec,
|
||||
{ name: 'loras', isOptional: false }
|
||||
)
|
||||
|
||||
expect(resolveInputType(spec)).toEqual(['IMAGE', 'STRING'])
|
||||
})
|
||||
|
||||
it('resolves nested combo types inside a dynamic group template', () => {
|
||||
const spec = transformInputSpecV1ToV2(
|
||||
[
|
||||
'COMFY_DYNAMICGROUP_V3',
|
||||
{
|
||||
template: {
|
||||
required: {
|
||||
mode: [['a', 'b'], {}]
|
||||
}
|
||||
}
|
||||
}
|
||||
] as InputSpec,
|
||||
{ name: 'loras', isOptional: false }
|
||||
)
|
||||
|
||||
expect(resolveInputType(spec)).toEqual(['COMBO'])
|
||||
})
|
||||
|
||||
it('returns an empty list for an invalid dynamic group spec', () => {
|
||||
const spec = transformInputSpecV1ToV2(
|
||||
['COMFY_DYNAMICGROUP_V3', { template: { required: {} } }] as InputSpec,
|
||||
{ name: 'loras', isOptional: false }
|
||||
)
|
||||
spec.type = 'COMFY_DYNAMICGROUP_V3'
|
||||
spec.template = undefined as never
|
||||
|
||||
expect(resolveInputType(spec)).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -1,9 +1,5 @@
|
||||
import { transformInputSpecV1ToV2 } from '@/schemas/nodeDef/migration'
|
||||
import {
|
||||
zAutogrowOptions,
|
||||
zDynamicGroupInputSpec,
|
||||
zMatchTypeOptions
|
||||
} from '@/schemas/nodeDefSchema'
|
||||
import { zAutogrowOptions, zMatchTypeOptions } from '@/schemas/nodeDefSchema'
|
||||
import type { InputSpec } from '@/schemas/nodeDefSchema'
|
||||
import type { InputSpec as InputSpecV2 } from '@/schemas/nodeDef/nodeDefSchemaV2'
|
||||
|
||||
@@ -12,7 +8,6 @@ const dynamicTypeResolvers: Record<
|
||||
(inputSpec: InputSpecV2) => string[]
|
||||
> = {
|
||||
COMFY_AUTOGROW_V3: resolveAutogrowType,
|
||||
COMFY_DYNAMICGROUP_V3: resolveDynamicGroupType,
|
||||
COMFY_MATCHTYPE_V3: (input) =>
|
||||
zMatchTypeOptions
|
||||
.safeParse(input)
|
||||
@@ -25,21 +20,6 @@ export function resolveInputType(input: InputSpecV2): string[] {
|
||||
: input.type.split(',')
|
||||
}
|
||||
|
||||
function resolveDynamicGroupType(rawSpec: InputSpecV2): string[] {
|
||||
const parsed = zDynamicGroupInputSpec.safeParse([rawSpec.type, rawSpec])
|
||||
const template = parsed.data?.[1]?.template
|
||||
if (!template) return []
|
||||
const inputTypes: (Record<string, InputSpec> | undefined)[] = [
|
||||
template.required,
|
||||
template.optional
|
||||
]
|
||||
return inputTypes.flatMap((inputType) =>
|
||||
Object.entries(inputType ?? {}).flatMap(([name, v]) =>
|
||||
resolveInputType(transformInputSpecV1ToV2(v, { name }))
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
function resolveAutogrowType(rawSpec: InputSpecV2): string[] {
|
||||
const { input } = zAutogrowOptions.safeParse(rawSpec).data?.template ?? {}
|
||||
|
||||
|
||||
@@ -1,15 +1,11 @@
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { beforeEach, describe, expect, test } from 'vitest'
|
||||
|
||||
import type { DynamicGroupNode } from '@/core/graph/widgets/dynamicWidgets'
|
||||
import { describe, expect, test, vi } from 'vitest'
|
||||
import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { transformInputSpecV1ToV2 } from '@/schemas/nodeDef/migration'
|
||||
import type { InputSpec } from '@/schemas/nodeDefSchema'
|
||||
import { useLitegraphService } from '@/services/litegraphService'
|
||||
import type { HasInitialMinSize } from '@/services/litegraphService'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import { widgetId } from '@/types/widgetId'
|
||||
|
||||
setActivePinia(createTestingPinia())
|
||||
type DynamicInputs = ('INT' | 'STRING' | 'IMAGE' | DynamicInputs)[][]
|
||||
@@ -51,33 +47,6 @@ function addDynamicCombo(node: LGraphNode, inputs: DynamicInputs) {
|
||||
transformInputSpecV1ToV2(inputSpec, { name: namePrefix, isOptional: false })
|
||||
)
|
||||
}
|
||||
function addDynamicGroup(
|
||||
node: LGraphNode,
|
||||
template: object,
|
||||
{
|
||||
min,
|
||||
max,
|
||||
name = 'g',
|
||||
group_name
|
||||
}: {
|
||||
min?: number
|
||||
max?: number
|
||||
name?: string
|
||||
group_name?: string
|
||||
} = {}
|
||||
) {
|
||||
const options: Record<string, unknown> = { template }
|
||||
if (min !== undefined) options.min = min
|
||||
if (max !== undefined) options.max = max
|
||||
if (group_name !== undefined) options.group_name = group_name
|
||||
addNodeInput(
|
||||
node,
|
||||
transformInputSpecV1ToV2(['COMFY_DYNAMICGROUP_V3', options] as InputSpec, {
|
||||
name,
|
||||
isOptional: false
|
||||
})
|
||||
)
|
||||
}
|
||||
function addAutogrow(node: LGraphNode, template: unknown) {
|
||||
addNodeInput(
|
||||
node,
|
||||
@@ -318,259 +287,3 @@ describe('Autogrow', () => {
|
||||
])
|
||||
})
|
||||
})
|
||||
describe('Dynamic Groups', () => {
|
||||
const stringTemplate = { required: { a: ['STRING', {}] } }
|
||||
const widgetNames = (node: LGraphNode) => node.widgets!.map((w) => w.name)
|
||||
const inputNames = (node: LGraphNode) => node.inputs.map((i) => i.name)
|
||||
const widgetNamed = (node: LGraphNode, name: string) =>
|
||||
node.widgets!.find((w) => w.name === name)!
|
||||
|
||||
test('renders min rows on creation', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 2, max: 5 })
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a', 'g.1.a'])
|
||||
expect(inputNames(node)).toStrictEqual([])
|
||||
})
|
||||
|
||||
test('add row appends a new row up to max', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 0, max: 2 })
|
||||
expect(widgetNames(node)).toStrictEqual(['g'])
|
||||
|
||||
widgetNamed(node, 'g').callback?.(undefined)
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a'])
|
||||
|
||||
widgetNamed(node, 'g').callback?.(undefined)
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a', 'g.1.a'])
|
||||
|
||||
// At max, further adds are ignored.
|
||||
widgetNamed(node, 'g').callback?.(undefined)
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a', 'g.1.a'])
|
||||
})
|
||||
|
||||
test('controller disabled option set at max', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 0, max: 1 })
|
||||
expect(widgetNamed(node, 'g').options?.disabled).toBe(false)
|
||||
widgetNamed(node, 'g').callback?.(undefined)
|
||||
expect(widgetNamed(node, 'g').options?.disabled).toBe(true)
|
||||
})
|
||||
|
||||
test('remove row renumbers later rows', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 0, max: 5 })
|
||||
const state = (
|
||||
node as Parameters<typeof widgetNamed>[0] & {
|
||||
comfyDynamic: {
|
||||
dynamicGroup: Record<
|
||||
string,
|
||||
{ addRow: () => void; removeRow: (r: number) => void }
|
||||
>
|
||||
}
|
||||
}
|
||||
).comfyDynamic.dynamicGroup['g']
|
||||
state.addRow()
|
||||
state.addRow()
|
||||
state.addRow()
|
||||
|
||||
const row0Field = widgetNamed(node, 'g.0.a')
|
||||
const row2Field = widgetNamed(node, 'g.2.a')
|
||||
|
||||
state.removeRow(1)
|
||||
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a', 'g.1.a'])
|
||||
// Row 0 is untouched; the former row 2 shifts down into row 1.
|
||||
expect(widgetNamed(node, 'g.0.a')).toBe(row0Field)
|
||||
expect(widgetNamed(node, 'g.1.a')).toBe(row2Field)
|
||||
})
|
||||
|
||||
test('remove row disconnects linked sockets and renumbers inputs', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 0, max: 5 })
|
||||
const state = (
|
||||
node as Parameters<typeof widgetNamed>[0] & {
|
||||
comfyDynamic: {
|
||||
dynamicGroup: Record<
|
||||
string,
|
||||
{ addRow: () => void; removeRow: (r: number) => void }
|
||||
>
|
||||
}
|
||||
}
|
||||
).comfyDynamic.dynamicGroup['g']
|
||||
state.addRow()
|
||||
state.addRow()
|
||||
state.addRow()
|
||||
|
||||
const graph = new LGraph()
|
||||
graph.add(node)
|
||||
node.addInput('g.1.a', 'STRING')
|
||||
const row1Index = node.inputs.findIndex((i) => i.name === 'g.1.a')
|
||||
connectInput(node, row1Index, graph)
|
||||
const linkId = node.inputs[row1Index].link!
|
||||
node.addInput('g.2.a', 'STRING')
|
||||
|
||||
state.removeRow(1)
|
||||
|
||||
expect(graph.links[linkId]).toBeUndefined()
|
||||
expect(inputNames(node)).toStrictEqual(['g.1.a'])
|
||||
expect(node.inputs[0].link).toBeNull()
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a', 'g.1.a'])
|
||||
})
|
||||
|
||||
test('rows below min cannot be removed', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 1, max: 5 })
|
||||
const state = (
|
||||
node as Parameters<typeof widgetNamed>[0] & {
|
||||
comfyDynamic: {
|
||||
dynamicGroup: Record<string, { removeRow: (r: number) => void }>
|
||||
}
|
||||
}
|
||||
).comfyDynamic.dynamicGroup['g']
|
||||
|
||||
// Only the minimum row remains — removing it is a no-op.
|
||||
state.removeRow(0)
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a'])
|
||||
})
|
||||
|
||||
test('the first row can be removed while above the minimum count', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 1, max: 5 })
|
||||
const state = (node as unknown as DynamicGroupNode).comfyDynamic
|
||||
.dynamicGroup['g']
|
||||
state.addRow()
|
||||
state.addRow()
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a', 'g.1.a', 'g.2.a'])
|
||||
|
||||
state.removeRow(0)
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a', 'g.1.a'])
|
||||
})
|
||||
|
||||
test('cannot remove a row once at the minimum count', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 2, max: 5 })
|
||||
const state = (node as unknown as DynamicGroupNode).comfyDynamic
|
||||
.dynamicGroup['g']
|
||||
|
||||
state.removeRow(1)
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a', 'g.1.a'])
|
||||
})
|
||||
|
||||
test('respects max for socket-only field types', () => {
|
||||
const node = testNode()
|
||||
const graph = new LGraph()
|
||||
graph.add(node)
|
||||
addDynamicGroup(
|
||||
node,
|
||||
{ required: { image: ['IMAGE', {}] } },
|
||||
{ min: 0, max: 2 }
|
||||
)
|
||||
const state = (node as unknown as DynamicGroupNode).comfyDynamic
|
||||
.dynamicGroup['g']
|
||||
|
||||
for (let i = 0; i < 10; i++) state.addRow()
|
||||
|
||||
const groupInputs = node.inputs.filter((i) => i.name.startsWith('g.'))
|
||||
expect(groupInputs.map((i) => i.name)).toStrictEqual([
|
||||
'g.0.image',
|
||||
'g.1.image'
|
||||
])
|
||||
})
|
||||
|
||||
test('controller value setter rebuilds rows within min and max', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 1, max: 4 })
|
||||
const controller = widgetNamed(node, 'g')
|
||||
|
||||
controller.value = 3
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a', 'g.1.a', 'g.2.a'])
|
||||
expect(controller.value).toBe(3)
|
||||
|
||||
controller.value = 99
|
||||
expect(widgetNames(node)).toStrictEqual([
|
||||
'g',
|
||||
'g.0.a',
|
||||
'g.1.a',
|
||||
'g.2.a',
|
||||
'g.3.a'
|
||||
])
|
||||
expect(controller.value).toBe(4)
|
||||
|
||||
controller.value = 0
|
||||
expect(widgetNames(node)).toStrictEqual(['g', 'g.0.a'])
|
||||
expect(controller.value).toBe(1)
|
||||
})
|
||||
|
||||
test('stores group_name on dynamic group state', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, {
|
||||
min: 1,
|
||||
max: 3,
|
||||
name: 'loras',
|
||||
group_name: 'Lora'
|
||||
})
|
||||
const state = (node as unknown as DynamicGroupNode).comfyDynamic
|
||||
.dynamicGroup.loras
|
||||
|
||||
expect(state.groupName).toBe('Lora')
|
||||
})
|
||||
|
||||
test('remove row renames linked input widget metadata', () => {
|
||||
const node = testNode()
|
||||
addDynamicGroup(node, stringTemplate, { min: 0, max: 5 })
|
||||
const state = (node as unknown as DynamicGroupNode).comfyDynamic
|
||||
.dynamicGroup['g']
|
||||
state.addRow()
|
||||
state.addRow()
|
||||
|
||||
const row2Input = node.addInput('g.2.a', 'STRING')
|
||||
row2Input.widget = { name: 'g.2.a' }
|
||||
|
||||
state.removeRow(1)
|
||||
|
||||
expect(row2Input.name).toBe('g.1.a')
|
||||
expect(row2Input.widget?.name).toBe('g.1.a')
|
||||
})
|
||||
})
|
||||
|
||||
describe('Dynamic Groups widget value store cleanup', () => {
|
||||
const stringTemplate = { required: { a: ['STRING', {}] } }
|
||||
const widgetNamed = (node: LGraphNode, name: string) =>
|
||||
node.widgets!.find((w) => w.name === name)!
|
||||
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
})
|
||||
|
||||
test('remove row deletes the removed entry and renumbers survivors', () => {
|
||||
const node = testNode()
|
||||
const graph = new LGraph()
|
||||
graph.add(node)
|
||||
addDynamicGroup(node, stringTemplate, { min: 0, max: 5 })
|
||||
const state = (node as unknown as DynamicGroupNode).comfyDynamic
|
||||
.dynamicGroup['g']
|
||||
state.addRow()
|
||||
state.addRow()
|
||||
state.addRow()
|
||||
|
||||
widgetNamed(node, 'g.0.a').value = 'A'
|
||||
widgetNamed(node, 'g.1.a').value = 'B'
|
||||
widgetNamed(node, 'g.2.a').value = 'C'
|
||||
|
||||
state.removeRow(1)
|
||||
|
||||
const store = useWidgetValueStore()
|
||||
const graphId = graph.rootGraph.id
|
||||
const at = (name: string) =>
|
||||
store.getWidget(widgetId(graphId, node.id, name))?.value
|
||||
|
||||
// Removed row's value is gone, and the former row 2 shifts into row 1.
|
||||
expect(at('g.0.a')).toBe('A')
|
||||
expect(at('g.1.a')).toBe('C')
|
||||
expect(store.getWidget(widgetId(graphId, node.id, 'g.2.a'))).toBeUndefined()
|
||||
|
||||
// Adding a fresh row starts empty rather than leaking the old value.
|
||||
state.addRow()
|
||||
expect(at('g.2.a')).not.toBe('C')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -13,13 +13,11 @@ import type { LLink } from '@/lib/litegraph/src/LLink'
|
||||
import { commonType } from '@/lib/litegraph/src/utils/type'
|
||||
import { resolveNodeRootGraphId } from '@/lib/litegraph/src/utils/widget'
|
||||
import { transformInputSpecV1ToV2 } from '@/schemas/nodeDef/migration'
|
||||
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
|
||||
import type { ComboInputSpec, InputSpec } from '@/schemas/nodeDefSchema'
|
||||
import type { InputSpec as InputSpecV2 } from '@/schemas/nodeDef/nodeDefSchemaV2'
|
||||
import {
|
||||
zAutogrowOptions,
|
||||
zDynamicComboInputSpec,
|
||||
zDynamicGroupInputSpec,
|
||||
zMatchTypeOptions
|
||||
} from '@/schemas/nodeDefSchema'
|
||||
import { useLitegraphService } from '@/services/litegraphService'
|
||||
@@ -30,18 +28,6 @@ import { widgetId } from '@/types/widgetId'
|
||||
|
||||
const INLINE_INPUTS = false
|
||||
|
||||
type DynamicGroupState = {
|
||||
min: number
|
||||
max: number
|
||||
groupName?: string
|
||||
inputSpecs: InputSpecV2[]
|
||||
addRow: () => void
|
||||
removeRow: (row: number) => void
|
||||
}
|
||||
export type DynamicGroupNode = LGraphNode & {
|
||||
comfyDynamic: { dynamicGroup: Record<string, DynamicGroupState> }
|
||||
}
|
||||
|
||||
type MatchTypeNode = LGraphNode &
|
||||
Pick<Required<LGraphNode>, 'onConnectionsChange'> & {
|
||||
comfyDynamic: { matchType: Record<string, Record<string, string>> }
|
||||
@@ -228,254 +214,7 @@ function dynamicComboWidget(
|
||||
return { widget, minWidth, minHeight }
|
||||
}
|
||||
|
||||
function withComfyDynamicGroup(
|
||||
node: LGraphNode
|
||||
): asserts node is DynamicGroupNode {
|
||||
if (node.comfyDynamic?.dynamicGroup) return
|
||||
node.comfyDynamic ??= {}
|
||||
node.comfyDynamic.dynamicGroup = {}
|
||||
}
|
||||
|
||||
const fieldName = (group: string, row: number, field: string) =>
|
||||
`${group}.${row}.${field}`
|
||||
|
||||
/** Rename a field that sits above the removed row, shifting its index down. */
|
||||
function shiftedFieldName(
|
||||
group: string,
|
||||
name: string,
|
||||
removedRow: number
|
||||
): string | undefined {
|
||||
const prefix = `${group}.`
|
||||
if (!name.startsWith(prefix)) return undefined
|
||||
const rest = name.slice(prefix.length)
|
||||
const dot = rest.indexOf('.')
|
||||
if (dot === -1) return undefined
|
||||
const row = Number(rest.slice(0, dot))
|
||||
if (!Number.isInteger(row) || row <= removedRow) return undefined
|
||||
return fieldName(group, row - 1, rest.slice(dot + 1))
|
||||
}
|
||||
|
||||
const isGroupField = (group: string, name: string) =>
|
||||
name.startsWith(`${group}.`)
|
||||
|
||||
const belongsToRow = (group: string, name: string, row: number): boolean =>
|
||||
name.startsWith(`${group}.${row}.`)
|
||||
|
||||
function rowIndexOf(group: string, name: string): number | undefined {
|
||||
if (!isGroupField(group, name)) return undefined
|
||||
const rest = name.slice(group.length + 1)
|
||||
const dot = rest.indexOf('.')
|
||||
if (dot === -1) return undefined
|
||||
const row = Number(rest.slice(0, dot))
|
||||
return Number.isInteger(row) ? row : undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Counts distinct rows in the group. Fields may be backed by a widget, an input
|
||||
* socket, or both, so both collections are scanned to keep the count accurate
|
||||
* for socket-only field types (e.g. IMAGE).
|
||||
*/
|
||||
function countGroupRows(group: string, node: LGraphNode): number {
|
||||
const rows = new Set<number>()
|
||||
for (const w of node.widgets ?? []) {
|
||||
const row = rowIndexOf(group, w.name)
|
||||
if (row !== undefined) rows.add(row)
|
||||
}
|
||||
for (const input of node.inputs) {
|
||||
const row = rowIndexOf(group, input.name)
|
||||
if (row !== undefined) rows.add(row)
|
||||
}
|
||||
return rows.size
|
||||
}
|
||||
|
||||
/** Build field widgets for a single row, returning them detached from the node. */
|
||||
function createRow(
|
||||
group: string,
|
||||
row: number,
|
||||
state: DynamicGroupState,
|
||||
node: DynamicGroupNode
|
||||
): IBaseWidget[] {
|
||||
const { addNodeInput } = useLitegraphService()
|
||||
const startLen = node.widgets!.length
|
||||
|
||||
for (const spec of state.inputSpecs)
|
||||
addNodeInput(node, {
|
||||
...spec,
|
||||
name: fieldName(group, row, spec.name),
|
||||
display_name: spec.display_name ?? spec.name,
|
||||
hidden: true,
|
||||
socketless: true
|
||||
})
|
||||
|
||||
return node.widgets!.splice(startLen)
|
||||
}
|
||||
|
||||
function insertRowAfterGroup(
|
||||
group: string,
|
||||
node: LGraphNode,
|
||||
rowWidgets: IBaseWidget[]
|
||||
): void {
|
||||
const lastIdx = node.widgets!.findLastIndex(
|
||||
(w) => w.name === group || isGroupField(group, w.name)
|
||||
)
|
||||
node.widgets!.splice(lastIdx + 1, 0, ...rowWidgets)
|
||||
}
|
||||
|
||||
function removeGroupInputs(
|
||||
node: DynamicGroupNode,
|
||||
predicate: (name: string) => boolean
|
||||
): void {
|
||||
for (let i = node.inputs.length - 1; i >= 0; i--) {
|
||||
if (predicate(node.inputs[i].name)) node.removeInput(i)
|
||||
}
|
||||
}
|
||||
|
||||
function syncController(group: string, node: DynamicGroupNode): void {
|
||||
const state = node.comfyDynamic.dynamicGroup[group]
|
||||
const controller = node.widgets?.find((w) => w.name === group)
|
||||
if (!state || !controller) return
|
||||
controller.options ??= {}
|
||||
controller.options.disabled = countGroupRows(group, node) >= state.max
|
||||
// Route through setSize (not `size[1] = …`) so the layout store and the Vue
|
||||
// node's min-height floor are updated; a direct buffer write bypasses the
|
||||
// size setter and leaves the node unable to shrink after rows are removed.
|
||||
node.setSize([node.size[0], node.computeSize()[1]])
|
||||
}
|
||||
|
||||
function addRow(group: string, node: DynamicGroupNode): void {
|
||||
const state = node.comfyDynamic.dynamicGroup[group]
|
||||
if (!state) return
|
||||
node.widgets ??= []
|
||||
const row = countGroupRows(group, node)
|
||||
if (row >= state.max) return
|
||||
insertRowAfterGroup(group, node, createRow(group, row, state, node))
|
||||
syncController(group, node)
|
||||
app.canvas?.setDirty(true, true)
|
||||
}
|
||||
|
||||
function removeRow(group: string, row: number, node: DynamicGroupNode): void {
|
||||
const state = node.comfyDynamic.dynamicGroup[group]
|
||||
if (!state || countGroupRows(group, node) <= state.min) return
|
||||
|
||||
const store = useWidgetValueStore()
|
||||
const graphId = resolveNodeRootGraphId(node)
|
||||
const keyFor = (name: string) =>
|
||||
graphId ? widgetId(graphId, node.id, name) : undefined
|
||||
|
||||
for (const w of remove(node.widgets!, (w) =>
|
||||
belongsToRow(group, w.name, row)
|
||||
)) {
|
||||
w.onRemove?.()
|
||||
const key = keyFor(w.name)
|
||||
if (key) store.deleteWidget(key)
|
||||
}
|
||||
removeGroupInputs(node, (name) => belongsToRow(group, name, row))
|
||||
|
||||
for (const w of node.widgets ?? []) {
|
||||
const shifted = shiftedFieldName(group, w.name, row)
|
||||
if (shifted === undefined) continue
|
||||
const from = keyFor(w.name)
|
||||
const to = keyFor(shifted)
|
||||
if (from && to) store.renameWidget(from, to)
|
||||
w.name = shifted
|
||||
}
|
||||
for (const inp of node.inputs) {
|
||||
const shifted = shiftedFieldName(group, inp.name, row)
|
||||
if (shifted === undefined) continue
|
||||
inp.name = shifted
|
||||
if (inp.widget) inp.widget.name = shifted
|
||||
}
|
||||
|
||||
syncController(group, node)
|
||||
app.canvas?.setDirty(true, true)
|
||||
}
|
||||
|
||||
/** Rebuild the group from scratch to hold exactly `count` rows. */
|
||||
function rebuildRows(group: string, count: number, node: DynamicGroupNode) {
|
||||
const state = node.comfyDynamic.dynamicGroup[group]
|
||||
if (!state) return
|
||||
node.widgets ??= []
|
||||
|
||||
const isRowMember = (name: string) => isGroupField(group, name)
|
||||
for (const w of remove(node.widgets, (w) => isRowMember(w.name)))
|
||||
w.onRemove?.()
|
||||
removeGroupInputs(node, isRowMember)
|
||||
|
||||
const insertAt = node.widgets.findIndex((w) => w.name === group) + 1
|
||||
const rowWidgets: IBaseWidget[] = []
|
||||
for (let row = 0; row < count; row++)
|
||||
rowWidgets.push(...createRow(group, row, state, node))
|
||||
node.widgets.splice(insertAt, 0, ...rowWidgets)
|
||||
}
|
||||
|
||||
function dynamicGroupWidget(
|
||||
node: LGraphNode,
|
||||
inputName: string,
|
||||
untypedInputData: InputSpec,
|
||||
_appArg: ComfyApp
|
||||
) {
|
||||
const parseResult = zDynamicGroupInputSpec.safeParse(untypedInputData)
|
||||
if (!parseResult.success) throw new Error('invalid DynamicGroup spec')
|
||||
const [, { template, min, max, group_name: groupName }] = parseResult.data
|
||||
|
||||
const toSpecs = (
|
||||
inputs: Record<string, InputSpec> | undefined,
|
||||
isOptional: boolean
|
||||
) =>
|
||||
Object.entries(inputs ?? {}).map(([name, spec]) =>
|
||||
transformInputSpecV1ToV2(spec, { name, isOptional })
|
||||
)
|
||||
const inputSpecs = [
|
||||
...toSpecs(template.required, false),
|
||||
...toSpecs(template.optional, true)
|
||||
]
|
||||
|
||||
withComfyDynamicGroup(node)
|
||||
const typedNode = node as DynamicGroupNode
|
||||
typedNode.comfyDynamic.dynamicGroup[inputName] = {
|
||||
min,
|
||||
max,
|
||||
groupName,
|
||||
inputSpecs,
|
||||
addRow: () => addRow(inputName, typedNode),
|
||||
removeRow: (row: number) => removeRow(inputName, row, typedNode)
|
||||
}
|
||||
|
||||
node.widgets ??= []
|
||||
const controller = node.addCustomWidget({
|
||||
name: inputName,
|
||||
type: 'dynamic_group',
|
||||
value: min,
|
||||
y: 0,
|
||||
serialize: true,
|
||||
callback: () => addRow(inputName, typedNode),
|
||||
options: { socketless: true, disabled: false, min, max }
|
||||
})
|
||||
|
||||
Object.defineProperty(controller, 'value', {
|
||||
get() {
|
||||
return countGroupRows(inputName, typedNode)
|
||||
},
|
||||
set(count: unknown) {
|
||||
if (typeof count !== 'number') return
|
||||
const state = typedNode.comfyDynamic.dynamicGroup[inputName]
|
||||
if (!state) return
|
||||
const clamped = Math.min(Math.max(count, state.min), state.max)
|
||||
rebuildRows(inputName, clamped, typedNode)
|
||||
syncController(inputName, typedNode)
|
||||
},
|
||||
configurable: true
|
||||
})
|
||||
|
||||
controller.value = min
|
||||
|
||||
return { widget: controller }
|
||||
}
|
||||
|
||||
export const dynamicWidgets = {
|
||||
COMFY_DYNAMICCOMBO_V3: dynamicComboWidget,
|
||||
COMFY_DYNAMICGROUP_V3: dynamicGroupWidget
|
||||
}
|
||||
export const dynamicWidgets = { COMFY_DYNAMICCOMBO_V3: dynamicComboWidget }
|
||||
const dynamicInputs: Record<
|
||||
string,
|
||||
(node: LGraphNode, inputSpec: InputSpecV2) => void
|
||||
|
||||
@@ -75,7 +75,6 @@ export interface IWidgetOptions<TValues = unknown> {
|
||||
|
||||
// Vue widget options
|
||||
disabled?: boolean
|
||||
removable?: boolean
|
||||
useGrouping?: boolean
|
||||
placeholder?: string
|
||||
showThumbnails?: boolean
|
||||
|
||||
@@ -913,8 +913,8 @@
|
||||
"nodes": "Nodes",
|
||||
"models": "Models",
|
||||
"assets": "Assets",
|
||||
"workflows": "Workflows",
|
||||
"templates": "Templates",
|
||||
"workflows": "Workflows",
|
||||
"templates": "Templates",
|
||||
"console": "Console",
|
||||
"menu": "Menu",
|
||||
"imported": "Imported",
|
||||
@@ -2256,12 +2256,6 @@
|
||||
"slots": "Node Slots Error",
|
||||
"widgets": "Node Widgets Error"
|
||||
},
|
||||
"dynamicGroup": {
|
||||
"addGroup": "Add {group_name}",
|
||||
"removeGroup": "Remove {group_name}",
|
||||
"group": "{group_name} #{index}",
|
||||
"defaultGroupName": "Group"
|
||||
},
|
||||
"oauth": {
|
||||
"consent": {
|
||||
"allow": "Continue",
|
||||
@@ -3106,7 +3100,7 @@
|
||||
"share": "Share"
|
||||
},
|
||||
"shortcuts": {
|
||||
"shortcuts": "Shortcuts",
|
||||
"shortcuts": "Shortcuts",
|
||||
"essentials": "Essential",
|
||||
"viewControls": "View Controls",
|
||||
"manageShortcuts": "Manage Shortcuts",
|
||||
|
||||
@@ -1,310 +0,0 @@
|
||||
import { render, screen } from '@testing-library/vue'
|
||||
import userEvent from '@testing-library/user-event'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { nextTick, reactive } from 'vue'
|
||||
import { createI18n } from 'vue-i18n'
|
||||
|
||||
import type { WidgetSlotMetadata } from '@/composables/graph/useGraphNodeManager'
|
||||
import type { DynamicGroupNode } from '@/core/graph/widgets/dynamicWidgets'
|
||||
import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
|
||||
import type { HasInitialMinSize } from '@/services/litegraphService'
|
||||
import { toNodeId } from '@/types/nodeId'
|
||||
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
|
||||
|
||||
import WidgetDynamicGroup from './WidgetDynamicGroup.vue'
|
||||
|
||||
const appMocks = vi.hoisted(() => ({
|
||||
graph: null as LGraph | null
|
||||
}))
|
||||
|
||||
const lifecycleMocks = vi.hoisted(() => ({
|
||||
manager: null as { vueNodeData: Map<unknown, unknown> } | null
|
||||
}))
|
||||
|
||||
const FieldStub = vi.hoisted(() => ({
|
||||
name: 'FieldStub',
|
||||
props: {
|
||||
modelValue: { type: [String, Number], default: '' },
|
||||
widget: { type: Object, required: true }
|
||||
},
|
||||
emits: ['update:modelValue'],
|
||||
template:
|
||||
'<input data-testid="field" :aria-label="widget.name" :data-disabled="widget.options?.disabled ? \'true\' : \'false\'" :value="modelValue" @input="$emit(\'update:modelValue\', $event.target.value)" />'
|
||||
}))
|
||||
|
||||
vi.mock('@/scripts/app', () => ({
|
||||
app: {
|
||||
get graph() {
|
||||
return appMocks.graph
|
||||
}
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock(
|
||||
'@/renderer/extensions/vueNodes/widgets/registry/widgetRegistry',
|
||||
() => ({
|
||||
getComponent: () => FieldStub
|
||||
})
|
||||
)
|
||||
|
||||
vi.mock('@/composables/graph/useVueNodeLifecycle', () => ({
|
||||
useVueNodeLifecycle: () => ({
|
||||
nodeManager: {
|
||||
get value() {
|
||||
return lifecycleMocks.manager
|
||||
}
|
||||
}
|
||||
})
|
||||
}))
|
||||
|
||||
const ButtonStub = {
|
||||
name: 'Button',
|
||||
props: { disabled: Boolean },
|
||||
template: '<button type="button" :disabled="disabled"><slot /></button>'
|
||||
}
|
||||
|
||||
const i18n = createI18n({
|
||||
legacy: false,
|
||||
locale: 'en',
|
||||
messages: {
|
||||
en: {
|
||||
dynamicGroup: {
|
||||
addGroup: 'Add {group_name}',
|
||||
removeGroup: 'Remove {group_name}',
|
||||
group: '{group_name} #{index}',
|
||||
defaultGroupName: 'Group'
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
function fieldWidget(name: string, value = ''): IBaseWidget {
|
||||
return {
|
||||
name,
|
||||
type: 'string',
|
||||
value,
|
||||
options: {},
|
||||
y: 0
|
||||
}
|
||||
}
|
||||
|
||||
function createDynamicGroupNode({
|
||||
min = 1,
|
||||
max = 3,
|
||||
groupName = 'Lora',
|
||||
fieldsPerRow = ['text'],
|
||||
rows = [0]
|
||||
}: {
|
||||
min?: number
|
||||
max?: number
|
||||
groupName?: string
|
||||
fieldsPerRow?: string[]
|
||||
rows?: number[]
|
||||
} = {}): DynamicGroupNode {
|
||||
const node = new LGraphNode('test') as DynamicGroupNode &
|
||||
Partial<HasInitialMinSize>
|
||||
node._initialMinSize = { width: 1, height: 1 }
|
||||
node.widgets = [
|
||||
{
|
||||
name: 'loras',
|
||||
type: 'dynamic_group',
|
||||
value: rows.length,
|
||||
options: { min, max },
|
||||
y: 0
|
||||
},
|
||||
...rows.flatMap((row) =>
|
||||
fieldsPerRow.map((field) => fieldWidget(`loras.${row}.${field}`))
|
||||
)
|
||||
]
|
||||
|
||||
const state = {
|
||||
min,
|
||||
max,
|
||||
groupName,
|
||||
inputSpecs: [],
|
||||
addRow: vi.fn(),
|
||||
removeRow: vi.fn()
|
||||
}
|
||||
node.comfyDynamic = { dynamicGroup: { loras: state } }
|
||||
|
||||
const graph = new LGraph()
|
||||
graph.add(node)
|
||||
appMocks.graph = graph
|
||||
|
||||
return node
|
||||
}
|
||||
|
||||
function mountWidgetDynamicGroup(node: DynamicGroupNode) {
|
||||
const state = node.comfyDynamic.dynamicGroup.loras
|
||||
const widget: SimplifiedWidget<number> = {
|
||||
name: 'loras',
|
||||
type: 'dynamic_group',
|
||||
value: node.widgets!.filter((w) => w.name.startsWith('loras.')).length,
|
||||
options: { min: state.min, max: state.max }
|
||||
}
|
||||
|
||||
return render(WidgetDynamicGroup, {
|
||||
global: {
|
||||
plugins: [i18n],
|
||||
stubs: { Button: ButtonStub }
|
||||
},
|
||||
props: {
|
||||
widget,
|
||||
nodeId: toNodeId(String(node.id)),
|
||||
nodeType: 'testnode'
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
describe('WidgetDynamicGroup', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia())
|
||||
appMocks.graph = null
|
||||
lifecycleMocks.manager = null
|
||||
})
|
||||
|
||||
it('renders one row per field widget with the configured group name', () => {
|
||||
mountWidgetDynamicGroup(createDynamicGroupNode({ min: 2, rows: [0, 1] }))
|
||||
|
||||
expect(screen.getByText('Lora #1')).toBeInTheDocument()
|
||||
expect(screen.getByText('Lora #2')).toBeInTheDocument()
|
||||
expect(screen.getAllByTestId('field')).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('renders multiple fields per row', () => {
|
||||
mountWidgetDynamicGroup(
|
||||
createDynamicGroupNode({
|
||||
rows: [0, 1],
|
||||
fieldsPerRow: ['text', 'strength']
|
||||
})
|
||||
)
|
||||
|
||||
expect(screen.getAllByTestId('field')).toHaveLength(4)
|
||||
expect(
|
||||
screen.getByRole('textbox', { name: 'loras.0.text' })
|
||||
).toBeInTheDocument()
|
||||
expect(
|
||||
screen.getByRole('textbox', { name: 'loras.1.strength' })
|
||||
).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('calls addRow when the add button is clicked', async () => {
|
||||
const node = createDynamicGroupNode({ min: 1, max: 3 })
|
||||
const user = userEvent.setup()
|
||||
|
||||
mountWidgetDynamicGroup(node)
|
||||
await user.click(screen.getByRole('button', { name: 'Add Lora' }))
|
||||
|
||||
expect(node.comfyDynamic.dynamicGroup.loras.addRow).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('calls removeRow with the correct row index', async () => {
|
||||
const node = createDynamicGroupNode({ min: 0, max: 3, rows: [0, 1, 2] })
|
||||
const user = userEvent.setup()
|
||||
|
||||
mountWidgetDynamicGroup(node)
|
||||
const removeButtons = screen.getAllByRole('button', { name: 'Remove Lora' })
|
||||
await user.click(removeButtons[2]!)
|
||||
|
||||
expect(node.comfyDynamic.dynamicGroup.loras.removeRow).toHaveBeenCalledWith(
|
||||
2
|
||||
)
|
||||
})
|
||||
|
||||
it('shows a remove button for every row while above the minimum count', () => {
|
||||
mountWidgetDynamicGroup(createDynamicGroupNode({ min: 2, rows: [0, 1, 2] }))
|
||||
|
||||
const removeButtons = screen.getAllByRole('button', { name: 'Remove Lora' })
|
||||
expect(removeButtons).toHaveLength(3)
|
||||
})
|
||||
|
||||
it('hides all remove buttons when row count equals min', () => {
|
||||
mountWidgetDynamicGroup(createDynamicGroupNode({ min: 1, rows: [0] }))
|
||||
|
||||
expect(
|
||||
screen.queryByRole('button', { name: 'Remove Lora' })
|
||||
).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('disables the add button when the group is at max capacity', () => {
|
||||
mountWidgetDynamicGroup(
|
||||
createDynamicGroupNode({ min: 0, max: 2, rows: [0, 1] })
|
||||
)
|
||||
|
||||
expect(screen.getByRole('button', { name: 'Add Lora' })).toBeDisabled()
|
||||
})
|
||||
|
||||
it('enables the add button when below max capacity', () => {
|
||||
mountWidgetDynamicGroup(
|
||||
createDynamicGroupNode({ min: 0, max: 3, rows: [0, 1] })
|
||||
)
|
||||
|
||||
expect(screen.getByRole('button', { name: 'Add Lora' })).not.toBeDisabled()
|
||||
})
|
||||
|
||||
it('updates a field widget value when edited', async () => {
|
||||
const node = createDynamicGroupNode({ rows: [0] })
|
||||
const rowWidget = node.widgets!.find((w) => w.name === 'loras.0.text')!
|
||||
const user = userEvent.setup()
|
||||
|
||||
mountWidgetDynamicGroup(node)
|
||||
|
||||
const field = screen.getByTestId('field')
|
||||
await user.clear(field)
|
||||
await user.type(field, 'my-lora')
|
||||
|
||||
expect(rowWidget.value).toBe('my-lora')
|
||||
})
|
||||
|
||||
it('disables a field as soon as its input slot becomes connected', async () => {
|
||||
const node = createDynamicGroupNode({ rows: [0] })
|
||||
const slotMetadata = reactive<WidgetSlotMetadata>({
|
||||
index: 0,
|
||||
linked: false,
|
||||
type: 'STRING'
|
||||
})
|
||||
lifecycleMocks.manager = {
|
||||
vueNodeData: new Map([
|
||||
[node.id, { widgets: [{ name: 'loras.0.text', slotMetadata }] }]
|
||||
])
|
||||
}
|
||||
|
||||
mountWidgetDynamicGroup(node)
|
||||
expect(screen.getByTestId('field')).toHaveAttribute(
|
||||
'data-disabled',
|
||||
'false'
|
||||
)
|
||||
|
||||
slotMetadata.linked = true
|
||||
await nextTick()
|
||||
|
||||
expect(screen.getByTestId('field')).toHaveAttribute('data-disabled', 'true')
|
||||
})
|
||||
|
||||
it('uses the default group name when groupName is not configured', () => {
|
||||
const node = createDynamicGroupNode({ rows: [0] })
|
||||
node.comfyDynamic.dynamicGroup.loras.groupName = undefined
|
||||
|
||||
mountWidgetDynamicGroup(node)
|
||||
|
||||
expect(screen.getByText('Group #1')).toBeInTheDocument()
|
||||
expect(
|
||||
screen.getByRole('button', { name: 'Add Group' })
|
||||
).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('renders nothing when the node is not on the graph', () => {
|
||||
const node = createDynamicGroupNode({ rows: [0, 1] })
|
||||
appMocks.graph = null
|
||||
|
||||
mountWidgetDynamicGroup(node)
|
||||
|
||||
expect(screen.queryAllByTestId('field')).toHaveLength(0)
|
||||
expect(
|
||||
screen.getByRole('button', { name: 'Add Group' })
|
||||
).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
@@ -1,253 +0,0 @@
|
||||
<template>
|
||||
<div
|
||||
class="col-span-2 grid grid-cols-[min-content_minmax(80px,min-content)_minmax(125px,1fr)] gap-x-2 gap-y-1"
|
||||
>
|
||||
<template v-for="row in rowIndices" :key="row">
|
||||
<div
|
||||
class="col-span-full mt-1 flex items-center justify-between border-t border-node-component-surface pt-1"
|
||||
>
|
||||
<span
|
||||
class="truncate text-xs font-medium text-node-component-slot-text"
|
||||
>
|
||||
{{
|
||||
t('dynamicGroup.group', { group_name: groupName, index: row + 1 })
|
||||
}}
|
||||
</span>
|
||||
<button
|
||||
v-if="canRemoveRows"
|
||||
v-tooltip.top="
|
||||
t('dynamicGroup.removeGroup', { group_name: groupName })
|
||||
"
|
||||
type="button"
|
||||
class="mr-1.75 flex cursor-pointer appearance-none border-0 bg-transparent p-0 text-node-component-slot-text/40 transition-colors duration-150 hover:text-danger-100 focus-visible:outline-none"
|
||||
:aria-label="t('dynamicGroup.removeGroup', { group_name: groupName })"
|
||||
@click="onRemoveRow(row)"
|
||||
>
|
||||
<span
|
||||
class="icon-[material-symbols--close] size-4"
|
||||
aria-hidden="true"
|
||||
/>
|
||||
</button>
|
||||
</div>
|
||||
<div
|
||||
v-for="fw in rowWidgets(row)"
|
||||
:key="fw.name"
|
||||
class="group col-span-full grid grid-cols-subgrid items-stretch"
|
||||
>
|
||||
<div
|
||||
:class="
|
||||
cn(
|
||||
'z-10 -ml-3 flex w-3 items-stretch opacity-0 transition-opacity duration-150 group-hover:opacity-100',
|
||||
fw.linked && 'opacity-100'
|
||||
)
|
||||
"
|
||||
>
|
||||
<InputSlot
|
||||
v-if="fw.slotData && fw.inputIndex !== undefined"
|
||||
:slot-data="fw.slotData"
|
||||
:node-id="resolvedNodeId"
|
||||
:node-type="nodeType"
|
||||
:index="fw.inputIndex"
|
||||
:connected="fw.linked"
|
||||
dot-only
|
||||
/>
|
||||
</div>
|
||||
<component
|
||||
:is="fw.component"
|
||||
:model-value="fw.value"
|
||||
:widget="fw.simplified"
|
||||
:node-id="nodeId"
|
||||
:node-type="nodeType"
|
||||
class="col-span-2"
|
||||
@update:model-value="fw.onUpdate"
|
||||
/>
|
||||
</div>
|
||||
</template>
|
||||
<Button
|
||||
:disabled="addDisabled"
|
||||
class="col-span-full mt-1 border-0 bg-component-node-widget-background text-node-component-slot-text"
|
||||
size="sm"
|
||||
variant="textonly"
|
||||
@click="onAddRow"
|
||||
>
|
||||
<span
|
||||
class="mr-1 icon-[material-symbols--add] size-4"
|
||||
aria-hidden="true"
|
||||
/>
|
||||
{{ t('dynamicGroup.addGroup', { group_name: groupName }) }}
|
||||
</Button>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
<script setup lang="ts">
|
||||
import { computed } from 'vue'
|
||||
import type { Component } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
|
||||
import Button from '@/components/ui/button/Button.vue'
|
||||
import type { WidgetSlotMetadata } from '@/composables/graph/useGraphNodeManager'
|
||||
import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
|
||||
import type { DynamicGroupNode } from '@/core/graph/widgets/dynamicWidgets'
|
||||
import type { INodeSlot } from '@/lib/litegraph/src/interfaces'
|
||||
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
|
||||
import InputSlot from '@/renderer/extensions/vueNodes/components/InputSlot.vue'
|
||||
import { getComponent } from '@/renderer/extensions/vueNodes/widgets/registry/widgetRegistry'
|
||||
import WidgetLegacy from '@/renderer/extensions/vueNodes/widgets/components/WidgetLegacy.vue'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useNodeDefStore } from '@/stores/nodeDefStore'
|
||||
import {
|
||||
stripGraphPrefix,
|
||||
useWidgetValueStore
|
||||
} from '@/stores/widgetValueStore'
|
||||
import type { SimplifiedWidget, WidgetValue } from '@/types/simplifiedWidget'
|
||||
import type { WidgetState } from '@/types/widgetState'
|
||||
import { toNodeId } from '@/types/nodeId'
|
||||
import type { NodeId } from '@/types/nodeId'
|
||||
import { widgetId } from '@/types/widgetId'
|
||||
import { cn } from '@comfyorg/tailwind-utils'
|
||||
|
||||
const { widget, nodeId, nodeType } = defineProps<{
|
||||
widget: SimplifiedWidget<number>
|
||||
nodeId: string
|
||||
nodeType?: string
|
||||
}>()
|
||||
|
||||
const { t } = useI18n()
|
||||
const widgetValueStore = useWidgetValueStore()
|
||||
const nodeDefStore = useNodeDefStore()
|
||||
const { nodeManager } = useVueNodeLifecycle()
|
||||
|
||||
const group = widget.name
|
||||
|
||||
const node = computed(() => {
|
||||
const graph = app.canvas?.graph ?? app.graph
|
||||
return graph?.getNodeById(toNodeId(nodeId)) as DynamicGroupNode | undefined
|
||||
})
|
||||
|
||||
const groupState = computed(
|
||||
() => node.value?.comfyDynamic?.dynamicGroup?.[group]
|
||||
)
|
||||
|
||||
// Reading `input.link` off the raw litegraph node isn't reactive, so widgets
|
||||
// wouldn't gray out on connect. The graph node manager tracks link changes and
|
||||
// exposes them via `slotMetadata`, which updates on `node:slot-links:changed`.
|
||||
const slotMetadataByName = computed(() => {
|
||||
const map = new Map<string, WidgetSlotMetadata>()
|
||||
const id = node.value?.id
|
||||
if (id == null) return map
|
||||
for (const w of nodeManager.value?.vueNodeData.get(id)?.widgets ?? []) {
|
||||
if (w.slotMetadata) map.set(w.name, w.slotMetadata)
|
||||
}
|
||||
return map
|
||||
})
|
||||
|
||||
const minRows = computed(() => groupState.value?.min ?? 0)
|
||||
const groupName = computed(
|
||||
() => groupState.value?.groupName ?? t('dynamicGroup.defaultGroupName')
|
||||
)
|
||||
|
||||
interface FieldWidgetView {
|
||||
name: string
|
||||
row: number
|
||||
component: Component
|
||||
simplified: SimplifiedWidget
|
||||
value: WidgetValue
|
||||
onUpdate: (value: WidgetValue) => void
|
||||
slotData?: INodeSlot
|
||||
inputIndex?: number
|
||||
linked: boolean
|
||||
}
|
||||
|
||||
function resolveWidgetState(w: IBaseWidget): WidgetState | undefined {
|
||||
if (w.widgetId) return widgetValueStore.getWidget(w.widgetId)
|
||||
const graphId = node.value?.graph?.rootGraph?.id
|
||||
if (!graphId) return undefined
|
||||
const localId = stripGraphPrefix(String(nodeId))
|
||||
if (!localId) return undefined
|
||||
return widgetValueStore.getWidget(widgetId(graphId, localId, w.name))
|
||||
}
|
||||
|
||||
function toFieldView(
|
||||
n: DynamicGroupNode,
|
||||
w: IBaseWidget,
|
||||
row: number,
|
||||
fieldName: string
|
||||
): FieldWidgetView {
|
||||
const state = resolveWidgetState(w)
|
||||
const value = state?.value ?? w.value
|
||||
const slotMeta = slotMetadataByName.value.get(w.name)
|
||||
const inputIndex =
|
||||
slotMeta?.index ??
|
||||
n.inputs.findIndex(
|
||||
(input) => input.name === w.name || input.widget?.name === w.name
|
||||
)
|
||||
const slotData = inputIndex === -1 ? undefined : n.inputs[inputIndex]
|
||||
const linked = slotMeta?.linked ?? slotData?.link != null
|
||||
const simplified: SimplifiedWidget = {
|
||||
name: w.name,
|
||||
type: state?.type ?? w.type,
|
||||
value,
|
||||
label: state?.label ?? w.label ?? fieldName,
|
||||
options: linked
|
||||
? { ...(state?.options ?? w.options), disabled: true }
|
||||
: (state?.options ?? w.options),
|
||||
spec: nodeDefStore.getInputSpecForWidget(n, w.name)
|
||||
}
|
||||
return {
|
||||
name: w.name,
|
||||
row,
|
||||
component: getComponent(w.type) ?? WidgetLegacy,
|
||||
simplified,
|
||||
value,
|
||||
onUpdate: (next: WidgetValue) => {
|
||||
if (state) state.value = next
|
||||
w.value = next ?? undefined
|
||||
w.callback?.(next)
|
||||
},
|
||||
slotData,
|
||||
inputIndex: inputIndex === -1 ? undefined : inputIndex,
|
||||
linked
|
||||
}
|
||||
}
|
||||
|
||||
const fieldWidgets = computed<FieldWidgetView[]>(() => {
|
||||
const n = node.value
|
||||
if (!n?.widgets) return []
|
||||
const prefix = `${group}.`
|
||||
const views: FieldWidgetView[] = []
|
||||
for (const w of n.widgets) {
|
||||
if (!w.name.startsWith(prefix)) continue
|
||||
const rest = w.name.slice(prefix.length)
|
||||
const dot = rest.indexOf('.')
|
||||
if (dot === -1) continue
|
||||
const row = Number(rest.slice(0, dot))
|
||||
if (!Number.isInteger(row)) continue
|
||||
views.push(toFieldView(n, w, row, rest.slice(dot + 1)))
|
||||
}
|
||||
return views
|
||||
})
|
||||
|
||||
const rowIndices = computed(() =>
|
||||
[...new Set(fieldWidgets.value.map((fw) => fw.row))].sort((a, b) => a - b)
|
||||
)
|
||||
|
||||
const addDisabled = computed(
|
||||
() => rowIndices.value.length >= (groupState.value?.max ?? Infinity)
|
||||
)
|
||||
|
||||
const canRemoveRows = computed(() => rowIndices.value.length > minRows.value)
|
||||
|
||||
const resolvedNodeId = computed<NodeId>(() => toNodeId(nodeId))
|
||||
|
||||
function rowWidgets(row: number): FieldWidgetView[] {
|
||||
return fieldWidgets.value.filter((fw) => fw.row === row)
|
||||
}
|
||||
|
||||
function onAddRow() {
|
||||
groupState.value?.addRow()
|
||||
}
|
||||
|
||||
function onRemoveRow(row: number) {
|
||||
groupState.value?.removeRow(row)
|
||||
}
|
||||
</script>
|
||||
@@ -78,10 +78,6 @@ const WidgetBoundingBoxes = defineAsyncComponent(
|
||||
const WidgetColors = defineAsyncComponent(
|
||||
() => import('@/components/palette/WidgetColors.vue')
|
||||
)
|
||||
const WidgetDynamicGroup = defineAsyncComponent(
|
||||
() =>
|
||||
import('@/renderer/extensions/vueNodes/widgets/components/WidgetDynamicGroup.vue')
|
||||
)
|
||||
|
||||
export const FOR_TESTING = {
|
||||
WidgetButton,
|
||||
@@ -256,14 +252,6 @@ const coreWidgetDefinitions: Array<[string, WidgetDefinition]> = [
|
||||
aliases: ['COLORS'],
|
||||
essential: false
|
||||
}
|
||||
],
|
||||
[
|
||||
'dynamic_group',
|
||||
{
|
||||
component: WidgetDynamicGroup,
|
||||
aliases: [],
|
||||
essential: false
|
||||
}
|
||||
]
|
||||
]
|
||||
|
||||
|
||||
@@ -304,7 +304,6 @@ export const useLitegraphService = () => {
|
||||
hidden: inputSpec.hidden
|
||||
})
|
||||
if (inputSpec.hidden !== undefined) widget.hidden = inputSpec.hidden
|
||||
if (inputSpec.socketless) widget.options.socketless = true
|
||||
if (dynamic) widget.tooltip = inputSpec.tooltip
|
||||
}
|
||||
|
||||
|
||||
@@ -182,35 +182,6 @@ describe('useWidgetValueStore', () => {
|
||||
expect(store.getWidget(seedA)).toBeUndefined()
|
||||
expect(store.deleteWidget(seedA)).toBe(false)
|
||||
})
|
||||
|
||||
it('renameWidget moves state to the new id and preserves object identity', () => {
|
||||
const store = useWidgetValueStore()
|
||||
const renamed = widgetId(graphA, toNodeId('node-1'), 'renamed')
|
||||
const original = store.registerWidget(seedA, state('number', 100))
|
||||
|
||||
expect(store.renameWidget(seedA, renamed)).toBe(true)
|
||||
expect(store.getWidget(seedA)).toBeUndefined()
|
||||
|
||||
const moved = store.getWidget(renamed)
|
||||
expect(moved).toBe(original)
|
||||
expect(moved?.value).toBe(100)
|
||||
expect(moved?.name).toBe('renamed')
|
||||
})
|
||||
|
||||
it('renameWidget discards any state already stored at the target id', () => {
|
||||
const store = useWidgetValueStore()
|
||||
const target = widgetId(graphA, toNodeId('node-1'), 'target')
|
||||
store.registerWidget(seedA, state('number', 1))
|
||||
store.registerWidget(target, state('number', 2))
|
||||
|
||||
expect(store.renameWidget(seedA, target)).toBe(true)
|
||||
expect(store.getWidget(target)?.value).toBe(1)
|
||||
})
|
||||
|
||||
it('renameWidget returns false for an unknown source', () => {
|
||||
const store = useWidgetValueStore()
|
||||
expect(store.renameWidget(seedA, seedB)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('direct property mutation', () => {
|
||||
|
||||
@@ -60,24 +60,6 @@ export const useWidgetValueStore = defineStore('widgetValue', () => {
|
||||
return getGraphWidgetStates(graphId).delete(widgetId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Moves an existing widget state to a new id, preserving the state object
|
||||
* identity so any widget holding a reference to it stays in sync. Any state
|
||||
* already stored under `toId` is discarded.
|
||||
*/
|
||||
function renameWidget(fromId: WidgetId, toId: WidgetId): boolean {
|
||||
if (fromId === toId) return false
|
||||
const { graphId: fromGraphId } = parseWidgetId(fromId)
|
||||
const state = getGraphWidgetStates(fromGraphId).get(fromId)
|
||||
if (!state) return false
|
||||
|
||||
const { graphId: toGraphId, name } = parseWidgetId(toId)
|
||||
getGraphWidgetStates(fromGraphId).delete(fromId)
|
||||
state.name = name
|
||||
getGraphWidgetStates(toGraphId).set(toId, state)
|
||||
return true
|
||||
}
|
||||
|
||||
function getNodeWidgets(graphId: UUID, localNodeId: NodeId): WidgetState[] {
|
||||
return [...getGraphWidgetStates(graphId).values()].filter(
|
||||
(state) => state.nodeId === localNodeId
|
||||
@@ -93,7 +75,6 @@ export const useWidgetValueStore = defineStore('widgetValue', () => {
|
||||
getWidget,
|
||||
setValue,
|
||||
deleteWidget,
|
||||
renameWidget,
|
||||
getNodeWidgets,
|
||||
clearGraph
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user