From 25b6be0f3ad55958d603e829d02a39c306cabe17 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 03:21:03 +0000 Subject: [PATCH 1/5] Initial plan From 83e44a4a12c89d375a726b742156931367757a98 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 03:26:51 +0000 Subject: [PATCH 2/5] Plan pre-activation compiler tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-copilot-auto.lock.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/smoke-copilot-auto.lock.yml b/.github/workflows/smoke-copilot-auto.lock.yml index 9cac699d23d..80f7930c2f1 100644 --- a/.github/workflows/smoke-copilot-auto.lock.yml +++ b/.github/workflows/smoke-copilot-auto.lock.yml @@ -149,6 +149,7 @@ jobs: GH_AW_INFO_FIREWALL_TYPE: "squid" GH_AW_INFO_FRONTMATTER_EMOJI: "🌸" GH_AW_COMPILED_STRICT: "true" + GH_AW_INFO_MODEL_COSTS: '{"providers":{"github-copilot":{"models":{"auto":{"cost":{"input":"8.5e-07","output":"1.55e-06"}}}}}}' GH_AW_INFO_FEATURES: '{"gh-aw-detection":false}' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: From 0199cfae28e293e7c3ee8ad1719147598aa970de Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 03:36:05 +0000 Subject: [PATCH 3/5] Add unit tests for pre-activation compiler job helpers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../compiler_pre_activation_job_test.go | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 pkg/workflow/compiler_pre_activation_job_test.go diff --git a/pkg/workflow/compiler_pre_activation_job_test.go b/pkg/workflow/compiler_pre_activation_job_test.go new file mode 100644 index 00000000000..f8dc4d2a6d9 --- /dev/null +++ b/pkg/workflow/compiler_pre_activation_job_test.go @@ -0,0 +1,131 @@ +//go:build !integration + +package workflow + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuildPreActivationPermissions(t *testing.T) { + t.Run("release mode without optional checks keeps permissions empty", func(t *testing.T) { + c := NewCompiler() + c.SetActionMode(ActionModeRelease) + + steps, permissions := c.buildPreActivationPermissions(&WorkflowData{Name: "wf"}, "./actions/setup") + stepsStr := strings.Join(steps, "\n") + + require.NotEmpty(t, steps) + assert.Contains(t, stepsStr, "Setup Scripts") + assert.NotContains(t, stepsStr, "Checkout actions folder") + assert.Empty(t, permissions) + }) + + t.Run("script mode merges inferred and explicit permissions", func(t *testing.T) { + c := NewCompiler() + c.SetActionMode(ActionModeScript) + + data := &WorkflowData{ + Name: "wf", + RateLimit: &RateLimitConfig{}, + LabelCommandDecentralized: true, + LabelCommandEvents: []string{"pull_request"}, + OnPermissions: NewPermissionsFromMap(map[PermissionScope]PermissionLevel{ + PermissionIssues: PermissionWrite, + }), + } + + steps, permissions := c.buildPreActivationPermissions(data, "./actions/setup") + stepsStr := strings.Join(steps, "\n") + + require.NotEmpty(t, steps) + assert.Contains(t, stepsStr, "Checkout actions folder") + assert.Contains(t, permissions, "actions: read") + assert.Contains(t, permissions, "contents: read") + assert.Contains(t, permissions, "issues: write") + assert.Contains(t, permissions, "pull-requests: read") + }) +} + +func TestApplyPreActivationIfConditionGuards(t *testing.T) { + baseIf := "github.ref == 'refs/heads/main'" + + t.Run("combines label and comment-author guards when eligible", func(t *testing.T) { + c := NewCompiler() + data := &WorkflowData{ + LabelNames: []string{"bug"}, + On: "issue_comment:\n types: [created]\n", + Bots: []string{"dependabot[bot]"}, + } + + result := c.applyPreActivationIfConditionGuards(data, true, baseIf) + + assert.Contains(t, result, "github.event.label == null") + assert.Contains(t, result, "github.event.label.name == 'bug'") + assert.Contains(t, result, "author_association") + assert.Contains(t, result, baseIf) + }) + + t.Run("does not add comment-author guard for expression-based bot list", func(t *testing.T) { + c := NewCompiler() + data := &WorkflowData{ + On: "issue_comment:\n types: [created]\n", + Bots: []string{"${{ vars.ALLOWED_BOT }}"}, + } + + result := c.applyPreActivationIfConditionGuards(data, true, baseIf) + + assert.NotContains(t, result, "author_association") + assert.Equal(t, baseIf, result) + }) + + t.Run("adds skip-author-associations guard", func(t *testing.T) { + c := NewCompiler() + data := &WorkflowData{ + SkipAuthorAssociations: map[string][]string{ + "issue_comment": {"OWNER", "MEMBER"}, + }, + } + + result := c.applyPreActivationIfConditionGuards(data, false, "") + + assert.Contains(t, result, "github.event_name") + assert.Contains(t, result, "github.event.comment.author_association") + }) +} + +func TestBuildPreActivationJob(t *testing.T) { + c := NewCompiler() + c.SetActionMode(ActionModeDev) + + data := &WorkflowData{ + Name: "pre-activation unit test", + If: "github.ref == 'refs/heads/main'", + LabelNames: []string{"bug"}, + On: "issue_comment:\n types: [created]\n", + Bots: []string{"dependabot[bot]"}, + OnNeeds: []string{"prepare", "prepare"}, + OnSteps: []map[string]any{ + { + "id": "gate", + "run": "echo gate", + "name": "gate", + }, + }, + } + + job, err := c.buildPreActivationJob(data, true) + require.NoError(t, err) + require.NotNil(t, job) + + assert.Contains(t, job.If, "github.event.label == null") + assert.Contains(t, job.If, "author_association") + assert.Contains(t, job.If, data.If) + assert.Equal(t, []string{"prepare"}, job.Needs) + assert.Contains(t, job.Outputs, "gate_result") + assert.Contains(t, job.Outputs, "matched_command") + assert.Contains(t, job.Outputs["matched_command"], "''") +} From 1e9aa613f0e2a9cdeeaa43ef5860a670b145276e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 03:54:11 +0000 Subject: [PATCH 4/5] docs(adr): add draft ADR-48506 for dedicated pre-activation compiler unit tests Co-Authored-By: Claude Sonnet 4.6 --- ...-unit-test-file-pre-activation-compiler.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 docs/adr/48506-add-dedicated-unit-test-file-pre-activation-compiler.md diff --git a/docs/adr/48506-add-dedicated-unit-test-file-pre-activation-compiler.md b/docs/adr/48506-add-dedicated-unit-test-file-pre-activation-compiler.md new file mode 100644 index 00000000000..3a5378e7a15 --- /dev/null +++ b/docs/adr/48506-add-dedicated-unit-test-file-pre-activation-compiler.md @@ -0,0 +1,44 @@ +# ADR-48506: Add Dedicated Unit Test File for Pre-Activation Job Compiler + +**Date**: 2026-07-28 +**Status**: Draft +**Deciders**: pelikhan + +--- + +### Context + +`compiler_pre_activation_job.go` is the largest compiler module in `pkg/workflow/` with 40 functions and 956 lines of code. Despite this scale, it had no corresponding `_test.go` file, making it the only large compiler file with zero dedicated unit test coverage. This gap meant that bugs in the core pre-activation job assembly logic — including guard composition, permission merging, and job structure building — could only be caught through broader integration tests or not at all. The module's complexity (particularly `buildPreActivationJob`, `applyPreActivationIfConditionGuards`, and `buildPreActivationPermissions`) made it a high-risk area for undetected regressions. + +### Decision + +We will add a dedicated unit test file `pkg/workflow/compiler_pre_activation_job_test.go` targeting the highest-value helper functions in isolation, using the `!integration` build tag so tests run in the standard unit test suite without requiring integration infrastructure. Coverage focuses on `buildPreActivationJob` (guard composition, needs deduplication, output wiring), `applyPreActivationIfConditionGuards` (label guard + comment-author guard logic, expression-based bot suppression, skip-author-associations clauses), and `buildPreActivationPermissions` (release mode vs. script mode permission merging). + +### Alternatives Considered + +#### Alternative 1: Extend Existing Compiler Test Files + +Add pre-activation coverage to `compiler_test.go` or other existing compiler test files. This was not chosen because those files are already integration-heavy; mixing isolated helper-level assertions into them would blur the unit/integration boundary, make failures harder to attribute, and increase cognitive overhead for future contributors navigating large, multi-purpose test files. + +#### Alternative 2: Rely Solely on Integration Tests + +Skip dedicated unit coverage entirely and depend on the existing integration test suite to catch regressions in this module. This was not chosen because integration tests are slower to run, harder to iterate on locally, and do not isolate failures at the function level — making it significantly harder to diagnose pre-activation logic bugs when they surface. + +### Consequences + +#### Positive +- Isolated, fast unit tests for the three most complex pre-activation helper functions, runnable with `make test-unit` +- Future contributors can verify and modify pre-activation behavior without spinning up integration infrastructure +- Follows the existing test style and `!integration` build tag convention established across `pkg/workflow/` + +#### Negative +- An additional test file to maintain as the pre-activation compiler evolves; helper fixtures must stay in sync with production code structure +- Unit tests at this level cannot catch cross-module integration failures — integration test coverage still required for end-to-end validation + +#### Neutral +- Uses the existing `testify` assert/require framework already standard across `pkg/workflow/` +- The `!integration` build tag is consistent with how other unit tests in this package exclude integration-only scenarios + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From de50c48b6e847853aca8462296867ae7aee119ca Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:06:23 +0000 Subject: [PATCH 5/5] revert: remove unrelated GH_AW_INFO_MODEL_COSTS drift from smoke-copilot-auto.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-copilot-auto.lock.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/smoke-copilot-auto.lock.yml b/.github/workflows/smoke-copilot-auto.lock.yml index 80f7930c2f1..9cac699d23d 100644 --- a/.github/workflows/smoke-copilot-auto.lock.yml +++ b/.github/workflows/smoke-copilot-auto.lock.yml @@ -149,7 +149,6 @@ jobs: GH_AW_INFO_FIREWALL_TYPE: "squid" GH_AW_INFO_FRONTMATTER_EMOJI: "🌸" GH_AW_COMPILED_STRICT: "true" - GH_AW_INFO_MODEL_COSTS: '{"providers":{"github-copilot":{"models":{"auto":{"cost":{"input":"8.5e-07","output":"1.55e-06"}}}}}}' GH_AW_INFO_FEATURES: '{"gh-aw-detection":false}' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: