Compare commits

..

2 Commits

Author SHA1 Message Date
huang47
b221925b32 fix: harden contribution policy checks 2026-07-12 15:30:04 -07:00
huang47
83d8d13bbc ci: enforce contribution readiness 2026-07-12 00:05:06 -07:00
37 changed files with 2908 additions and 1334 deletions

View File

@@ -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: |

View File

@@ -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:

View File

@@ -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:

View File

@@ -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. -->

View File

@@ -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: |

View File

@@ -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
View 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
View 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"

View File

@@ -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

View File

@@ -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

View File

@@ -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: |

View File

@@ -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: |

View File

@@ -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: |

View File

@@ -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

View File

@@ -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?

View File

@@ -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')
})
})

View File

@@ -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'),

View 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'
})
})
})

View 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: ![before](https://github.com/user-attachments/assets/11111111-1111-4111-8111-111111111111)\nAfter: ![after](https://github.com/user-attachments/assets/22222222-2222-4222-8222-222222222222)'
})
)
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:
'![Screenshot](https://github.com/user-attachments/assets/77777777-7777-4777-8777-777777777777)'
})
)
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)
})
})

View 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
})
}

View 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 })
}
})
})

View 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
})
}

View 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 })
}
})
})

View 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
})
}

View File

@@ -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
}
}

View File

@@ -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([])
})
})

View File

@@ -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 ?? {}

View File

@@ -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')
})
})

View File

@@ -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

View File

@@ -75,7 +75,6 @@ export interface IWidgetOptions<TValues = unknown> {
// Vue widget options
disabled?: boolean
removable?: boolean
useGrouping?: boolean
placeholder?: string
showThumbnails?: boolean

View File

@@ -913,8 +913,8 @@
"nodes": "Nodes",
"models": "Models",
"assets": "Assets",
"workflows": "Workflows",
"templates": "Templates",
"workflows": "Work­flows",
"templates": "Tem­plates",
"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": "Short­cuts",
"essentials": "Essential",
"viewControls": "View Controls",
"manageShortcuts": "Manage Shortcuts",

View File

@@ -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()
})
})

View File

@@ -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>

View File

@@ -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
}
]
]

View File

@@ -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
}

View File

@@ -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', () => {

View File

@@ -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
}