From a8c0d7bc62412fe14764b1b03057f7cc634fda69 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Oct 2025 14:42:58 +0000 Subject: [PATCH 1/6] Initial plan From 3c49816df101551fbcf11633190a91c2d2d96bd0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Oct 2025 14:49:24 +0000 Subject: [PATCH 2/6] Initial plan: Prettify permissions error messages Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/go-logger.lock.yml | 2 +- .github/workflows/technical-doc-writer.lock.yml | 2 +- .github/workflows/tidy.lock.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go-logger.lock.yml b/.github/workflows/go-logger.lock.yml index 86cb698ffbe..af22ed6fcba 100644 --- a/.github/workflows/go-logger.lock.yml +++ b/.github/workflows/go-logger.lock.yml @@ -90,7 +90,7 @@ jobs: with: persist-credentials: false - name: Set up Node.js - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 + uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 with: cache: npm cache-dependency-path: pkg/workflow/js/package-lock.json diff --git a/.github/workflows/technical-doc-writer.lock.yml b/.github/workflows/technical-doc-writer.lock.yml index 24a193cc30f..0e2f89b7799 100644 --- a/.github/workflows/technical-doc-writer.lock.yml +++ b/.github/workflows/technical-doc-writer.lock.yml @@ -493,7 +493,7 @@ jobs: with: persist-credentials: false - name: Setup Node.js - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 + uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 with: cache: npm cache-dependency-path: docs/package-lock.json diff --git a/.github/workflows/tidy.lock.yml b/.github/workflows/tidy.lock.yml index 964804b1d3f..1db02fce2cb 100644 --- a/.github/workflows/tidy.lock.yml +++ b/.github/workflows/tidy.lock.yml @@ -454,7 +454,7 @@ jobs: with: persist-credentials: false - name: Set up Node.js - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 + uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 with: cache: npm cache-dependency-path: pkg/workflow/js/package-lock.json From 056734df95d3239448c21c8d36c78c2dae38633f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Oct 2025 15:17:06 +0000 Subject: [PATCH 3/6] Prettify permissions error messages - Remove ERROR/WARNING prefixes (already rendered by compiler) - Change "GitHub MCP" to "github" - Make messages more compact by merging toolset details inline - Update "Required by toolsets:" to show inline as "(required by X)" - Simplify excess permissions message - Update test expectations to match new format - Fix test using allowed: instead of toolsets: Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_test.go | 2 +- .../permissions_excess_compilation_test.go | 6 +- pkg/workflow/permissions_validator.go | 58 +++++++++---------- pkg/workflow/permissions_validator_test.go | 42 ++++++++------ 4 files changed, 57 insertions(+), 51 deletions(-) diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index 801d517b7db..f567bbd010b 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -4203,7 +4203,7 @@ permissions: issues: write tools: github: - allowed: [list_issues, create_issue] + toolsets: [repos, issues] engine: claude --- diff --git a/pkg/workflow/permissions_excess_compilation_test.go b/pkg/workflow/permissions_excess_compilation_test.go index 84ae9c572ba..73c34e892ed 100644 --- a/pkg/workflow/permissions_excess_compilation_test.go +++ b/pkg/workflow/permissions_excess_compilation_test.go @@ -38,7 +38,7 @@ network: strictMode: true, expectCompileSuccess: true, expectWarning: true, - expectedWarningMessage: "WARNING: Over-provisioned permissions", + expectedWarningMessage: "Over-provisioned permissions detected for github toolsets:", }, { name: "Excess permissions in regular mode - should ignore silently", @@ -188,14 +188,14 @@ tools: // Check for warnings if tt.expectWarning { - if !strings.Contains(output, "WARNING") { + if !strings.Contains(output, "warning:") { t.Errorf("Expected warning in output but none found. Output: %s", output) } if tt.expectedWarningMessage != "" && !strings.Contains(output, tt.expectedWarningMessage) { t.Errorf("Expected warning message containing '%s' but got: %s", tt.expectedWarningMessage, output) } } else { - if strings.Contains(output, "WARNING") { + if strings.Contains(output, "warning:") { t.Errorf("Did not expect warning but found one. Output: %s", output) } } diff --git a/pkg/workflow/permissions_validator.go b/pkg/workflow/permissions_validator.go index d271c9732e2..538717bd173 100644 --- a/pkg/workflow/permissions_validator.go +++ b/pkg/workflow/permissions_validator.go @@ -292,37 +292,39 @@ func formatMissingPermissionsMessage(result *PermissionsValidationResult) string sort.Strings(scopes) var lines []string - lines = append(lines, "ERROR: Missing required permissions for GitHub MCP toolsets:") - + + // Build permission list with toolset details inline + var permLines []string for _, scopeStr := range scopes { scope := PermissionScope(scopeStr) level := result.MissingPermissions[scope] - lines = append(lines, fmt.Sprintf(" - %s: %s", scope, level)) - } - - // Add toolset details - if len(result.MissingToolsetDetails) > 0 { - lines = append(lines, "") - lines = append(lines, "Required by toolsets:") - - var toolsetNames []string - for toolset := range result.MissingToolsetDetails { - toolsetNames = append(toolsetNames, toolset) - } - sort.Strings(toolsetNames) - - for _, toolset := range toolsetNames { - scopes := result.MissingToolsetDetails[toolset] - scopeStrs := make([]string, len(scopes)) - for i, s := range scopes { - scopeStrs[i] = string(s) + + // Find which toolsets need this permission + var requiredBy []string + if len(result.MissingToolsetDetails) > 0 { + for toolset, toolsetScopes := range result.MissingToolsetDetails { + for _, ts := range toolsetScopes { + if ts == scope { + requiredBy = append(requiredBy, toolset) + break + } + } } - lines = append(lines, fmt.Sprintf(" - %s: needs %s", toolset, strings.Join(scopeStrs, ", "))) + } + + // Format: "- scope: level (required by toolset1, toolset2)" + if len(requiredBy) > 0 { + sort.Strings(requiredBy) + permLines = append(permLines, fmt.Sprintf(" - %s: %s (required by %s)", scope, level, strings.Join(requiredBy, ", "))) + } else { + permLines = append(permLines, fmt.Sprintf(" - %s: %s", scope, level)) } } + lines = append(lines, "Missing required permissions for github toolsets:") + lines = append(lines, permLines...) lines = append(lines, "") - lines = append(lines, "Suggested fix: Add the following to your workflow frontmatter:") + lines = append(lines, "Add to your workflow frontmatter:") lines = append(lines, "permissions:") for _, scopeStr := range scopes { scope := PermissionScope(scopeStr) @@ -342,20 +344,16 @@ func formatExcessPermissionsMessage(result *PermissionsValidationResult, strict sort.Strings(scopes) var lines []string - // Always use WARNING prefix (strict mode warns, regular mode is silent) - prefix := "WARNING" - - lines = append(lines, fmt.Sprintf("%s: Over-provisioned permissions detected for GitHub MCP toolsets:", prefix)) + lines = append(lines, "Over-provisioned permissions detected for github toolsets:") for _, scopeStr := range scopes { scope := PermissionScope(scopeStr) level := result.ExcessPermissions[scope] - lines = append(lines, fmt.Sprintf(" - %s: %s (not required by configured toolsets)", scope, level)) + lines = append(lines, fmt.Sprintf(" - %s: %s (not required)", scope, level)) } lines = append(lines, "") - lines = append(lines, "Principle of least privilege: Only grant permissions that are needed.") - lines = append(lines, "Consider removing these permissions or adjusting your toolsets configuration.") + lines = append(lines, "Only grant permissions that are needed.") return strings.Join(lines, "\n") } diff --git a/pkg/workflow/permissions_validator_test.go b/pkg/workflow/permissions_validator_test.go index eb37d09cdf3..8613a5257d9 100644 --- a/pkg/workflow/permissions_validator_test.go +++ b/pkg/workflow/permissions_validator_test.go @@ -404,13 +404,13 @@ func TestFormatValidationMessage(t *testing.T) { }, strict: false, expectContains: []string{ - "ERROR: Missing required permissions", - "contents: write", - "issues: write", - "Required by toolsets:", - "repos: needs contents", - "issues: needs issues", - "Suggested fix:", + "Missing required permissions for github toolsets:", + "contents: write (required by repos)", + "issues: write (required by issues)", + "Add to your workflow frontmatter:", + }, + expectNotContains: []string{ + "ERROR:", }, }, { @@ -424,10 +424,13 @@ func TestFormatValidationMessage(t *testing.T) { }, strict: false, expectContains: []string{ - "WARNING: Over-provisioned permissions", - "actions: read (not required", - "discussions: write (not required", - "Principle of least privilege", + "Over-provisioned permissions detected for github toolsets:", + "actions: read (not required)", + "discussions: write (not required)", + "Only grant permissions that are needed.", + }, + expectNotContains: []string{ + "WARNING:", }, }, { @@ -440,11 +443,12 @@ func TestFormatValidationMessage(t *testing.T) { }, strict: true, expectContains: []string{ - "WARNING: Over-provisioned permissions", + "Over-provisioned permissions detected for github toolsets:", "actions: read", }, expectNotContains: []string{ "ERROR:", + "WARNING:", }, }, { @@ -460,11 +464,15 @@ func TestFormatValidationMessage(t *testing.T) { }, strict: false, expectContains: []string{ - "ERROR: Missing required permissions", + "Missing required permissions for github toolsets:", "contents: write", - "WARNING: Over-provisioned permissions", + "Over-provisioned permissions detected for github toolsets:", "actions: read", }, + expectNotContains: []string{ + "ERROR:", + "WARNING:", + }, }, } @@ -533,7 +541,7 @@ func TestValidatePermissions_ComplexScenarios(t *testing.T) { "read-only": false, }, expectMsg: []string{ - "ERROR: Missing required permissions", + "Missing required permissions for github toolsets:", "contents: write", "issues: write", "pull-requests: write", @@ -547,7 +555,7 @@ func TestValidatePermissions_ComplexScenarios(t *testing.T) { "read-only": false, }, expectMsg: []string{ - "WARNING: Over-provisioned permissions", + "Over-provisioned permissions detected for github toolsets:", }, }, { @@ -558,7 +566,7 @@ func TestValidatePermissions_ComplexScenarios(t *testing.T) { "read-only": false, }, expectMsg: []string{ - "ERROR: Missing required permissions", + "Missing required permissions for github toolsets:", "discussions: write", }, }, From f93dce04aef075593e7e17e32fd0bb39fde5258a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Oct 2025 15:23:06 +0000 Subject: [PATCH 4/6] Run go fmt --- pkg/workflow/permissions_validator.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/workflow/permissions_validator.go b/pkg/workflow/permissions_validator.go index 538717bd173..217b3ccd819 100644 --- a/pkg/workflow/permissions_validator.go +++ b/pkg/workflow/permissions_validator.go @@ -292,13 +292,13 @@ func formatMissingPermissionsMessage(result *PermissionsValidationResult) string sort.Strings(scopes) var lines []string - + // Build permission list with toolset details inline var permLines []string for _, scopeStr := range scopes { scope := PermissionScope(scopeStr) level := result.MissingPermissions[scope] - + // Find which toolsets need this permission var requiredBy []string if len(result.MissingToolsetDetails) > 0 { @@ -311,7 +311,7 @@ func formatMissingPermissionsMessage(result *PermissionsValidationResult) string } } } - + // Format: "- scope: level (required by toolset1, toolset2)" if len(requiredBy) > 0 { sort.Strings(requiredBy) From 850f9feeba552a1cb73e7af3c254795bdc782a33 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Oct 2025 15:24:20 +0000 Subject: [PATCH 5/6] Run make fmt-json --- pkg/workflow/schemas/github-workflow.json | 76 +++-------------------- 1 file changed, 9 insertions(+), 67 deletions(-) diff --git a/pkg/workflow/schemas/github-workflow.json b/pkg/workflow/schemas/github-workflow.json index 160824c1de3..6b93ceff0b8 100644 --- a/pkg/workflow/schemas/github-workflow.json +++ b/pkg/workflow/schemas/github-workflow.json @@ -983,19 +983,9 @@ "$ref": "#/definitions/types", "items": { "type": "string", - "enum": [ - "created", - "rerequested", - "completed", - "requested_action" - ] + "enum": ["created", "rerequested", "completed", "requested_action"] }, - "default": [ - "created", - "rerequested", - "completed", - "requested_action" - ] + "default": ["created", "rerequested", "completed", "requested_action"] } } }, @@ -1207,13 +1197,7 @@ "type": "string", "enum": ["created", "closed", "opened", "edited", "deleted"] }, - "default": [ - "created", - "closed", - "opened", - "edited", - "deleted" - ] + "default": ["created", "closed", "opened", "edited", "deleted"] } } }, @@ -1231,23 +1215,9 @@ "$ref": "#/definitions/types", "items": { "type": "string", - "enum": [ - "created", - "updated", - "closed", - "reopened", - "edited", - "deleted" - ] + "enum": ["created", "updated", "closed", "reopened", "edited", "deleted"] }, - "default": [ - "created", - "updated", - "closed", - "reopened", - "edited", - "deleted" - ] + "default": ["created", "updated", "closed", "reopened", "edited", "deleted"] } } }, @@ -1260,21 +1230,9 @@ "$ref": "#/definitions/types", "items": { "type": "string", - "enum": [ - "created", - "moved", - "converted", - "edited", - "deleted" - ] + "enum": ["created", "moved", "converted", "edited", "deleted"] }, - "default": [ - "created", - "moved", - "converted", - "edited", - "deleted" - ] + "default": ["created", "moved", "converted", "edited", "deleted"] } } }, @@ -1558,25 +1516,9 @@ "$ref": "#/definitions/types", "items": { "type": "string", - "enum": [ - "published", - "unpublished", - "created", - "edited", - "deleted", - "prereleased", - "released" - ] + "enum": ["published", "unpublished", "created", "edited", "deleted", "prereleased", "released"] }, - "default": [ - "published", - "unpublished", - "created", - "edited", - "deleted", - "prereleased", - "released" - ] + "default": ["published", "unpublished", "created", "edited", "deleted", "prereleased", "released"] } } }, From d551d920ab85a4810f41a9663624b5a5f893de17 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 30 Oct 2025 15:31:58 +0000 Subject: [PATCH 6/6] Add changeset for prettified permissions validation error [skip-ci] messages --- .changeset/patch-prettify-permissions-validation-errors.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/patch-prettify-permissions-validation-errors.md diff --git a/.changeset/patch-prettify-permissions-validation-errors.md b/.changeset/patch-prettify-permissions-validation-errors.md new file mode 100644 index 00000000000..d9d6db32f3d --- /dev/null +++ b/.changeset/patch-prettify-permissions-validation-errors.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Prettify permissions validation error messages