diff --git a/README.md b/README.md index f9d14a89..c278a6d5 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ The match object allows control over the matching options. You can specify the l The base match object is defined as: ```yml -- changed-files: +- changed-files: - any-glob-to-any-file: ['list', 'of', 'globs'] - any-glob-to-all-files: ['list', 'of', 'globs'] - all-globs-to-any-file: ['list', 'of', 'globs'] @@ -132,7 +132,7 @@ Documentation: - changed-files: - any-glob-to-any-file: ['docs/*', 'guides/*'] -# Add 'Documentation' label to any change to .md files within the entire repository +# Add 'Documentation' label to any change to .md files within the entire repository Documentation: - changed-files: - any-glob-to-any-file: '**/*.md' @@ -153,6 +153,96 @@ release: - base-branch: 'main' ``` +#### Configuration Options + +The labeler configuration file (`.github/labeler.yml`) supports the following top-level options: + +| Name | Description | +|------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `changed-files-labels-limit` | Maximum number of new labels to apply based on changed files (must be a non-negative integer). If exceeded, no changed-files labels are applied for that run. | +| `max-files-changed` | Maximum number of total changed files (must be a non-negative integer). If exceeded, all file-based labeling is skipped. | + +##### Limiting changed-files labels + +When working with large PRs (e.g., tree-wide refactors) that touch many components, you may want to prevent the labeler from adding too many labels. Set `changed-files-labels-limit` in your `.github/labeler.yml` configuration file to limit the number of labels that can be applied based on changed files patterns. + +**Important behaviors:** + +- The limit counts only **new** labels that would be added by changed-files rules. Labels already present on the PR are not counted toward the limit. +- If the number of new changed-files labels **exceeds** the limit, **all** new changed-files labels are skipped for that run. +- If the number of new changed-files labels **equals** the limit, labels are still applied normally. +- Labels based on branch conditions (`head-branch`, `base-branch`) are **not affected** by the limit. +- **Any label definition that includes a `changed-files` rule is considered a changed-files label** and is subject to the limit, regardless of which condition caused the label to match. For example, a label with both `head-branch` and `changed-files` rules will be subject to the limit even if it matches via the branch rule. +- If both `max-files-changed` and `changed-files-labels-limit` are configured at the same time, `max-files-changed` is evaluated first, and if it triggers, `changed-files-labels-limit` is not evaluated. + +##### Example + +```yml +# .github/labeler.yml + +# Limit changed-files based labels to 5 +changed-files-labels-limit: 5 + +# Label definitions - these are subject to the limit +frontend: + - changed-files: + - any-glob-to-any-file: 'src/frontend/**' + +backend: + - changed-files: + - any-glob-to-any-file: 'src/backend/**' + +docs: + - changed-files: + - any-glob-to-any-file: 'docs/**' + +# This label has both branch and changed-files rules. +# It is still subject to the limit because it includes changed-files. +mixed: + - any: + - head-branch: '^feature/' + - changed-files: + - any-glob-to-any-file: 'src/mixed/**' + +# Branch-based labels are NOT affected by the limit +feature: + - head-branch: '^feature/' +``` + +##### Skipping labeling for large PRs + +When a PR modifies a very large number of files (e.g., tree-wide refactors, automated code formatting), you may want to skip file-based labeling entirely. Set `max-files-changed` in your `.github/labeler.yml` configuration file to skip all file-based labeling when the total number of changed files exceeds the threshold. + +**Important behaviors:** + +- If the total number of changed files **exceeds** the limit, all file-based labeling is skipped entirely. +- If the total number of changed files **equals** the limit, labels are still applied normally. +- Labels based **only** on branch conditions (`head-branch`, `base-branch`) are **not affected** by the limit. +- **Any label definition that includes a `changed-files` rule is considered a file-based label** and will be skipped, regardless of which condition caused the label to match. For example, a label with both `head-branch` and `changed-files` rules will be skipped even if it would match via the branch rule. +- Pre-existing labels on the PR are **preserved** — changed-files configs are not evaluated at all, so `sync-labels` will not remove them. + +##### Example + +```yml +# .github/labeler.yml + +# Skip file-based labeling if more than 100 files changed +max-files-changed: 100 + +# These labels will be skipped if > 100 files changed +frontend: + - changed-files: + - any-glob-to-any-file: 'src/frontend/**' + +backend: + - changed-files: + - any-glob-to-any-file: 'src/backend/**' + +# Branch-based labels are NOT affected +release: + - base-branch: 'main' +``` + ### Create Workflow Create a workflow (e.g. `.github/workflows/labeler.yml` see [Creating a Workflow file](https://docs.github.com/en/actions/writing-workflows/quickstart#creating-your-first-workflow)) to utilize the labeler action with content: @@ -213,10 +303,10 @@ jobs: pull-requests: write runs-on: ubuntu-latest steps: - + # Label PRs 1, 2, and 3 - uses: actions/labeler@v6 - with: + with: pr-number: | 1 2 @@ -225,9 +315,9 @@ jobs: **Note:** in normal usage the `pr-number` input is not required as the action will detect the PR number from the workflow context. -#### Outputs +#### Outputs -Labeler provides the following outputs: +Labeler provides the following outputs: | Name | Description | |--------------|-----------------------------------------------------------| @@ -249,13 +339,13 @@ jobs: steps: - id: label-the-PR uses: actions/labeler@v6 - + - id: run-frontend-tests if: contains(steps.label-the-PR.outputs.all-labels, 'frontend') run: | echo "Running frontend tests..." # Put your commands for running frontend tests here - + - id: run-backend-tests if: contains(steps.label-the-PR.outputs.all-labels, 'backend') run: | @@ -291,7 +381,7 @@ To ensure the action works correctly, include the following permissions in your issues: write ``` -### Manual Label Creation as an Alternative to Granting issues write Permission +### Manual Label Creation as an Alternative to Granting issues write Permission If you prefer not to grant the `issues: write` permission in your workflow, you can manually create all required labels in the repository before the action runs. diff --git a/__tests__/fixtures/limit_0.yml b/__tests__/fixtures/limit_0.yml new file mode 100644 index 00000000..44fd3a4e --- /dev/null +++ b/__tests__/fixtures/limit_0.yml @@ -0,0 +1,18 @@ +# Limit to 0 changed-files labels (none allowed) +changed-files-labels-limit: 0 + +# Labels based on changed files +component-a: + - changed-files: + - any-glob-to-any-file: ['components/a/**'] + +component-b: + - changed-files: + - any-glob-to-any-file: ['components/b/**'] + +# Labels based on branch patterns only +test-branch: + - head-branch: '^test/' + +feature-branch: + - head-branch: '/feature/' diff --git a/__tests__/fixtures/limit_1.yml b/__tests__/fixtures/limit_1.yml new file mode 100644 index 00000000..5649d0cb --- /dev/null +++ b/__tests__/fixtures/limit_1.yml @@ -0,0 +1,26 @@ +# Limit to 1 changed-files label +changed-files-labels-limit: 1 + +# Labels based on changed files +component-a: + - changed-files: + - any-glob-to-any-file: ['components/a/**'] + +component-b: + - changed-files: + - any-glob-to-any-file: ['components/b/**'] + +component-c: + - changed-files: + - any-glob-to-any-file: ['components/c/**'] + +component-d: + - changed-files: + - any-glob-to-any-file: ['components/d/**'] + +# Labels based on branch patterns only +test-branch: + - head-branch: '^test/' + +feature-branch: + - head-branch: '/feature/' diff --git a/__tests__/fixtures/limit_2.yml b/__tests__/fixtures/limit_2.yml new file mode 100644 index 00000000..46f3f715 --- /dev/null +++ b/__tests__/fixtures/limit_2.yml @@ -0,0 +1,26 @@ +# Limit to 2 changed-files labels +changed-files-labels-limit: 2 + +# Labels based on changed files +component-a: + - changed-files: + - any-glob-to-any-file: ['components/a/**'] + +component-b: + - changed-files: + - any-glob-to-any-file: ['components/b/**'] + +component-c: + - changed-files: + - any-glob-to-any-file: ['components/c/**'] + +component-d: + - changed-files: + - any-glob-to-any-file: ['components/d/**'] + +# Labels based on branch patterns only +test-branch: + - head-branch: '^test/' + +feature-branch: + - head-branch: '/feature/' diff --git a/__tests__/fixtures/limit_3.yml b/__tests__/fixtures/limit_3.yml new file mode 100644 index 00000000..b6da1248 --- /dev/null +++ b/__tests__/fixtures/limit_3.yml @@ -0,0 +1,26 @@ +# Limit to 3 changed-files labels +changed-files-labels-limit: 3 + +# Labels based on changed files +component-a: + - changed-files: + - any-glob-to-any-file: ['components/a/**'] + +component-b: + - changed-files: + - any-glob-to-any-file: ['components/b/**'] + +component-c: + - changed-files: + - any-glob-to-any-file: ['components/c/**'] + +component-d: + - changed-files: + - any-glob-to-any-file: ['components/d/**'] + +# Labels based on branch patterns only +test-branch: + - head-branch: '^test/' + +feature-branch: + - head-branch: '/feature/' diff --git a/__tests__/fixtures/max_files_5.yml b/__tests__/fixtures/max_files_5.yml new file mode 100644 index 00000000..fdff9a70 --- /dev/null +++ b/__tests__/fixtures/max_files_5.yml @@ -0,0 +1,15 @@ +# Skip file-based labeling if more than 5 files changed +max-files-changed: 5 + +# Labels based on changed files +component-a: + - changed-files: + - any-glob-to-any-file: ['components/a/**'] + +component-b: + - changed-files: + - any-glob-to-any-file: ['components/b/**'] + +component-c: + - changed-files: + - any-glob-to-any-file: ['components/c/**'] diff --git a/__tests__/fixtures/max_files_with_branch.yml b/__tests__/fixtures/max_files_with_branch.yml new file mode 100644 index 00000000..e52a14e5 --- /dev/null +++ b/__tests__/fixtures/max_files_with_branch.yml @@ -0,0 +1,15 @@ +# Skip file-based labeling if more than 3 files changed +max-files-changed: 3 + +# Labels based on changed files +component-a: + - changed-files: + - any-glob-to-any-file: ['components/a/**'] + +component-b: + - changed-files: + - any-glob-to-any-file: ['components/b/**'] + +# Branch-based label (should not be affected) +test-branch: + - head-branch: ['^test/'] diff --git a/__tests__/fixtures/mixed_labels.yml b/__tests__/fixtures/mixed_labels.yml new file mode 100644 index 00000000..992dbd77 --- /dev/null +++ b/__tests__/fixtures/mixed_labels.yml @@ -0,0 +1,23 @@ +# Labels based on changed files +component-a: + - changed-files: + - any-glob-to-any-file: ['components/a/**'] + +component-b: + - changed-files: + - any-glob-to-any-file: ['components/b/**'] + +component-c: + - changed-files: + - any-glob-to-any-file: ['components/c/**'] + +component-d: + - changed-files: + - any-glob-to-any-file: ['components/d/**'] + +# Labels based on branch patterns only +test-branch: + - head-branch: '^test/' + +feature-branch: + - head-branch: '/feature/' diff --git a/__tests__/fixtures/mixed_rules.yml b/__tests__/fixtures/mixed_rules.yml new file mode 100644 index 00000000..3f5ecf04 --- /dev/null +++ b/__tests__/fixtures/mixed_rules.yml @@ -0,0 +1,16 @@ +# Test fixture for mixed rules behavior +# A label with both branch and changed-files rules is considered a "changed-files label" +# and is subject to the limit, even if it matches via the branch rule +changed-files-labels-limit: 0 + +# This label has both branch and changed-files rules +# It should be subject to the limit even if matched via branch +mixed-label: + - any: + - head-branch: '^test/' + - changed-files: + - any-glob-to-any-file: ['components/a/**'] + +# Pure branch-based label - not subject to limit +pure-branch-label: + - head-branch: '^test/' diff --git a/__tests__/labeler.test.ts b/__tests__/labeler.test.ts index 75d25ef2..b3f31b4c 100644 --- a/__tests__/labeler.test.ts +++ b/__tests__/labeler.test.ts @@ -9,7 +9,9 @@ import { MatchConfig, toMatchConfig, getLabelConfigMapFromObject, - BaseMatchConfig + getLabelConfigResultFromObject, + BaseMatchConfig, + configUsesChangedFiles } from '../src/api/get-label-configs'; jest.mock('@actions/core'); @@ -60,6 +62,204 @@ describe('getLabelConfigMapFromObject', () => { const result = getLabelConfigMapFromObject(yamlObject); expect(result).toEqual(expected); }); + + it('ignores top-level options like changed-files-labels-limit and max-files-changed', () => { + const configWithLimit = { + 'changed-files-labels-limit': 5, + 'max-files-changed': 100, + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigMapFromObject(configWithLimit); + expect(result.has('changed-files-labels-limit')).toBe(false); + expect(result.has('max-files-changed')).toBe(false); + expect(result.has('label1')).toBe(true); + }); +}); + +describe('getLabelConfigResultFromObject', () => { + it('extracts changed-files-labels-limit as a number', () => { + const config = { + 'changed-files-labels-limit': 5, + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.changedFilesLimit).toBe(5); + expect(result.labelConfigs.has('label1')).toBe(true); + }); + + it('parses changed-files-labels-limit from string', () => { + const config = { + 'changed-files-labels-limit': '10', + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.changedFilesLimit).toBe(10); + }); + + it('trims whitespace when parsing string values', () => { + const config = { + 'changed-files-labels-limit': ' 5 ', + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.changedFilesLimit).toBe(5); + }); + + it('returns undefined changedFilesLimit when not set', () => { + const config = { + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.changedFilesLimit).toBeUndefined(); + }); + + it('throws error for invalid changed-files-labels-limit value', () => { + const config = { + 'changed-files-labels-limit': 'invalid', + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + expect(() => getLabelConfigResultFromObject(config)).toThrow( + /Invalid value for 'changed-files-labels-limit'/ + ); + }); + + it('throws error for negative changed-files-labels-limit value', () => { + const config = { + 'changed-files-labels-limit': -1, + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + expect(() => getLabelConfigResultFromObject(config)).toThrow( + /must be a non-negative integer/ + ); + }); + + it('throws error for string with trailing characters', () => { + const config = { + 'changed-files-labels-limit': '10abc', + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + expect(() => getLabelConfigResultFromObject(config)).toThrow( + /must be a non-negative integer/ + ); + }); + + it('throws error for decimal string', () => { + const config = { + 'changed-files-labels-limit': '3.2', + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + expect(() => getLabelConfigResultFromObject(config)).toThrow( + /must be a non-negative integer/ + ); + }); + + it('throws error for float number', () => { + const config = { + 'changed-files-labels-limit': 3.2, + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + expect(() => getLabelConfigResultFromObject(config)).toThrow( + /must be a non-negative integer/ + ); + }); + + it('accepts zero as a valid changed-files-labels-limit', () => { + const config = { + 'changed-files-labels-limit': 0, + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.changedFilesLimit).toBe(0); + }); + + it('extracts max-files-changed as a number', () => { + const config = { + 'max-files-changed': 100, + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.maxFilesChanged).toBe(100); + expect(result.labelConfigs.has('label1')).toBe(true); + }); + + it('parses max-files-changed from string', () => { + const config = { + 'max-files-changed': '50', + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.maxFilesChanged).toBe(50); + }); + + it('returns undefined maxFilesChanged when not set', () => { + const config = { + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.maxFilesChanged).toBeUndefined(); + }); + + it('throws error for invalid max-files-changed value', () => { + const config = { + 'max-files-changed': 'invalid', + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + expect(() => getLabelConfigResultFromObject(config)).toThrow( + /Invalid value for 'max-files-changed'/ + ); + }); + + it('throws error for negative max-files-changed value', () => { + const config = { + 'max-files-changed': -1, + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + expect(() => getLabelConfigResultFromObject(config)).toThrow( + /must be a non-negative integer/ + ); + }); + + it('accepts zero as a valid max-files-changed', () => { + const config = { + 'max-files-changed': 0, + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.maxFilesChanged).toBe(0); + }); + + it('supports both options together', () => { + const config = { + 'changed-files-labels-limit': 5, + 'max-files-changed': 100, + label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}] + }; + const result = getLabelConfigResultFromObject(config); + expect(result.changedFilesLimit).toBe(5); + expect(result.maxFilesChanged).toBe(100); + }); + + it('throws a clear error when max-files-changed is used as a label', () => { + const config = { + 'max-files-changed': [ + {'changed-files': [{'any-glob-to-any-file': ['*.txt']}]} + ] + }; + expect(() => getLabelConfigResultFromObject(config)).toThrow( + /reserved top-level option and cannot be used as a label name/ + ); + }); + + it('throws a clear error when changed-files-labels-limit is used as a label', () => { + const config = { + 'changed-files-labels-limit': [ + {'changed-files': [{'any-glob-to-any-file': ['*.txt']}]} + ] + }; + expect(() => getLabelConfigResultFromObject(config)).toThrow( + /reserved top-level option and cannot be used as a label name/ + ); + }); }); describe('toMatchConfig', () => { @@ -164,6 +364,52 @@ describe('checkMatchConfigs', () => { }); }); +describe('configUsesChangedFiles', () => { + it('returns true when config has changed-files in any block', () => { + const matchConfig: MatchConfig[] = [ + {any: [{changedFiles: [{anyGlobToAnyFile: ['*.txt']}]}]} + ]; + expect(configUsesChangedFiles(matchConfig)).toBe(true); + }); + + it('returns true when config has changed-files in all block', () => { + const matchConfig: MatchConfig[] = [ + {all: [{changedFiles: [{allGlobsToAllFiles: ['*.txt']}]}]} + ]; + expect(configUsesChangedFiles(matchConfig)).toBe(true); + }); + + it('returns false when config only has branch patterns', () => { + const matchConfig: MatchConfig[] = [ + {any: [{headBranch: ['^test/']}]}, + {any: [{baseBranch: ['main']}]} + ]; + expect(configUsesChangedFiles(matchConfig)).toBe(false); + }); + + it('returns false when config has empty changed-files array', () => { + const matchConfig: MatchConfig[] = [{any: [{changedFiles: []}]}]; + expect(configUsesChangedFiles(matchConfig)).toBe(false); + }); + + it('returns false when config has changed-files with empty objects', () => { + const matchConfig: MatchConfig[] = [{any: [{changedFiles: [{}]}]}]; + expect(configUsesChangedFiles(matchConfig)).toBe(false); + }); + + it('returns true when config has mixed branch and changed-files patterns', () => { + const matchConfig: MatchConfig[] = [ + { + any: [ + {changedFiles: [{anyGlobToAnyFile: ['*.txt']}]}, + {headBranch: ['^feature/']} + ] + } + ]; + expect(configUsesChangedFiles(matchConfig)).toBe(true); + }); +}); + describe('labeler error handling', () => { const mockClient = {} as any; const mockPullRequest = { @@ -183,9 +429,10 @@ describe('labeler error handling', () => { } ]); - (api.getLabelConfigs as jest.Mock).mockResolvedValue( - new Map([['new-label', ['dummy-config']]]) - ); + (api.getLabelConfigs as jest.Mock).mockResolvedValue({ + labelConfigs: new Map([['new-label', ['dummy-config']]]), + changedFilesLimit: undefined + }); // Force match so "new-label" is always added jest.spyOn({checkMatchConfigs}, 'checkMatchConfigs').mockReturnValue(true); diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index 0490f795..434db55e 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -37,7 +37,17 @@ const yamlFixtures = { 'branches.yml': fs.readFileSync('__tests__/fixtures/branches.yml'), 'only_pdfs.yml': fs.readFileSync('__tests__/fixtures/only_pdfs.yml'), 'not_supported.yml': fs.readFileSync('__tests__/fixtures/not_supported.yml'), - 'any_and_all.yml': fs.readFileSync('__tests__/fixtures/any_and_all.yml') + 'any_and_all.yml': fs.readFileSync('__tests__/fixtures/any_and_all.yml'), + 'mixed_labels.yml': fs.readFileSync('__tests__/fixtures/mixed_labels.yml'), + 'limit_0.yml': fs.readFileSync('__tests__/fixtures/limit_0.yml'), + 'limit_1.yml': fs.readFileSync('__tests__/fixtures/limit_1.yml'), + 'limit_2.yml': fs.readFileSync('__tests__/fixtures/limit_2.yml'), + 'limit_3.yml': fs.readFileSync('__tests__/fixtures/limit_3.yml'), + 'mixed_rules.yml': fs.readFileSync('__tests__/fixtures/mixed_rules.yml'), + 'max_files_5.yml': fs.readFileSync('__tests__/fixtures/max_files_5.yml'), + 'max_files_with_branch.yml': fs.readFileSync( + '__tests__/fixtures/max_files_with_branch.yml' + ) }; const configureInput = ( @@ -440,6 +450,341 @@ describe('run', () => { expect(setLabelsMock).toHaveBeenCalledTimes(0); }); + describe('changed-files-labels-limit', () => { + it('applies all labels when count is within limit', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('limit_3.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file.ts', + 'components/b/file.ts' + ); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['component-a', 'component-b'] + }); + }); + + it('skips changed-files labels when count exceeds limit', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('limit_2.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file.ts', + 'components/b/file.ts', + 'components/c/file.ts' + ); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + // No labels should be applied since changed-files labels exceed limit + expect(setLabelsMock).toHaveBeenCalledTimes(0); + }); + + it('still applies branch-based labels when changed-files limit is exceeded', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'test/some-feature'}; + usingLabelerConfigYaml('limit_1.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file.ts', + 'components/b/file.ts', + 'components/c/file.ts' + ); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + // Only the branch-based label should be applied + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['test-branch'] + }); + }); + + it('applies all labels when no limit is set', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('mixed_labels.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file.ts', + 'components/b/file.ts', + 'components/c/file.ts', + 'components/d/file.ts' + ); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['component-a', 'component-b', 'component-c', 'component-d'] + }); + }); + + it('does not count preexisting labels toward the limit', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('limit_2.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file.ts', + 'components/b/file.ts', + 'components/c/file.ts', + 'components/d/file.ts' + ); + getPullMock.mockResolvedValue({ + data: {labels: [{name: 'component-a'}, {name: 'component-b'}]} + }); + + await run(); + + // component-a and component-b are preexisting, so only 2 new labels (c, d) would be added + // which equals the limit of 2, so labels should be applied + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['component-a', 'component-b', 'component-c', 'component-d'] + }); + }); + + it('skips new labels when new count exceeds limit even with preexisting', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('limit_2.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file.ts', + 'components/b/file.ts', + 'components/c/file.ts', + 'components/d/file.ts' + ); + getPullMock.mockResolvedValue({ + data: {labels: [{name: 'component-a'}]} + }); + + await run(); + + // component-a is preexisting, so 3 new labels (b, c, d) would be added + // which exceeds the limit of 2, so no new changed-files labels are applied + expect(setLabelsMock).toHaveBeenCalledTimes(0); + }); + + it('applies labels when new count equals the limit', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('limit_2.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file.ts', + 'components/b/file.ts' + ); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['component-a', 'component-b'] + }); + }); + + it('skips all changed-files labels when limit is 0', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'test/some-feature'}; + usingLabelerConfigYaml('limit_0.yml'); + mockGitHubResponseChangedFiles('components/a/file.ts'); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + // With limit 0, only branch-based labels should be applied + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['test-branch'] + }); + }); + + it('treats labels with mixed rules as changed-files labels', async () => { + // A label that has both branch and changed-files rules is considered + // a "changed-files label" and subject to the limit, even if it matches + // via the branch rule + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'test/some-feature'}; + usingLabelerConfigYaml('mixed_rules.yml'); + mockGitHubResponseChangedFiles('unrelated/file.ts'); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + // The mixed-label matches via branch rule but is still subject to limit + // because it contains a changed-files rule in its definition. + // Only pure-branch-label should be applied. + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['pure-branch-label'] + }); + }); + }); + + describe('max-files-changed', () => { + it('applies labels when changed files count is within limit', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('max_files_5.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file.ts', + 'components/b/file.ts', + 'components/c/file.ts' + ); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['component-a', 'component-b', 'component-c'] + }); + }); + + it('skips file-based labels when changed files exceed limit', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('max_files_5.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file1.ts', + 'components/a/file2.ts', + 'components/b/file1.ts', + 'components/b/file2.ts', + 'components/c/file1.ts', + 'components/c/file2.ts' // 6 files > limit of 5 + ); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + // No labels should be applied since changed files exceed limit + expect(setLabelsMock).toHaveBeenCalledTimes(0); + }); + + it('applies labels when changed files count equals limit', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('max_files_5.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file1.ts', + 'components/a/file2.ts', + 'components/b/file1.ts', + 'components/b/file2.ts', + 'components/c/file.ts' // exactly 5 files = limit + ); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['component-a', 'component-b', 'component-c'] + }); + }); + + it('still applies branch-based labels when max-files-changed is exceeded', async () => { + configureInput({}); + github.context.payload.pull_request!.head = {ref: 'test/some-feature'}; + usingLabelerConfigYaml('max_files_with_branch.yml'); + mockGitHubResponseChangedFiles( + 'components/a/file1.ts', + 'components/a/file2.ts', + 'components/b/file1.ts', + 'components/b/file2.ts' // 4 files > limit of 3 + ); + getPullMock.mockResolvedValue({ + data: {labels: []} + }); + + await run(); + + // Only the branch-based label should be applied + expect(setLabelsMock).toHaveBeenCalledTimes(1); + expect(setLabelsMock).toHaveBeenCalledWith({ + owner: 'monalisa', + repo: 'helloworld', + issue_number: 123, + labels: ['test-branch'] + }); + }); + + it('preserves preexisting changed-files labels with sync-labels when max-files-changed is exceeded', async () => { + configureInput({'sync-labels': true}); + github.context.payload.pull_request!.head = {ref: 'main'}; + usingLabelerConfigYaml('max_files_5.yml'); + mockGitHubResponseChangedFiles( + 'unrelated/file1.ts', + 'unrelated/file2.ts', + 'unrelated/file3.ts', + 'unrelated/file4.ts', + 'unrelated/file5.ts', + 'unrelated/file6.ts' // 6 files > limit of 5 + ); + getPullMock.mockResolvedValue({ + data: {labels: [{name: 'component-a'}]} // preexisting label + }); + + await run(); + + // No setLabels call because labels should remain unchanged + // (component-a is preserved, not removed by sync-labels) + expect(setLabelsMock).toHaveBeenCalledTimes(0); + }); + }); + it('should use local configuration file if it exists', async () => { const configFile = 'only_pdfs.yml'; const configFilePath = path.join(__dirname, 'fixtures', configFile); diff --git a/dist/index.js b/dist/index.js index 07550f22..af2aa0f4 100644 --- a/dist/index.js +++ b/dist/index.js @@ -269,8 +269,10 @@ var __importDefault = (this && this.__importDefault) || function (mod) { }; Object.defineProperty(exports, "__esModule", ({ value: true })); exports.getLabelConfigs = void 0; +exports.getLabelConfigResultFromObject = getLabelConfigResultFromObject; exports.getLabelConfigMapFromObject = getLabelConfigMapFromObject; exports.toMatchConfig = toMatchConfig; +exports.configUsesChangedFiles = configUsesChangedFiles; const core = __importStar(__nccwpck_require__(7484)); const yaml = __importStar(__nccwpck_require__(4281)); const fs_1 = __importDefault(__nccwpck_require__(9896)); @@ -278,6 +280,29 @@ const get_content_1 = __nccwpck_require__(6519); const changedFiles_1 = __nccwpck_require__(5145); const branch_1 = __nccwpck_require__(2234); const ALLOWED_CONFIG_KEYS = ['changed-files', 'head-branch', 'base-branch']; +const TOP_LEVEL_OPTIONS = ['changed-files-labels-limit', 'max-files-changed']; +/** + * Parses and validates a non-negative integer value from the configuration. + */ +function parseNonNegativeInteger(value, optionName) { + if (typeof value === 'number') { + if (!Number.isFinite(value) || !Number.isInteger(value) || value < 0) { + throw new Error(`Invalid value for '${optionName}': must be a non-negative integer (got ${value})`); + } + return value; + } + if (typeof value === 'string') { + const trimmed = value.trim(); + if (!/^\d+$/.test(trimmed)) { + throw new Error(`Invalid value for '${optionName}': must be a non-negative integer (got '${value}')`); + } + return Number(trimmed); + } + if (Array.isArray(value)) { + throw new Error(`'${optionName}' is a reserved top-level option and cannot be used as a label name. Please rename it.`); + } + throw new Error(`Invalid value for '${optionName}': expected a non-negative integer`); +} const getLabelConfigs = (client, configurationPath) => Promise.resolve() .then(() => { if (!fs_1.default.existsSync(configurationPath)) { @@ -298,13 +323,35 @@ const getLabelConfigs = (client, configurationPath) => Promise.resolve() .then(configuration => { // loads (hopefully) a `{[label:string]: MatchConfig[]}`, but is `any`: const configObject = yaml.load(configuration); - // transform `any` => `Map` or throw if yaml is malformed: - return getLabelConfigMapFromObject(configObject); + // transform `any` => `LabelConfigResult` or throw if yaml is malformed: + return getLabelConfigResultFromObject(configObject); }); exports.getLabelConfigs = getLabelConfigs; +function getLabelConfigResultFromObject(configObject) { + // Extract top-level options + let changedFilesLimit; + let maxFilesChanged; + const limitValue = configObject === null || configObject === void 0 ? void 0 : configObject['changed-files-labels-limit']; + if (limitValue !== undefined) { + changedFilesLimit = parseNonNegativeInteger(limitValue, 'changed-files-labels-limit'); + } + const maxFilesValue = configObject === null || configObject === void 0 ? void 0 : configObject['max-files-changed']; + if (maxFilesValue !== undefined) { + maxFilesChanged = parseNonNegativeInteger(maxFilesValue, 'max-files-changed'); + } + return { + labelConfigs: getLabelConfigMapFromObject(configObject), + changedFilesLimit, + maxFilesChanged + }; +} function getLabelConfigMapFromObject(configObject) { const labelMap = new Map(); for (const label in configObject) { + // Skip top-level options + if (TOP_LEVEL_OPTIONS.includes(label)) { + continue; + } const configOptions = configObject[label]; if (!Array.isArray(configOptions) || !configOptions.every(opts => typeof opts === 'object')) { @@ -354,6 +401,31 @@ function toMatchConfig(config) { const branchConfig = (0, branch_1.toBranchMatchConfig)(config); return Object.assign(Object.assign({}, changedFilesConfig), branchConfig); } +/** + * Checks if any of the match configs for a label use changed-files patterns. + * This is used to determine if a label should be counted toward the changed-files limit. + */ +function configUsesChangedFiles(matchConfigs) { + for (const config of matchConfigs) { + if (config.all) { + for (const baseConfig of config.all) { + if (baseConfig.changedFiles && + baseConfig.changedFiles.some(cf => Object.keys(cf).length > 0)) { + return true; + } + } + } + if (config.any) { + for (const baseConfig of config.any) { + if (baseConfig.changedFiles && + baseConfig.changedFiles.some(cf => Object.keys(cf).length > 0)) { + return true; + } + } + } + } + return false; +} /***/ }), @@ -1038,6 +1110,7 @@ const pluginRetry = __importStar(__nccwpck_require__(3450)); const api = __importStar(__nccwpck_require__(6063)); const lodash_isequal_1 = __importDefault(__nccwpck_require__(9471)); const get_inputs_1 = __nccwpck_require__(1219); +const get_label_configs_1 = __nccwpck_require__(8554); const changedFiles_1 = __nccwpck_require__(5145); const branch_1 = __nccwpck_require__(2234); // GitHub Issues cannot have more than 100 labels @@ -1062,18 +1135,47 @@ function labeler() { _c = pullRequests_1_1.value; _d = false; const pullRequest = _c; - const labelConfigs = yield api.getLabelConfigs(client, configPath); + const { labelConfigs, changedFilesLimit, maxFilesChanged } = yield api.getLabelConfigs(client, configPath); + // Check if total changed files exceeds the max-files-changed threshold + const skipChangedFilesLabeling = maxFilesChanged !== undefined && + pullRequest.changedFiles.length > maxFilesChanged; + if (skipChangedFilesLabeling) { + core.info(`Total changed files (${pullRequest.changedFiles.length}) exceeds max-files-changed (${maxFilesChanged}), skipping file-based labeling`); + } const preexistingLabels = pullRequest.data.labels.map(l => l.name); const allLabels = new Set(preexistingLabels); + // Track labels that would be added based on changed-files patterns + const changedFilesLabels = new Set(); for (const [label, configs] of labelConfigs.entries()) { core.debug(`processing ${label}`); + // If this config uses changed-files and we're skipping file-based labeling, + // don't evaluate it at all (skip add/remove) to preserve preexisting labels + const usesChangedFiles = (0, get_label_configs_1.configUsesChangedFiles)(configs); + if (skipChangedFilesLabeling && usesChangedFiles) { + core.debug(`skipping ${label} (uses changed-files and max-files-changed exceeded)`); + continue; + } if (checkMatchConfigs(pullRequest.changedFiles, configs, dot)) { allLabels.add(label); + // Track if this label uses changed-files patterns + if (usesChangedFiles) { + changedFilesLabels.add(label); + } } else if (syncLabels) { allLabels.delete(label); } } + // Check if changed-files labels should be skipped due to labels limit + const newChangedFilesLabels = [...changedFilesLabels].filter(l => !preexistingLabels.includes(l)); + if (changedFilesLimit !== undefined && + newChangedFilesLabels.length > changedFilesLimit) { + core.info(`Changed-files labels (${newChangedFilesLabels.length}) exceed limit (${changedFilesLimit}), skipping: ${newChangedFilesLabels.join(', ')}`); + // Remove all new changed-files labels + for (const label of newChangedFilesLabels) { + allLabels.delete(label); + } + } const labelsToApply = [...allLabels].slice(0, GITHUB_MAX_LABELS); const excessLabels = [...allLabels].slice(GITHUB_MAX_LABELS); let finalLabels = labelsToApply; diff --git a/src/api/get-label-configs.ts b/src/api/get-label-configs.ts index 4db33f28..891e0229 100644 --- a/src/api/get-label-configs.ts +++ b/src/api/get-label-configs.ts @@ -18,12 +18,53 @@ export interface MatchConfig { export type BaseMatchConfig = BranchMatchConfig & ChangedFilesMatchConfig; +export interface LabelConfigResult { + labelConfigs: Map; + changedFilesLimit?: number; + maxFilesChanged?: number; +} + const ALLOWED_CONFIG_KEYS = ['changed-files', 'head-branch', 'base-branch']; +const TOP_LEVEL_OPTIONS = ['changed-files-labels-limit', 'max-files-changed']; + +/** + * Parses and validates a non-negative integer value from the configuration. + */ +function parseNonNegativeInteger(value: unknown, optionName: string): number { + if (typeof value === 'number') { + if (!Number.isFinite(value) || !Number.isInteger(value) || value < 0) { + throw new Error( + `Invalid value for '${optionName}': must be a non-negative integer (got ${value})` + ); + } + return value; + } + + if (typeof value === 'string') { + const trimmed = value.trim(); + if (!/^\d+$/.test(trimmed)) { + throw new Error( + `Invalid value for '${optionName}': must be a non-negative integer (got '${value}')` + ); + } + return Number(trimmed); + } + + if (Array.isArray(value)) { + throw new Error( + `'${optionName}' is a reserved top-level option and cannot be used as a label name. Please rename it.` + ); + } + + throw new Error( + `Invalid value for '${optionName}': expected a non-negative integer` + ); +} export const getLabelConfigs = ( client: ClientType, configurationPath: string -): Promise> => +): Promise => Promise.resolve() .then(() => { if (!fs.existsSync(configurationPath)) { @@ -54,15 +95,49 @@ export const getLabelConfigs = ( // loads (hopefully) a `{[label:string]: MatchConfig[]}`, but is `any`: const configObject: any = yaml.load(configuration); - // transform `any` => `Map` or throw if yaml is malformed: - return getLabelConfigMapFromObject(configObject); + // transform `any` => `LabelConfigResult` or throw if yaml is malformed: + return getLabelConfigResultFromObject(configObject); }); +export function getLabelConfigResultFromObject( + configObject: any +): LabelConfigResult { + // Extract top-level options + let changedFilesLimit: number | undefined; + let maxFilesChanged: number | undefined; + + const limitValue = configObject?.['changed-files-labels-limit']; + if (limitValue !== undefined) { + changedFilesLimit = parseNonNegativeInteger( + limitValue, + 'changed-files-labels-limit' + ); + } + + const maxFilesValue = configObject?.['max-files-changed']; + if (maxFilesValue !== undefined) { + maxFilesChanged = parseNonNegativeInteger( + maxFilesValue, + 'max-files-changed' + ); + } + + return { + labelConfigs: getLabelConfigMapFromObject(configObject), + changedFilesLimit, + maxFilesChanged + }; +} + export function getLabelConfigMapFromObject( configObject: any ): Map { const labelMap: Map = new Map(); for (const label in configObject) { + // Skip top-level options + if (TOP_LEVEL_OPTIONS.includes(label)) { + continue; + } const configOptions = configObject[label]; if ( !Array.isArray(configOptions) || @@ -124,3 +199,33 @@ export function toMatchConfig(config: any): BaseMatchConfig { ...branchConfig }; } + +/** + * Checks if any of the match configs for a label use changed-files patterns. + * This is used to determine if a label should be counted toward the changed-files limit. + */ +export function configUsesChangedFiles(matchConfigs: MatchConfig[]): boolean { + for (const config of matchConfigs) { + if (config.all) { + for (const baseConfig of config.all) { + if ( + baseConfig.changedFiles && + baseConfig.changedFiles.some(cf => Object.keys(cf).length > 0) + ) { + return true; + } + } + } + if (config.any) { + for (const baseConfig of config.any) { + if ( + baseConfig.changedFiles && + baseConfig.changedFiles.some(cf => Object.keys(cf).length > 0) + ) { + return true; + } + } + } + } + return false; +} diff --git a/src/labeler.ts b/src/labeler.ts index dfe438f2..b969275b 100644 --- a/src/labeler.ts +++ b/src/labeler.ts @@ -5,7 +5,11 @@ import * as api from './api'; import isEqual from 'lodash.isequal'; import {getInputs} from './get-inputs'; -import {BaseMatchConfig, MatchConfig} from './api/get-label-configs'; +import { + BaseMatchConfig, + MatchConfig, + configUsesChangedFiles +} from './api/get-label-configs'; import {checkAllChangedFiles, checkAnyChangedFiles} from './changedFiles'; @@ -35,22 +39,68 @@ export async function labeler() { const pullRequests = api.getPullRequests(client, prNumbers); for await (const pullRequest of pullRequests) { - const labelConfigs: Map = await api.getLabelConfigs( - client, - configPath - ); + const {labelConfigs, changedFilesLimit, maxFilesChanged} = + await api.getLabelConfigs(client, configPath); + + // Check if total changed files exceeds the max-files-changed threshold + const skipChangedFilesLabeling = + maxFilesChanged !== undefined && + pullRequest.changedFiles.length > maxFilesChanged; + + if (skipChangedFilesLabeling) { + core.info( + `Total changed files (${pullRequest.changedFiles.length}) exceeds max-files-changed (${maxFilesChanged}), skipping file-based labeling` + ); + } + const preexistingLabels = pullRequest.data.labels.map(l => l.name); const allLabels: Set = new Set(preexistingLabels); + // Track labels that would be added based on changed-files patterns + const changedFilesLabels: Set = new Set(); + for (const [label, configs] of labelConfigs.entries()) { core.debug(`processing ${label}`); + + // If this config uses changed-files and we're skipping file-based labeling, + // don't evaluate it at all (skip add/remove) to preserve preexisting labels + const usesChangedFiles = configUsesChangedFiles(configs); + if (skipChangedFilesLabeling && usesChangedFiles) { + core.debug( + `skipping ${label} (uses changed-files and max-files-changed exceeded)` + ); + continue; + } + if (checkMatchConfigs(pullRequest.changedFiles, configs, dot)) { allLabels.add(label); + // Track if this label uses changed-files patterns + if (usesChangedFiles) { + changedFilesLabels.add(label); + } } else if (syncLabels) { allLabels.delete(label); } } + // Check if changed-files labels should be skipped due to labels limit + const newChangedFilesLabels = [...changedFilesLabels].filter( + l => !preexistingLabels.includes(l) + ); + + if ( + changedFilesLimit !== undefined && + newChangedFilesLabels.length > changedFilesLimit + ) { + core.info( + `Changed-files labels (${newChangedFilesLabels.length}) exceed limit (${changedFilesLimit}), skipping: ${newChangedFilesLabels.join(', ')}` + ); + // Remove all new changed-files labels + for (const label of newChangedFilesLabels) { + allLabels.delete(label); + } + } + const labelsToApply = [...allLabels].slice(0, GITHUB_MAX_LABELS); const excessLabels = [...allLabels].slice(GITHUB_MAX_LABELS);