Add focused unit coverage for pre-activation job compiler helpers

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

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"], "''")
+}