From 2c998defc0104e8e062599c603237100b9dd44a1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 02:36:22 +0000 Subject: [PATCH 1/7] Initial plan From 626c38a415cd575fd604bde43bd8eadcfa72d0d0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 03:14:24 +0000 Subject: [PATCH 2/7] Add top-level github-app frontmatter support as fallback for nested token minting Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../test-top-level-github-app-activation.md | 25 ++ .../test-top-level-github-app-checkout.md | 27 ++ .../test-top-level-github-app-dependencies.md | 26 ++ .../test-top-level-github-app-mcp.md | 28 ++ .../test-top-level-github-app-override.md | 33 ++ .../test-top-level-github-app-safe-outputs.md | 25 ++ pkg/parser/schemas/main_workflow_schema.json | 4 + .../compiler_orchestrator_workflow.go | 86 ++++ pkg/workflow/compiler_types.go | 1 + .../top_level_github_app_integration_test.go | 396 ++++++++++++++++++ 10 files changed, 651 insertions(+) create mode 100644 pkg/cli/workflows/test-top-level-github-app-activation.md create mode 100644 pkg/cli/workflows/test-top-level-github-app-checkout.md create mode 100644 pkg/cli/workflows/test-top-level-github-app-dependencies.md create mode 100644 pkg/cli/workflows/test-top-level-github-app-mcp.md create mode 100644 pkg/cli/workflows/test-top-level-github-app-override.md create mode 100644 pkg/cli/workflows/test-top-level-github-app-safe-outputs.md create mode 100644 pkg/workflow/top_level_github_app_integration_test.go diff --git a/pkg/cli/workflows/test-top-level-github-app-activation.md b/pkg/cli/workflows/test-top-level-github-app-activation.md new file mode 100644 index 00000000000..00dce1efadc --- /dev/null +++ b/pkg/cli/workflows/test-top-level-github-app-activation.md @@ -0,0 +1,25 @@ +--- +on: + issues: + types: [opened] + reaction: eyes +permissions: + contents: read + +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +# Top-Level GitHub App Fallback for Activation + +This workflow demonstrates using a top-level github-app as a fallback for activation operations. + +The top-level `github-app` is automatically applied to the activation job (reactions, status +comments, skip-if checks) when no `on.github-app` is defined. + +When an issue is opened, react with 👀 and create a follow-up issue using the GitHub App token. diff --git a/pkg/cli/workflows/test-top-level-github-app-checkout.md b/pkg/cli/workflows/test-top-level-github-app-checkout.md new file mode 100644 index 00000000000..f8596805893 --- /dev/null +++ b/pkg/cli/workflows/test-top-level-github-app-checkout.md @@ -0,0 +1,27 @@ +--- +on: + issues: + types: [opened] +permissions: + contents: read + +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +checkout: + repository: myorg/private-repo + path: private +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +# Top-Level GitHub App Fallback for Checkout + +This workflow demonstrates using a top-level github-app as a fallback for checkout operations. + +The top-level `github-app` is automatically applied to checkout operations that do not have +their own `github-app` or `github-token` configured. + +This is useful for checking out private repositories using the GitHub App installation token. diff --git a/pkg/cli/workflows/test-top-level-github-app-dependencies.md b/pkg/cli/workflows/test-top-level-github-app-dependencies.md new file mode 100644 index 00000000000..098ecbfc91d --- /dev/null +++ b/pkg/cli/workflows/test-top-level-github-app-dependencies.md @@ -0,0 +1,26 @@ +--- +on: + issues: + types: [opened] +permissions: + contents: read + +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +dependencies: + packages: + - myorg/private-skill +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +# Top-Level GitHub App Fallback for APM Dependencies + +This workflow demonstrates using a top-level github-app as a fallback for APM dependencies. + +The top-level `github-app` is automatically applied to APM package installations when no +`dependencies.github-app` is configured. This allows installing APM packages from private +repositories across organizations using the GitHub App installation token. diff --git a/pkg/cli/workflows/test-top-level-github-app-mcp.md b/pkg/cli/workflows/test-top-level-github-app-mcp.md new file mode 100644 index 00000000000..7a46b57b669 --- /dev/null +++ b/pkg/cli/workflows/test-top-level-github-app-mcp.md @@ -0,0 +1,28 @@ +--- +on: + issues: + types: [opened] +permissions: + contents: read + +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +tools: + github: + mode: remote + toolsets: [default] +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +# Top-Level GitHub App Fallback for GitHub MCP Tool + +This workflow demonstrates using a top-level github-app as a fallback for tools.github +token minting operations. + +The top-level `github-app` is automatically applied to the GitHub MCP tool configuration +when no `tools.github.github-app` is defined. This mints a GitHub App installation access +token for the GitHub MCP server to use when making API calls. diff --git a/pkg/cli/workflows/test-top-level-github-app-override.md b/pkg/cli/workflows/test-top-level-github-app-override.md new file mode 100644 index 00000000000..a286fa0502f --- /dev/null +++ b/pkg/cli/workflows/test-top-level-github-app-override.md @@ -0,0 +1,33 @@ +--- +on: + issues: + types: [opened] + reaction: eyes + github-app: + app-id: ${{ vars.ACTIVATION_APP_ID }} + private-key: ${{ secrets.ACTIVATION_APP_KEY }} +permissions: + contents: read + +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +safe-outputs: + github-app: + app-id: ${{ vars.SAFE_OUTPUTS_APP_ID }} + private-key: ${{ secrets.SAFE_OUTPUTS_APP_KEY }} + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +# Section-Specific GitHub App Takes Precedence Over Top-Level + +This workflow demonstrates that section-specific github-app configurations take precedence +over the top-level github-app fallback. + +- `on.github-app` is explicitly set → activation uses ACTIVATION_APP_ID, not APP_ID +- `safe-outputs.github-app` is explicitly set → safe-outputs uses SAFE_OUTPUTS_APP_ID, not APP_ID + +The top-level github-app (APP_ID) is only used as a fallback when sections do not define +their own github-app. diff --git a/pkg/cli/workflows/test-top-level-github-app-safe-outputs.md b/pkg/cli/workflows/test-top-level-github-app-safe-outputs.md new file mode 100644 index 00000000000..3ef2ebe05a9 --- /dev/null +++ b/pkg/cli/workflows/test-top-level-github-app-safe-outputs.md @@ -0,0 +1,25 @@ +--- +on: + issues: + types: [opened] +permissions: + contents: read + +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +safe-outputs: + create-issue: + title-prefix: "[automated] " + labels: [automation] +engine: copilot +--- + +# Top-Level GitHub App Fallback for Safe Outputs + +This workflow demonstrates using a top-level github-app as a fallback for safe-outputs. + +The top-level `github-app` is automatically applied to the safe-outputs job when no +section-specific `github-app` is defined under `safe-outputs:`. + +When an issue is opened, analyze it and create a follow-up issue using the GitHub App token. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index a0f4abd5dc8..c5713c64db9 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -8162,6 +8162,10 @@ "additionalProperties": false } ] + }, + "github-app": { + "$ref": "#/$defs/github_app", + "description": "Top-level GitHub App configuration used as a fallback for all nested github-app token minting operations (on, safe-outputs, checkout, tools.github, dependencies). When a nested section does not define its own github-app, this top-level configuration is used automatically." } }, "additionalProperties": false, diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 90fd4d35641..f31b4f9ba9e 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -564,6 +564,87 @@ func (c *Compiler) mergeJobsFromYAMLImports(mainJobs map[string]any, mergedJobsJ return result } +// extractTopLevelGitHubApp extracts the 'github-app' field from the top-level frontmatter. +// This provides a single GitHub App configuration that serves as a fallback for all nested +// github-app token minting operations (on, safe-outputs, checkout, tools.github, dependencies). +func extractTopLevelGitHubApp(frontmatter map[string]any) *GitHubAppConfig { + appAny, ok := frontmatter["github-app"] + if !ok { + return nil + } + appMap, ok := appAny.(map[string]any) + if !ok { + return nil + } + app := parseAppConfig(appMap) + if app.AppID == "" || app.PrivateKey == "" { + return nil + } + return app +} + +// applyTopLevelGitHubAppFallbacks applies the top-level github-app as a fallback for all +// nested github-app token minting operations when no section-specific github-app is configured. +// Precedence: section-specific github-app > top-level github-app > no token minting. +func applyTopLevelGitHubAppFallbacks(data *WorkflowData) { + fallback := data.TopLevelGitHubApp + if fallback == nil { + return + } + + // Fallback for activation (on.github-app) + if data.ActivationGitHubApp == nil { + orchestratorWorkflowLog.Print("Applying top-level github-app fallback for activation") + data.ActivationGitHubApp = fallback + } + + // Fallback for safe-outputs + if data.SafeOutputs != nil && data.SafeOutputs.GitHubApp == nil { + orchestratorWorkflowLog.Print("Applying top-level github-app fallback for safe-outputs") + data.SafeOutputs.GitHubApp = fallback + } + + // Fallback for checkout configs: apply only when neither github-token nor github-app is set + for _, cfg := range data.CheckoutConfigs { + if cfg.GitHubApp == nil && cfg.GitHubToken == "" { + orchestratorWorkflowLog.Print("Applying top-level github-app fallback for checkout") + cfg.GitHubApp = fallback + } + } + + // Fallback for tools.github (MCP GitHub token minting) + if data.ParsedTools != nil && data.ParsedTools.GitHub != nil && data.ParsedTools.GitHub.GitHubApp == nil { + orchestratorWorkflowLog.Print("Applying top-level github-app fallback for tools.github") + data.ParsedTools.GitHub.GitHubApp = fallback + // Also update the raw tools map so applyDefaultTools (called from applyDefaults in + // processOnSectionAndFilters) does not lose the fallback when it rebuilds ParsedTools + // from the map. + if github, ok := data.Tools["github"].(map[string]any); ok { + appMap := map[string]any{ + "app-id": fallback.AppID, + "private-key": fallback.PrivateKey, + } + if fallback.Owner != "" { + appMap["owner"] = fallback.Owner + } + if len(fallback.Repositories) > 0 { + repos := make([]any, len(fallback.Repositories)) + for i, r := range fallback.Repositories { + repos[i] = r + } + appMap["repositories"] = repos + } + github["github-app"] = appMap + } + } + + // Fallback for APM dependencies + if data.APMDependencies != nil && data.APMDependencies.GitHubApp == nil { + orchestratorWorkflowLog.Print("Applying top-level github-app fallback for dependencies") + data.APMDependencies.GitHubApp = fallback + } +} + // extractAdditionalConfigurations extracts cache-memory, repo-memory, mcp-scripts, and safe-outputs configurations func (c *Compiler) extractAdditionalConfigurations( frontmatter map[string]any, @@ -611,6 +692,7 @@ func (c *Compiler) extractAdditionalConfigurations( workflowData.SkipBots = c.mergeSkipBots(c.extractSkipBots(frontmatter), importsResult.MergedSkipBots) workflowData.ActivationGitHubToken = c.resolveActivationGitHubToken(frontmatter, importsResult) workflowData.ActivationGitHubApp = c.resolveActivationGitHubApp(frontmatter, importsResult) + workflowData.TopLevelGitHubApp = extractTopLevelGitHubApp(frontmatter) // Use the already extracted output configuration workflowData.SafeOutputs = safeOutputs @@ -678,6 +760,10 @@ func (c *Compiler) extractAdditionalConfigurations( // This ensures every workflow with safe-outputs has at least one meaningful action handler. applyDefaultCreateIssue(workflowData) + // Apply the top-level github-app as a fallback for all nested github-app token minting operations. + // This runs last so that all section-specific configurations have been resolved first. + applyTopLevelGitHubAppFallbacks(workflowData) + return nil } diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index c1e81d5e6a7..dc3ed383288 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -394,6 +394,7 @@ type WorkflowData struct { StatusComment *bool // whether to post status comments (default: true when ai-reaction is set, false otherwise) ActivationGitHubToken string // custom github token from on.github-token for reactions/comments ActivationGitHubApp *GitHubAppConfig // github app config from on.github-app for minting activation tokens + TopLevelGitHubApp *GitHubAppConfig // top-level github-app fallback for all nested github-app token minting operations LockForAgent bool // whether to lock the issue during agent workflow execution Jobs map[string]any // custom job configurations with dependencies Cache string // cache configuration diff --git a/pkg/workflow/top_level_github_app_integration_test.go b/pkg/workflow/top_level_github_app_integration_test.go new file mode 100644 index 00000000000..54b67383ecc --- /dev/null +++ b/pkg/workflow/top_level_github_app_integration_test.go @@ -0,0 +1,396 @@ +//go:build integration + +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestTopLevelGitHubAppFallback tests that a top-level github-app field in the frontmatter +// is used as a fallback for all nested github-app token minting operations when no +// section-specific github-app is configured. +func TestTopLevelGitHubAppFallback(t *testing.T) { + tmpDir := testutil.TempDir(t, "top-level-github-app-test") + + t.Run("fallback applied to safe-outputs when no section-specific github-app", func(t *testing.T) { + content := `--- +name: Top Level GitHub App Safe Outputs Fallback +on: + issues: + types: [opened] +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +Test workflow verifying top-level github-app fallback for safe-outputs. +` + mdPath := filepath.Join(tmpDir, "test-safe-outputs-fallback.md") + require.NoError(t, os.WriteFile(mdPath, []byte(content), 0600)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mdPath) + require.NoError(t, err, "Workflow with top-level github-app should compile successfully") + + lockPath := filepath.Join(tmpDir, "test-safe-outputs-fallback.lock.yml") + compiledBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + compiled := string(compiledBytes) + + // The safe-outputs job should use the top-level github-app for token minting + assert.Contains(t, compiled, "id: safe-outputs-app-token", + "Safe outputs job should generate a token minting step") + assert.Contains(t, compiled, "app-id: ${{ vars.APP_ID }}", + "Token minting step should use the top-level APP_ID") + assert.Contains(t, compiled, "private-key: ${{ secrets.APP_PRIVATE_KEY }}", + "Token minting step should use the top-level APP_PRIVATE_KEY") + }) + + t.Run("fallback applied to activation when no on.github-app", func(t *testing.T) { + content := `--- +name: Top Level GitHub App Activation Fallback +on: + issues: + types: [opened] + reaction: eyes +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +Test workflow verifying top-level github-app fallback for activation. +` + mdPath := filepath.Join(tmpDir, "test-activation-fallback.md") + require.NoError(t, os.WriteFile(mdPath, []byte(content), 0600)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mdPath) + require.NoError(t, err, "Workflow with top-level github-app should compile successfully") + + lockPath := filepath.Join(tmpDir, "test-activation-fallback.lock.yml") + compiledBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + compiled := string(compiledBytes) + + // The activation job should use the top-level github-app for token minting + assert.Contains(t, compiled, "id: activation-app-token", + "Activation job should generate a token minting step using top-level github-app") + assert.Contains(t, compiled, "app-id: ${{ vars.APP_ID }}", + "Token minting step should use the top-level APP_ID") + // The reaction step should use the minted app token + assert.Contains(t, compiled, "github-token: ${{ steps.activation-app-token.outputs.token }}", + "Activation step should use the minted app token") + }) + + t.Run("fallback applied to checkout when no checkout.github-app", func(t *testing.T) { + content := `--- +name: Top Level GitHub App Checkout Fallback +on: + issues: + types: [opened] +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +checkout: + repository: myorg/private-repo + path: private +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +Test workflow verifying top-level github-app fallback for checkout. +` + mdPath := filepath.Join(tmpDir, "test-checkout-fallback.md") + require.NoError(t, os.WriteFile(mdPath, []byte(content), 0600)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mdPath) + require.NoError(t, err, "Workflow with top-level github-app should compile successfully") + + lockPath := filepath.Join(tmpDir, "test-checkout-fallback.lock.yml") + compiledBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + compiled := string(compiledBytes) + + // The checkout should use the top-level github-app for token minting + assert.Contains(t, compiled, "id: checkout-app-token-0", + "Checkout should generate a token minting step using top-level github-app") + assert.Contains(t, compiled, "app-id: ${{ vars.APP_ID }}", + "Token minting step should use the top-level APP_ID") + }) + + t.Run("fallback applied to tools.github when no tools.github.github-app", func(t *testing.T) { + content := `--- +name: Top Level GitHub App MCP Fallback +on: + issues: + types: [opened] +permissions: + contents: read + issues: read + pull-requests: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +tools: + github: + mode: remote + toolsets: [default] +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +Test workflow verifying top-level github-app fallback for tools.github. +` + mdPath := filepath.Join(tmpDir, "test-mcp-fallback.md") + require.NoError(t, os.WriteFile(mdPath, []byte(content), 0600)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mdPath) + require.NoError(t, err, "Workflow with top-level github-app should compile successfully") + + lockPath := filepath.Join(tmpDir, "test-mcp-fallback.lock.yml") + compiledBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + compiled := string(compiledBytes) + + // The agent job should use the top-level github-app for GitHub MCP token minting + assert.Contains(t, compiled, "id: github-mcp-app-token", + "Agent job should generate a GitHub MCP token minting step using top-level github-app") + assert.Contains(t, compiled, "app-id: ${{ vars.APP_ID }}", + "Token minting step should use the top-level APP_ID") + }) + + t.Run("fallback applied to APM dependencies when no dependencies.github-app", func(t *testing.T) { + content := `--- +name: Top Level GitHub App APM Dependencies Fallback +on: + issues: + types: [opened] +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +dependencies: + packages: + - myorg/private-skill +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +Test workflow verifying top-level github-app fallback for APM dependencies. +` + mdPath := filepath.Join(tmpDir, "test-dependencies-fallback.md") + require.NoError(t, os.WriteFile(mdPath, []byte(content), 0600)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mdPath) + require.NoError(t, err, "Workflow with top-level github-app should compile successfully") + + lockPath := filepath.Join(tmpDir, "test-dependencies-fallback.lock.yml") + compiledBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + compiled := string(compiledBytes) + + // The activation job should have an APM app token minting step using the top-level github-app + assert.Contains(t, compiled, "id: apm-app-token", + "Activation job should generate an APM app token minting step using top-level github-app") + assert.Contains(t, compiled, "app-id: ${{ vars.APP_ID }}", + "APM token minting step should use the top-level APP_ID") + }) + + t.Run("section-specific github-app takes precedence over top-level", func(t *testing.T) { + content := `--- +name: Section Specific GitHub App Precedence +on: + issues: + types: [opened] + reaction: eyes + github-app: + app-id: ${{ vars.ACTIVATION_APP_ID }} + private-key: ${{ secrets.ACTIVATION_APP_KEY }} +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +safe-outputs: + github-app: + app-id: ${{ vars.SAFE_OUTPUTS_APP_ID }} + private-key: ${{ secrets.SAFE_OUTPUTS_APP_KEY }} + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +Test workflow verifying section-specific github-app takes precedence over top-level fallback. +` + mdPath := filepath.Join(tmpDir, "test-section-precedence.md") + require.NoError(t, os.WriteFile(mdPath, []byte(content), 0600)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mdPath) + require.NoError(t, err, "Workflow with section-specific github-app configs should compile successfully") + + lockPath := filepath.Join(tmpDir, "test-section-precedence.lock.yml") + compiledBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + compiled := string(compiledBytes) + + // The safe-outputs job should use SAFE_OUTPUTS_APP_ID (section-specific), not APP_ID (top-level) + assert.Contains(t, compiled, "app-id: ${{ vars.SAFE_OUTPUTS_APP_ID }}", + "Safe outputs job should use section-specific SAFE_OUTPUTS_APP_ID") + assert.Contains(t, compiled, "app-id: ${{ vars.ACTIVATION_APP_ID }}", + "Activation job should use section-specific ACTIVATION_APP_ID") + // The top-level APP_ID should NOT appear anywhere because it's overridden by section-specific + // configs in both on.github-app and safe-outputs.github-app. SAFE_OUTPUTS_APP_ID and + // ACTIVATION_APP_ID are distinct values from APP_ID, so their presence does not conflict + // with this assertion. + assert.NotContains(t, compiled, "app-id: ${{ vars.APP_ID }}", + "Top-level APP_ID should NOT be used when section-specific configs are present") + }) + + t.Run("no fallback applied when no top-level github-app", func(t *testing.T) { + content := `--- +name: No Top Level GitHub App +on: + issues: + types: [opened] +permissions: + contents: read +safe-outputs: + create-issue: + title-prefix: "[automated] " +engine: copilot +--- + +Test workflow verifying no token minting when no github-app is configured. +` + mdPath := filepath.Join(tmpDir, "test-no-fallback.md") + require.NoError(t, os.WriteFile(mdPath, []byte(content), 0600)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mdPath) + require.NoError(t, err, "Workflow without github-app should compile successfully") + + lockPath := filepath.Join(tmpDir, "test-no-fallback.lock.yml") + compiledBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + compiled := string(compiledBytes) + + // No token minting steps should be generated + assert.NotContains(t, compiled, "id: safe-outputs-app-token", + "No safe-outputs token minting step should be generated without github-app") + assert.NotContains(t, compiled, "id: activation-app-token", + "No activation token minting step should be generated without github-app") + }) +} + +// TestTopLevelGitHubAppWorkflowFiles verifies that the sample workflow files in +// pkg/cli/workflows compile successfully and produce the expected token minting steps. +func TestTopLevelGitHubAppWorkflowFiles(t *testing.T) { + tmpDir := testutil.TempDir(t, "top-level-github-app-workflow-files-test") + + tests := []struct { + name string + workflowFile string + expectContains []string + }{ + { + name: "safe-outputs fallback workflow file", + workflowFile: "../cli/workflows/test-top-level-github-app-safe-outputs.md", + expectContains: []string{ + "id: safe-outputs-app-token", + "app-id: ${{ vars.APP_ID }}", + "private-key: ${{ secrets.APP_PRIVATE_KEY }}", + }, + }, + { + name: "activation fallback workflow file", + workflowFile: "../cli/workflows/test-top-level-github-app-activation.md", + expectContains: []string{ + "id: activation-app-token", + "app-id: ${{ vars.APP_ID }}", + "github-token: ${{ steps.activation-app-token.outputs.token }}", + }, + }, + { + name: "checkout fallback workflow file", + workflowFile: "../cli/workflows/test-top-level-github-app-checkout.md", + expectContains: []string{ + "id: checkout-app-token-0", + "app-id: ${{ vars.APP_ID }}", + }, + }, + { + name: "dependencies fallback workflow file", + workflowFile: "../cli/workflows/test-top-level-github-app-dependencies.md", + expectContains: []string{ + "id: apm-app-token", + "app-id: ${{ vars.APP_ID }}", + }, + }, + { + name: "section-specific override workflow file", + workflowFile: "../cli/workflows/test-top-level-github-app-override.md", + expectContains: []string{ + "app-id: ${{ vars.SAFE_OUTPUTS_APP_ID }}", + "app-id: ${{ vars.ACTIVATION_APP_ID }}", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + src, err := os.ReadFile(tt.workflowFile) + require.NoError(t, err, "Failed to read workflow file %s", tt.workflowFile) + + baseName := filepath.Base(tt.workflowFile) + mdDst := filepath.Join(tmpDir, baseName) + require.NoError(t, os.WriteFile(mdDst, src, 0600)) + + compiler := NewCompiler() + err = compiler.CompileWorkflow(mdDst) + require.NoError(t, err, "Workflow file %s should compile successfully", tt.workflowFile) + + lockName := strings.TrimSuffix(baseName, ".md") + ".lock.yml" + lockPath := filepath.Join(tmpDir, lockName) + compiledBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + compiled := string(compiledBytes) + + for _, expected := range tt.expectContains { + assert.Contains(t, compiled, expected, + "Compiled workflow should contain %q", expected) + } + }) + } +} From 3d9dc5619f0ac6558c28b82ba26b8c55afcb3e45 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 03:32:13 +0000 Subject: [PATCH 3/7] Support importing top-level github-app from shared agent workflows Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/import_field_extractor.go | 40 +++- pkg/parser/import_processor.go | 1 + .../compiler_orchestrator_workflow.go | 24 ++- .../top_level_github_app_import_test.go | 193 ++++++++++++++++++ 4 files changed, 252 insertions(+), 6 deletions(-) create mode 100644 pkg/workflow/top_level_github_app_import_test.go diff --git a/pkg/parser/import_field_extractor.go b/pkg/parser/import_field_extractor.go index 5621d441010..fd0a5d35141 100644 --- a/pkg/parser/import_field_extractor.go +++ b/pkg/parser/import_field_extractor.go @@ -50,6 +50,8 @@ type importAccumulator struct { // First on.github-token / on.github-app found across all imported files (first-wins strategy) activationGitHubToken string activationGitHubApp string // JSON-encoded GitHubAppConfig + // First top-level github-app found across all imported files (first-wins strategy) + topLevelGitHubApp string // JSON-encoded GitHubAppConfig } // newImportAccumulator creates and initializes a new importAccumulator. @@ -228,6 +230,14 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import } } + // Extract top-level github-app from imported file (first-wins: only set if not yet populated) + if acc.topLevelGitHubApp == "" { + if appJSON := extractTopLevelGitHubApp(string(content)); appJSON != "" { + acc.topLevelGitHubApp = appJSON + log.Printf("Extracted top-level github-app from import: %s", item.fullPath) + } + } + // Extract and merge plugins from imported file (merge into set to avoid duplicates). // Handles both simple string format and object format with MCP configs. pluginsContent, err := extractFrontmatterField(string(content), "plugins", "[]") @@ -331,6 +341,7 @@ func (acc *importAccumulator) toImportsResult(topologicalOrder []string) *Import ImportInputs: acc.importInputs, MergedActivationGitHubToken: acc.activationGitHubToken, MergedActivationGitHubApp: acc.activationGitHubApp, + MergedTopLevelGitHubApp: acc.topLevelGitHubApp, } } @@ -370,11 +381,10 @@ func extractOnGitHubToken(content string) string { return token } -// extractOnGitHubApp returns the JSON-encoded on.github-app object from workflow content. -// Returns "" if the field is absent, not a valid object, or missing required fields. -func extractOnGitHubApp(content string) string { - appJSON, err := extractOnSectionAnyField(content, "github-app") - if err != nil || appJSON == "" || appJSON == "null" { +// validateGitHubAppJSON validates that a JSON-encoded GitHub App configuration has the required +// fields (app-id and private-key). Returns the input JSON if valid, or "" otherwise. +func validateGitHubAppJSON(appJSON string) string { + if appJSON == "" || appJSON == "null" { return "" } var appMap map[string]any @@ -389,3 +399,23 @@ func extractOnGitHubApp(content string) string { } return appJSON } + +// extractOnGitHubApp returns the JSON-encoded on.github-app object from workflow content. +// Returns "" if the field is absent, not a valid object, or missing required fields. +func extractOnGitHubApp(content string) string { + appJSON, err := extractOnSectionAnyField(content, "github-app") + if err != nil { + return "" + } + return validateGitHubAppJSON(appJSON) +} + +// extractTopLevelGitHubApp returns the JSON-encoded top-level github-app object from workflow content. +// Returns "" if the field is absent, not a valid object, or missing required fields. +func extractTopLevelGitHubApp(content string) string { + appJSON, err := extractFrontmatterField(content, "github-app", "") + if err != nil { + return "" + } + return validateGitHubAppJSON(appJSON) +} diff --git a/pkg/parser/import_processor.go b/pkg/parser/import_processor.go index 872afbf88ad..3acc23c63fa 100644 --- a/pkg/parser/import_processor.go +++ b/pkg/parser/import_processor.go @@ -34,6 +34,7 @@ type ImportsResult struct { MergedSkipBots []string // Merged skip-bots list from all imports (union of usernames) MergedActivationGitHubToken string // GitHub token from on.github-token in first imported workflow that defines it MergedActivationGitHubApp string // JSON-encoded on.github-app from first imported workflow that defines it + MergedTopLevelGitHubApp string // JSON-encoded top-level github-app from first imported workflow that defines it MergedPostSteps string // Merged post-steps configuration from all imports (appended in order) MergedLabels []string // Merged labels from all imports (union of label names) MergedCaches []string // Merged cache configurations from all imports (appended in order) diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index f31b4f9ba9e..f032aa38b92 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -583,6 +583,28 @@ func extractTopLevelGitHubApp(frontmatter map[string]any) *GitHubAppConfig { return app } +// resolveTopLevelGitHubApp resolves the top-level github-app for token minting fallback. +// Precedence: +// 1. Current workflow's top-level github-app (explicit override wins) +// 2. First top-level github-app found across imported shared workflows +// 3. Nil (no fallback configured) +func resolveTopLevelGitHubApp(frontmatter map[string]any, importsResult *parser.ImportsResult) *GitHubAppConfig { + if app := extractTopLevelGitHubApp(frontmatter); app != nil { + return app + } + if importsResult != nil && importsResult.MergedTopLevelGitHubApp != "" { + var appMap map[string]any + if err := json.Unmarshal([]byte(importsResult.MergedTopLevelGitHubApp), &appMap); err == nil { + app := parseAppConfig(appMap) + if app.AppID != "" && app.PrivateKey != "" { + orchestratorWorkflowLog.Print("Using top-level github-app from imported shared workflow") + return app + } + } + } + return nil +} + // applyTopLevelGitHubAppFallbacks applies the top-level github-app as a fallback for all // nested github-app token minting operations when no section-specific github-app is configured. // Precedence: section-specific github-app > top-level github-app > no token minting. @@ -692,7 +714,7 @@ func (c *Compiler) extractAdditionalConfigurations( workflowData.SkipBots = c.mergeSkipBots(c.extractSkipBots(frontmatter), importsResult.MergedSkipBots) workflowData.ActivationGitHubToken = c.resolveActivationGitHubToken(frontmatter, importsResult) workflowData.ActivationGitHubApp = c.resolveActivationGitHubApp(frontmatter, importsResult) - workflowData.TopLevelGitHubApp = extractTopLevelGitHubApp(frontmatter) + workflowData.TopLevelGitHubApp = resolveTopLevelGitHubApp(frontmatter, importsResult) // Use the already extracted output configuration workflowData.SafeOutputs = safeOutputs diff --git a/pkg/workflow/top_level_github_app_import_test.go b/pkg/workflow/top_level_github_app_import_test.go new file mode 100644 index 00000000000..9eb0af9debc --- /dev/null +++ b/pkg/workflow/top_level_github_app_import_test.go @@ -0,0 +1,193 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestTopLevelGitHubAppImport tests that a top-level github-app can be imported +// from a shared agent workflow and propagated as a fallback for all nested operations. +func TestTopLevelGitHubAppImport(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + // Create a temporary directory simulating .github/workflows layout + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + // Shared workflow that declares a top-level github-app + sharedWorkflow := `--- +github-app: + app-id: ${{ vars.SHARED_APP_ID }} + private-key: ${{ secrets.SHARED_APP_SECRET }} +safe-outputs: + create-issue: +--- + +# Shared GitHub App Configuration + +This shared workflow provides a top-level github-app for all operations. +` + require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "shared-app.md"), []byte(sharedWorkflow), 0644)) + + // Main workflow that imports the shared workflow but does NOT set its own github-app + mainWorkflow := `--- +on: issues +permissions: + contents: read +imports: + - ./shared-app.md +safe-outputs: + create-issue: +--- + +# Main Workflow + +This workflow imports the top-level github-app from the shared workflow. +` + mainFile := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mainFile, []byte(mainWorkflow), 0644)) + + // Change to workflows directory so relative imports resolve correctly + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + // The top-level github-app from the shared workflow should be resolved + require.NotNil(t, data.TopLevelGitHubApp, "TopLevelGitHubApp should be populated from import") + assert.Equal(t, "${{ vars.SHARED_APP_ID }}", data.TopLevelGitHubApp.AppID, + "TopLevelGitHubApp.AppID should come from the shared workflow") + assert.Equal(t, "${{ secrets.SHARED_APP_SECRET }}", data.TopLevelGitHubApp.PrivateKey, + "TopLevelGitHubApp.PrivateKey should come from the shared workflow") + + // The fallback should also propagate to safe-outputs (since safe-outputs has no explicit github-app) + require.NotNil(t, data.SafeOutputs, "SafeOutputs should be populated") + require.NotNil(t, data.SafeOutputs.GitHubApp, + "SafeOutputs.GitHubApp should be populated from the imported top-level github-app") + assert.Equal(t, "${{ vars.SHARED_APP_ID }}", data.SafeOutputs.GitHubApp.AppID, + "SafeOutputs should use the imported top-level github-app") +} + +// TestTopLevelGitHubAppImportOverride tests that the current workflow's own top-level +// github-app takes precedence over one imported from a shared workflow. +func TestTopLevelGitHubAppImportOverride(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + // Shared workflow with a top-level github-app + sharedWorkflow := `--- +github-app: + app-id: ${{ vars.SHARED_APP_ID }} + private-key: ${{ secrets.SHARED_APP_SECRET }} +safe-outputs: + create-issue: +--- + +# Shared GitHub App Configuration +` + require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "shared-app.md"), []byte(sharedWorkflow), 0644)) + + // Main workflow that has its OWN top-level github-app (should win) + mainWorkflow := `--- +on: issues +permissions: + contents: read +imports: + - ./shared-app.md +github-app: + app-id: ${{ vars.LOCAL_APP_ID }} + private-key: ${{ secrets.LOCAL_APP_SECRET }} +safe-outputs: + create-issue: +--- + +# Main Workflow + +This workflow's own top-level github-app takes precedence over the imported one. +` + mainFile := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mainFile, []byte(mainWorkflow), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + // The current workflow's top-level github-app should win + require.NotNil(t, data.TopLevelGitHubApp, "TopLevelGitHubApp should be populated") + assert.Equal(t, "${{ vars.LOCAL_APP_ID }}", data.TopLevelGitHubApp.AppID, + "Current workflow's github-app should take precedence over the imported one") + assert.Equal(t, "${{ secrets.LOCAL_APP_SECRET }}", data.TopLevelGitHubApp.PrivateKey, + "Current workflow's github-app should take precedence over the imported one") +} + +// TestTopLevelGitHubAppImportActivation tests that a top-level github-app imported from a shared +// workflow is propagated to the activation job (reactions/status comments). +func TestTopLevelGitHubAppImportActivation(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + sharedWorkflow := `--- +github-app: + app-id: ${{ vars.SHARED_APP_ID }} + private-key: ${{ secrets.SHARED_APP_SECRET }} +safe-outputs: + create-issue: +--- + +# Shared GitHub App Configuration +` + require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "shared-app.md"), []byte(sharedWorkflow), 0644)) + + // Workflow with a reaction trigger – no explicit on.github-app + mainWorkflow := `--- +on: + issues: + types: [opened] + reaction: eyes +permissions: + contents: read +imports: + - ./shared-app.md +safe-outputs: + create-issue: +--- + +# Main Workflow With Reaction +` + mainFile := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mainFile, []byte(mainWorkflow), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + // The imported top-level github-app should propagate to activation + require.NotNil(t, data.ActivationGitHubApp, + "ActivationGitHubApp should be populated from the imported top-level github-app") + assert.Equal(t, "${{ vars.SHARED_APP_ID }}", data.ActivationGitHubApp.AppID, + "Activation should use the imported top-level github-app") +} From 8ade6cbca8179a6f10e22067df99d41607ca2c44 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 03:43:44 +0000 Subject: [PATCH 4/7] Fix tools.github fallback edge cases and add missing MCP integration test entry Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../compiler_orchestrator_workflow.go | 40 ++++++++------ .../top_level_github_app_import_test.go | 54 ++++++++++++++++++- .../top_level_github_app_integration_test.go | 8 +++ 3 files changed, 85 insertions(+), 17 deletions(-) diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index f032aa38b92..bd8194ac843 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -634,29 +634,37 @@ func applyTopLevelGitHubAppFallbacks(data *WorkflowData) { } } - // Fallback for tools.github (MCP GitHub token minting) - if data.ParsedTools != nil && data.ParsedTools.GitHub != nil && data.ParsedTools.GitHub.GitHubApp == nil { + // Fallback for tools.github (MCP GitHub token minting). + // Skip when a custom github-token is already configured for the MCP server (token takes priority). + if data.ParsedTools != nil && data.ParsedTools.GitHub != nil && + data.ParsedTools.GitHub.GitHubApp == nil && data.ParsedTools.GitHub.GitHubToken == "" { orchestratorWorkflowLog.Print("Applying top-level github-app fallback for tools.github") data.ParsedTools.GitHub.GitHubApp = fallback // Also update the raw tools map so applyDefaultTools (called from applyDefaults in // processOnSectionAndFilters) does not lose the fallback when it rebuilds ParsedTools // from the map. - if github, ok := data.Tools["github"].(map[string]any); ok { - appMap := map[string]any{ - "app-id": fallback.AppID, - "private-key": fallback.PrivateKey, - } - if fallback.Owner != "" { - appMap["owner"] = fallback.Owner - } - if len(fallback.Repositories) > 0 { - repos := make([]any, len(fallback.Repositories)) - for i, r := range fallback.Repositories { - repos[i] = r - } - appMap["repositories"] = repos + appMap := map[string]any{ + "app-id": fallback.AppID, + "private-key": fallback.PrivateKey, + } + if fallback.Owner != "" { + appMap["owner"] = fallback.Owner + } + if len(fallback.Repositories) > 0 { + repos := make([]any, len(fallback.Repositories)) + for i, r := range fallback.Repositories { + repos[i] = r } + appMap["repositories"] = repos + } + // Normalize data.Tools["github"] to a map so the github-app survives re-parsing. + // Configurations like `github: true` are normalized here rather than losing the fallback. + if github, ok := data.Tools["github"].(map[string]any); ok { + // Already a map; inject into existing settings. github["github-app"] = appMap + } else { + // Non-map value (e.g. true/false) — create a fresh map preserving the enabled signal. + data.Tools["github"] = map[string]any{"github-app": appMap} } } diff --git a/pkg/workflow/top_level_github_app_import_test.go b/pkg/workflow/top_level_github_app_import_test.go index 9eb0af9debc..82b089b0ce3 100644 --- a/pkg/workflow/top_level_github_app_import_test.go +++ b/pkg/workflow/top_level_github_app_import_test.go @@ -137,7 +137,59 @@ This workflow's own top-level github-app takes precedence over the imported one. "Current workflow's github-app should take precedence over the imported one") } -// TestTopLevelGitHubAppImportActivation tests that a top-level github-app imported from a shared +// TestTopLevelGitHubAppToolsGitHubTokenSkip tests that the fallback is NOT applied +// to tools.github when a custom github-token is already configured for the MCP server. +func TestTopLevelGitHubAppToolsGitHubTokenSkip(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + // Use ParseWorkflowFile directly with inline frontmatter + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + // Workflow with top-level github-app but tools.github uses an explicit github-token + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +tools: + github: + mode: remote + toolsets: [default] + github-token: ${{ secrets.CUSTOM_PAT }} +engine: copilot +--- + +# Workflow With Explicit GitHub Token for MCP + +When tools.github.github-token is set, the top-level github-app fallback should NOT override it. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + // The top-level github-app should be resolved at the top level + require.NotNil(t, data.TopLevelGitHubApp, "TopLevelGitHubApp should be populated") + + // But it must NOT be injected into tools.github because github-token takes precedence + require.NotNil(t, data.ParsedTools, "ParsedTools should be populated") + require.NotNil(t, data.ParsedTools.GitHub, "ParsedTools.GitHub should be populated") + assert.Equal(t, "${{ secrets.CUSTOM_PAT }}", data.ParsedTools.GitHub.GitHubToken, + "tools.github.github-token should be preserved") + assert.Nil(t, data.ParsedTools.GitHub.GitHubApp, + "tools.github.github-app should NOT be set when github-token is configured") +} + // workflow is propagated to the activation job (reactions/status comments). func TestTopLevelGitHubAppImportActivation(t *testing.T) { compiler := NewCompilerWithVersion("1.0.0") diff --git a/pkg/workflow/top_level_github_app_integration_test.go b/pkg/workflow/top_level_github_app_integration_test.go index 54b67383ecc..984cbc06f49 100644 --- a/pkg/workflow/top_level_github_app_integration_test.go +++ b/pkg/workflow/top_level_github_app_integration_test.go @@ -366,6 +366,14 @@ func TestTopLevelGitHubAppWorkflowFiles(t *testing.T) { "app-id: ${{ vars.ACTIVATION_APP_ID }}", }, }, + { + name: "tools.github MCP fallback workflow file", + workflowFile: "../cli/workflows/test-top-level-github-app-mcp.md", + expectContains: []string{ + "id: github-mcp-app-token", + "app-id: ${{ vars.APP_ID }}", + }, + }, } for _, tt := range tests { From ac4385561f0b5c3ce4759bd8d7e988a2b608a0cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 10:30:53 +0000 Subject: [PATCH 5/7] Fix github:false edge case - skip fallback when GitHub MCP tool is explicitly disabled Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../compiler_orchestrator_workflow.go | 6 ++- .../top_level_github_app_import_test.go | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index bd8194ac843..a6f077267de 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -636,8 +636,10 @@ func applyTopLevelGitHubAppFallbacks(data *WorkflowData) { // Fallback for tools.github (MCP GitHub token minting). // Skip when a custom github-token is already configured for the MCP server (token takes priority). + // Also skip when tools.github is explicitly disabled (github: false) — do not re-enable it. if data.ParsedTools != nil && data.ParsedTools.GitHub != nil && - data.ParsedTools.GitHub.GitHubApp == nil && data.ParsedTools.GitHub.GitHubToken == "" { + data.ParsedTools.GitHub.GitHubApp == nil && data.ParsedTools.GitHub.GitHubToken == "" && + data.Tools["github"] != false { orchestratorWorkflowLog.Print("Applying top-level github-app fallback for tools.github") data.ParsedTools.GitHub.GitHubApp = fallback // Also update the raw tools map so applyDefaultTools (called from applyDefaults in @@ -663,7 +665,7 @@ func applyTopLevelGitHubAppFallbacks(data *WorkflowData) { // Already a map; inject into existing settings. github["github-app"] = appMap } else { - // Non-map value (e.g. true/false) — create a fresh map preserving the enabled signal. + // Non-map value (e.g. true) — create a fresh map. data.Tools["github"] = map[string]any{"github-app": appMap} } } diff --git a/pkg/workflow/top_level_github_app_import_test.go b/pkg/workflow/top_level_github_app_import_test.go index 82b089b0ce3..31e2c4cb658 100644 --- a/pkg/workflow/top_level_github_app_import_test.go +++ b/pkg/workflow/top_level_github_app_import_test.go @@ -190,6 +190,52 @@ When tools.github.github-token is set, the top-level github-app fallback should "tools.github.github-app should NOT be set when github-token is configured") } +// TestTopLevelGitHubAppToolsGitHubFalseSkip tests that the fallback is NOT applied +// to tools.github when github is explicitly disabled (github: false). +func TestTopLevelGitHubAppToolsGitHubFalseSkip(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + // Workflow with top-level github-app but tools.github explicitly disabled + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +tools: + github: false +engine: copilot +--- + +# Workflow With GitHub Tool Explicitly Disabled + +When tools.github is set to false, the top-level github-app fallback should NOT re-enable it. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + // The top-level github-app should be resolved at the top level + require.NotNil(t, data.TopLevelGitHubApp, "TopLevelGitHubApp should be populated") + + // tools.github should remain disabled — applyDefaultTools removes the key when false. + // After compilation, ParsedTools.GitHub should be nil (no GitHub MCP tool enabled). + assert.Nil(t, data.ParsedTools.GitHub, + "ParsedTools.GitHub should be nil when tools.github: false — fallback must not re-enable it") +} + // workflow is propagated to the activation job (reactions/status comments). func TestTopLevelGitHubAppImportActivation(t *testing.T) { compiler := NewCompilerWithVersion("1.0.0") From b426ffd2023c9e968cf13b02f7bfbf161d781440 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 10:47:45 +0000 Subject: [PATCH 6/7] Add comprehensive override/skip tests for all five fallback targets; fix activation and safe-outputs github-token skip guards Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../compiler_orchestrator_workflow.go | 7 +- .../top_level_github_app_import_test.go | 546 ++++++++++++++++++ 2 files changed, 551 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index a6f077267de..66035496309 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -615,13 +615,16 @@ func applyTopLevelGitHubAppFallbacks(data *WorkflowData) { } // Fallback for activation (on.github-app) - if data.ActivationGitHubApp == nil { + // Skip when on.github-token is already explicitly configured — the token takes priority over + // app-based auth at runtime and injecting a github-app fallback would flip that precedence. + if data.ActivationGitHubApp == nil && data.ActivationGitHubToken == "" { orchestratorWorkflowLog.Print("Applying top-level github-app fallback for activation") data.ActivationGitHubApp = fallback } // Fallback for safe-outputs - if data.SafeOutputs != nil && data.SafeOutputs.GitHubApp == nil { + // Also skip when a custom github-token is already configured for safe-outputs. + if data.SafeOutputs != nil && data.SafeOutputs.GitHubApp == nil && data.SafeOutputs.GitHubToken == "" { orchestratorWorkflowLog.Print("Applying top-level github-app fallback for safe-outputs") data.SafeOutputs.GitHubApp = fallback } diff --git a/pkg/workflow/top_level_github_app_import_test.go b/pkg/workflow/top_level_github_app_import_test.go index 31e2c4cb658..c653ad1cd92 100644 --- a/pkg/workflow/top_level_github_app_import_test.go +++ b/pkg/workflow/top_level_github_app_import_test.go @@ -289,3 +289,549 @@ safe-outputs: assert.Equal(t, "${{ vars.SHARED_APP_ID }}", data.ActivationGitHubApp.AppID, "Activation should use the imported top-level github-app") } + +// TestTopLevelGitHubAppActivationOverride tests that an explicit on.github-app configuration +// takes precedence over the top-level github-app fallback. +func TestTopLevelGitHubAppActivationOverride(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: + issues: + types: [opened] + reaction: eyes + github-app: + app-id: ${{ vars.ACTIVATION_APP_ID }} + private-key: ${{ secrets.ACTIVATION_APP_KEY }} +permissions: + contents: read +github-app: + app-id: ${{ vars.TOP_LEVEL_APP_ID }} + private-key: ${{ secrets.TOP_LEVEL_APP_KEY }} +safe-outputs: + create-issue: +engine: copilot +--- + +# Workflow With Explicit on.github-app Override + +When on.github-app is explicitly set, it takes precedence over the top-level github-app. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.NotNil(t, data.TopLevelGitHubApp, "TopLevelGitHubApp should be populated") + assert.Equal(t, "${{ vars.TOP_LEVEL_APP_ID }}", data.TopLevelGitHubApp.AppID, + "TopLevelGitHubApp should hold the top-level app") + + // on.github-app should win over the top-level fallback + require.NotNil(t, data.ActivationGitHubApp, "ActivationGitHubApp should be populated") + assert.Equal(t, "${{ vars.ACTIVATION_APP_ID }}", data.ActivationGitHubApp.AppID, + "ActivationGitHubApp should use the section-specific on.github-app, not the top-level fallback") +} + +// TestTopLevelGitHubAppActivationTokenSkip tests that the top-level github-app fallback +// is NOT applied to activation when on.github-token is explicitly configured, because +// at runtime app tokens take precedence over tokens and injecting the fallback would flip +// the user's intended auth precedence. +func TestTopLevelGitHubAppActivationTokenSkip(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: + issues: + types: [opened] + reaction: eyes + github-token: ${{ secrets.CUSTOM_ACTIVATION_TOKEN }} +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +safe-outputs: + create-issue: +engine: copilot +--- + +# Workflow With on.github-token — Fallback Must Not Apply + +When on.github-token is set, the top-level github-app must NOT be applied to activation. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.NotNil(t, data.TopLevelGitHubApp, "TopLevelGitHubApp should be populated") + assert.Equal(t, "${{ secrets.CUSTOM_ACTIVATION_TOKEN }}", data.ActivationGitHubToken, + "ActivationGitHubToken should be preserved") + assert.Nil(t, data.ActivationGitHubApp, + "ActivationGitHubApp must be nil when on.github-token is set — fallback must not override it") +} + +// TestTopLevelGitHubAppSafeOutputsFallback tests that the top-level github-app is applied +// to safe-outputs when no section-specific github-app is configured. +func TestTopLevelGitHubAppSafeOutputsFallback(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +safe-outputs: + create-issue: +engine: copilot +--- + +# Top-level github-app fallback for safe-outputs. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.NotNil(t, data.SafeOutputs, "SafeOutputs should be populated") + require.NotNil(t, data.SafeOutputs.GitHubApp, + "SafeOutputs.GitHubApp should be populated from top-level fallback") + assert.Equal(t, "${{ vars.APP_ID }}", data.SafeOutputs.GitHubApp.AppID, + "SafeOutputs should use the top-level github-app fallback") +} + +// TestTopLevelGitHubAppSafeOutputsOverride tests that a section-specific safe-outputs.github-app +// takes precedence over the top-level github-app fallback. +func TestTopLevelGitHubAppSafeOutputsOverride(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.TOP_LEVEL_APP_ID }} + private-key: ${{ secrets.TOP_LEVEL_APP_KEY }} +safe-outputs: + github-app: + app-id: ${{ vars.SAFE_OUTPUTS_APP_ID }} + private-key: ${{ secrets.SAFE_OUTPUTS_APP_KEY }} + create-issue: +engine: copilot +--- + +# Section-specific safe-outputs.github-app overrides top-level fallback. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.NotNil(t, data.SafeOutputs, "SafeOutputs should be populated") + require.NotNil(t, data.SafeOutputs.GitHubApp, "SafeOutputs.GitHubApp should be populated") + assert.Equal(t, "${{ vars.SAFE_OUTPUTS_APP_ID }}", data.SafeOutputs.GitHubApp.AppID, + "SafeOutputs.GitHubApp should use the section-specific app, not the top-level fallback") +} + +// TestTopLevelGitHubAppSafeOutputsTokenSkip tests that the top-level github-app fallback +// is NOT applied to safe-outputs when safe-outputs.github-token is explicitly set. +func TestTopLevelGitHubAppSafeOutputsTokenSkip(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +safe-outputs: + github-token: ${{ secrets.CUSTOM_SAFE_OUTPUTS_TOKEN }} + create-issue: +engine: copilot +--- + +# safe-outputs.github-token is set — top-level github-app must not override it. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.NotNil(t, data.SafeOutputs, "SafeOutputs should be populated") + assert.Equal(t, "${{ secrets.CUSTOM_SAFE_OUTPUTS_TOKEN }}", data.SafeOutputs.GitHubToken, + "SafeOutputs.GitHubToken should be preserved") + assert.Nil(t, data.SafeOutputs.GitHubApp, + "SafeOutputs.GitHubApp must be nil when safe-outputs.github-token is configured") +} + +// TestTopLevelGitHubAppCheckoutFallback tests that the top-level github-app is applied +// to a checkout entry that has no explicit auth (no github-app, no github-token). +func TestTopLevelGitHubAppCheckoutFallback(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +checkout: + repository: myorg/private-repo + path: private +safe-outputs: + create-issue: +engine: copilot +--- + +# Top-level github-app fallback for checkout. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.Len(t, data.CheckoutConfigs, 1, "Should have one checkout config") + require.NotNil(t, data.CheckoutConfigs[0].GitHubApp, + "CheckoutConfig.GitHubApp should be populated from top-level fallback") + assert.Equal(t, "${{ vars.APP_ID }}", data.CheckoutConfigs[0].GitHubApp.AppID, + "Checkout should use the top-level github-app fallback") +} + +// TestTopLevelGitHubAppCheckoutOverride tests that a section-specific checkout.github-app +// takes precedence over the top-level github-app fallback. +func TestTopLevelGitHubAppCheckoutOverride(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.TOP_LEVEL_APP_ID }} + private-key: ${{ secrets.TOP_LEVEL_APP_KEY }} +checkout: + - repository: myorg/private-repo + path: private + github-app: + app-id: ${{ vars.CHECKOUT_APP_ID }} + private-key: ${{ secrets.CHECKOUT_APP_KEY }} +safe-outputs: + create-issue: +engine: copilot +--- + +# checkout.github-app overrides the top-level github-app fallback. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.Len(t, data.CheckoutConfigs, 1, "Should have one checkout config") + require.NotNil(t, data.CheckoutConfigs[0].GitHubApp, "CheckoutConfig.GitHubApp should be populated") + assert.Equal(t, "${{ vars.CHECKOUT_APP_ID }}", data.CheckoutConfigs[0].GitHubApp.AppID, + "Checkout should use its own section-specific github-app, not the top-level fallback") +} + +// TestTopLevelGitHubAppCheckoutTokenSkip tests that the top-level github-app fallback is NOT +// applied to a checkout entry that has an explicit github-token set. +func TestTopLevelGitHubAppCheckoutTokenSkip(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +checkout: + - repository: myorg/private-repo + path: private + github-token: ${{ secrets.CHECKOUT_PAT }} +safe-outputs: + create-issue: +engine: copilot +--- + +# checkout.github-token is set — top-level github-app must not override it. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.Len(t, data.CheckoutConfigs, 1, "Should have one checkout config") + assert.Equal(t, "${{ secrets.CHECKOUT_PAT }}", data.CheckoutConfigs[0].GitHubToken, + "CheckoutConfig.GitHubToken should be preserved") + assert.Nil(t, data.CheckoutConfigs[0].GitHubApp, + "CheckoutConfig.GitHubApp must be nil when checkout.github-token is configured") +} + +// TestTopLevelGitHubAppToolsFallback tests that the top-level github-app is applied +// to tools.github when no section-specific auth is configured. +func TestTopLevelGitHubAppToolsFallback(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +tools: + github: + mode: remote + toolsets: [default] +safe-outputs: + create-issue: +engine: copilot +--- + +# Top-level github-app fallback for tools.github. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.NotNil(t, data.ParsedTools, "ParsedTools should be populated") + require.NotNil(t, data.ParsedTools.GitHub, "ParsedTools.GitHub should be populated") + require.NotNil(t, data.ParsedTools.GitHub.GitHubApp, + "ParsedTools.GitHub.GitHubApp should be populated from top-level fallback") + assert.Equal(t, "${{ vars.APP_ID }}", data.ParsedTools.GitHub.GitHubApp.AppID, + "tools.github should use the top-level github-app fallback") +} + +// TestTopLevelGitHubAppToolsOverride tests that a section-specific tools.github.github-app +// takes precedence over the top-level github-app fallback. +func TestTopLevelGitHubAppToolsOverride(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.TOP_LEVEL_APP_ID }} + private-key: ${{ secrets.TOP_LEVEL_APP_KEY }} +tools: + github: + mode: remote + toolsets: [default] + github-app: + app-id: ${{ vars.MCP_APP_ID }} + private-key: ${{ secrets.MCP_APP_KEY }} +safe-outputs: + create-issue: +engine: copilot +--- + +# tools.github.github-app overrides the top-level github-app fallback. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.NotNil(t, data.ParsedTools, "ParsedTools should be populated") + require.NotNil(t, data.ParsedTools.GitHub, "ParsedTools.GitHub should be populated") + require.NotNil(t, data.ParsedTools.GitHub.GitHubApp, "ParsedTools.GitHub.GitHubApp should be populated") + assert.Equal(t, "${{ vars.MCP_APP_ID }}", data.ParsedTools.GitHub.GitHubApp.AppID, + "tools.github should use its own section-specific github-app, not the top-level fallback") +} + +// TestTopLevelGitHubAppDependenciesFallback tests that the top-level github-app is applied +// to APM dependencies when no section-specific github-app is configured. +func TestTopLevelGitHubAppDependenciesFallback(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +dependencies: + packages: + - myorg/private-skill +safe-outputs: + create-issue: +engine: copilot +--- + +# Top-level github-app fallback for APM dependencies. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.NotNil(t, data.APMDependencies, "APMDependencies should be populated") + require.NotNil(t, data.APMDependencies.GitHubApp, + "APMDependencies.GitHubApp should be populated from top-level fallback") + assert.Equal(t, "${{ vars.APP_ID }}", data.APMDependencies.GitHubApp.AppID, + "APM dependencies should use the top-level github-app fallback") +} + +// TestTopLevelGitHubAppDependenciesOverride tests that a section-specific dependencies.github-app +// takes precedence over the top-level github-app fallback. +func TestTopLevelGitHubAppDependenciesOverride(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + workflowContent := `--- +on: issues +permissions: + contents: read +github-app: + app-id: ${{ vars.TOP_LEVEL_APP_ID }} + private-key: ${{ secrets.TOP_LEVEL_APP_KEY }} +dependencies: + packages: + - myorg/private-skill + github-app: + app-id: ${{ vars.DEPS_APP_ID }} + private-key: ${{ secrets.DEPS_APP_KEY }} +safe-outputs: + create-issue: +engine: copilot +--- + +# dependencies.github-app overrides the top-level github-app fallback. +` + mdPath := filepath.Join(workflowsDir, "main.md") + require.NoError(t, os.WriteFile(mdPath, []byte(workflowContent), 0644)) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(workflowsDir)) + defer func() { _ = os.Chdir(origDir) }() + + data, err := compiler.ParseWorkflowFile("main.md") + require.NoError(t, err) + + require.NotNil(t, data.APMDependencies, "APMDependencies should be populated") + require.NotNil(t, data.APMDependencies.GitHubApp, "APMDependencies.GitHubApp should be populated") + assert.Equal(t, "${{ vars.DEPS_APP_ID }}", data.APMDependencies.GitHubApp.AppID, + "APM dependencies should use section-specific github-app, not the top-level fallback") +} From a9c5c39aedad06c77b9b961d6f47d05e27c76252 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 11:18:24 +0000 Subject: [PATCH 7/7] Extract topLevelFallbackNeeded helper and apply consistently across all five fallback sites Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../compiler_orchestrator_workflow.go | 44 ++++++++++++------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 66035496309..af882d0dddd 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -605,43 +605,55 @@ func resolveTopLevelGitHubApp(frontmatter map[string]any, importsResult *parser. return nil } +// topLevelFallbackNeeded reports whether the top-level github-app should be applied as a +// fallback for a given section. It returns true when the section has neither an explicit +// github-app nor an explicit github-token already configured. +// +// Rules (consistent across all sections): +// - If a section-specific github-app is set → keep it, no fallback needed. +// - If a section-specific github-token is set → keep it, no fallback needed (a token +// already provides the auth; injecting a github-app would silently change precedence). +// - Otherwise → apply the top-level fallback. +func topLevelFallbackNeeded(app *GitHubAppConfig, token string) bool { + return app == nil && token == "" +} + // applyTopLevelGitHubAppFallbacks applies the top-level github-app as a fallback for all // nested github-app token minting operations when no section-specific github-app is configured. -// Precedence: section-specific github-app > top-level github-app > no token minting. +// Precedence: section-specific github-app > section-specific github-token > top-level github-app. +// +// Every section uses topLevelFallbackNeeded to decide whether the fallback is required, +// ensuring consistent behaviour across all token-minting sites. func applyTopLevelGitHubAppFallbacks(data *WorkflowData) { fallback := data.TopLevelGitHubApp if fallback == nil { return } - // Fallback for activation (on.github-app) - // Skip when on.github-token is already explicitly configured — the token takes priority over - // app-based auth at runtime and injecting a github-app fallback would flip that precedence. - if data.ActivationGitHubApp == nil && data.ActivationGitHubToken == "" { + // Fallback for activation (on.github-app / on.github-token) + if topLevelFallbackNeeded(data.ActivationGitHubApp, data.ActivationGitHubToken) { orchestratorWorkflowLog.Print("Applying top-level github-app fallback for activation") data.ActivationGitHubApp = fallback } - // Fallback for safe-outputs - // Also skip when a custom github-token is already configured for safe-outputs. - if data.SafeOutputs != nil && data.SafeOutputs.GitHubApp == nil && data.SafeOutputs.GitHubToken == "" { + // Fallback for safe-outputs (safe-outputs.github-app / safe-outputs.github-token) + if data.SafeOutputs != nil && topLevelFallbackNeeded(data.SafeOutputs.GitHubApp, data.SafeOutputs.GitHubToken) { orchestratorWorkflowLog.Print("Applying top-level github-app fallback for safe-outputs") data.SafeOutputs.GitHubApp = fallback } - // Fallback for checkout configs: apply only when neither github-token nor github-app is set + // Fallback for checkout configs (checkout.github-app / checkout.github-token per entry) for _, cfg := range data.CheckoutConfigs { - if cfg.GitHubApp == nil && cfg.GitHubToken == "" { + if topLevelFallbackNeeded(cfg.GitHubApp, cfg.GitHubToken) { orchestratorWorkflowLog.Print("Applying top-level github-app fallback for checkout") cfg.GitHubApp = fallback } } - // Fallback for tools.github (MCP GitHub token minting). - // Skip when a custom github-token is already configured for the MCP server (token takes priority). - // Also skip when tools.github is explicitly disabled (github: false) — do not re-enable it. + // Fallback for tools.github (tools.github.github-app / tools.github.github-token). + // Also skipped when tools.github is explicitly disabled (github: false) — do not re-enable it. if data.ParsedTools != nil && data.ParsedTools.GitHub != nil && - data.ParsedTools.GitHub.GitHubApp == nil && data.ParsedTools.GitHub.GitHubToken == "" && + topLevelFallbackNeeded(data.ParsedTools.GitHub.GitHubApp, data.ParsedTools.GitHub.GitHubToken) && data.Tools["github"] != false { orchestratorWorkflowLog.Print("Applying top-level github-app fallback for tools.github") data.ParsedTools.GitHub.GitHubApp = fallback @@ -673,8 +685,8 @@ func applyTopLevelGitHubAppFallbacks(data *WorkflowData) { } } - // Fallback for APM dependencies - if data.APMDependencies != nil && data.APMDependencies.GitHubApp == nil { + // Fallback for APM dependencies (dependencies.github-app; no github-token field) + if data.APMDependencies != nil && topLevelFallbackNeeded(data.APMDependencies.GitHubApp, "") { orchestratorWorkflowLog.Print("Applying top-level github-app fallback for dependencies") data.APMDependencies.GitHubApp = fallback }