From 26786717e247a2d5c4255967bf711319b7c6c43c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:54:24 +0000 Subject: [PATCH 1/3] Initial plan From dcf590a8a0e0812e7c7e6aa8e9ac4dea4bc06a60 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:16:14 +0000 Subject: [PATCH 2/3] migrate deprecated needs.activation.outputs references to steps.sanitized.outputs - Fix detectTextOutputUsage() to also detect deprecated needs.activation.outputs.{text,title,body} so workflows not yet migrated still compile correctly when event context alone is insufficient - Add TestDetectTextOutputUsage cases for deprecated form - Add TestExpressionExtractor_DeprecatedActivationOutputWarning to verify the deprecation warning is emitted (and the correct transformed mapping produced) - Update TestExpressionExtractor_GenerateEnvVarName to use the modern steps.sanitized.outputs.text form (matches what the pipeline actually passes) - Update TestExpressionExtractor_NoCollisions to use steps.sanitized.outputs.text - Update runtime_import.test.cjs tests for wrapExpressionsInTemplateConditionals and extractAndReplacePlaceholders to use modern form (compiled templates never contain the deprecated form at runtime) - Add modern-form fuzz seeds alongside deprecated ones and clarify the purpose of the deprecated seeds in both FuzzRenderExpressions and FuzzExtractExpressions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/runtime_import.test.cjs | 10 +-- pkg/workflow/compiler_orchestrator_tools.go | 18 ++++- pkg/workflow/compute_text_lazy_test.go | 17 +++++ .../expression_extraction_fuzz_test.go | 4 +- pkg/workflow/expression_extraction_test.go | 70 +++++++++++++++++-- 5 files changed, 106 insertions(+), 13 deletions(-) diff --git a/actions/setup/js/runtime_import.test.cjs b/actions/setup/js/runtime_import.test.cjs index 831a5a5947f..3c8604a741e 100644 --- a/actions/setup/js/runtime_import.test.cjs +++ b/actions/setup/js/runtime_import.test.cjs @@ -1943,8 +1943,8 @@ describe("runtime_import", () => { it("should produce {{#if }} (falsy) for github.actor when context is unavailable", () => { expect(wrapExpressionsInTemplateConditionals("{{#if github.actor}}body{{/if}}")).toBe("{{#if }}body{{/if}}"); }); - it("should produce {{#if }} (falsy) for needs.activation.outputs.text when env var is absent", () => { - expect(wrapExpressionsInTemplateConditionals("{{#if needs.activation.outputs.text}}body{{/if}}")).toBe("{{#if }}body{{/if}}"); + it("should produce {{#if }} (falsy) for steps.sanitized.outputs.text when env var is absent", () => { + expect(wrapExpressionsInTemplateConditionals("{{#if steps.sanitized.outputs.text}}body{{/if}}")).toBe("{{#if }}body{{/if}}"); }); it("should produce {{#if }} (falsy) for steps.foo.outputs.bar when env var is absent", () => { expect(wrapExpressionsInTemplateConditionals("{{#if steps.foo.outputs.bar}}body{{/if}}")).toBe("{{#if }}body{{/if}}"); @@ -2045,9 +2045,9 @@ describe("runtime_import", () => { const input = "{{#if ${{ github.event.issue.number }} }}body{{/if}}"; expect(extractAndReplacePlaceholders(input)).toBe("{{#if __GH_AW_GITHUB_EVENT_ISSUE_NUMBER__ }}body{{/if}}"); }); - it("should convert needs.activation.outputs.text", () => { - const input = "{{#if ${{ needs.activation.outputs.text }} }}body{{/if}}"; - expect(extractAndReplacePlaceholders(input)).toBe("{{#if __GH_AW_NEEDS_ACTIVATION_OUTPUTS_TEXT__ }}body{{/if}}"); + it("should convert steps.sanitized.outputs.text", () => { + const input = "{{#if ${{ steps.sanitized.outputs.text }} }}body{{/if}}"; + expect(extractAndReplacePlaceholders(input)).toBe("{{#if __GH_AW_STEPS_SANITIZED_OUTPUTS_TEXT__ }}body{{/if}}"); }); it("should leave content without wrapped expressions unchanged", () => { const input = "{{#if __GH_AW_GITHUB_ACTOR__ }}body{{/if}}"; diff --git a/pkg/workflow/compiler_orchestrator_tools.go b/pkg/workflow/compiler_orchestrator_tools.go index 59724e13397..4e9a1fc71ce 100644 --- a/pkg/workflow/compiler_orchestrator_tools.go +++ b/pkg/workflow/compiler_orchestrator_tools.go @@ -428,13 +428,27 @@ func (c *Compiler) tryParseFrontmatterConfig(frontmatter map[string]any) *Frontm } // detectTextOutputUsage checks if the markdown content uses ${{ steps.sanitized.outputs.text }}, -// ${{ steps.sanitized.outputs.title }}, or ${{ steps.sanitized.outputs.body }} +// ${{ steps.sanitized.outputs.title }}, or ${{ steps.sanitized.outputs.body }}. +// It also recognises the deprecated ${{ needs.activation.outputs.{text,title,body} }} forms so +// that workflows that have not yet been migrated still compile correctly. func (c *Compiler) detectTextOutputUsage(markdownContent string) bool { - // Check for any of the text-related output expressions + // Check for any of the text-related output expressions (modern form) hasTextUsage := strings.Contains(markdownContent, "${{ steps.sanitized.outputs.text }}") hasTitleUsage := strings.Contains(markdownContent, "${{ steps.sanitized.outputs.title }}") hasBodyUsage := strings.Contains(markdownContent, "${{ steps.sanitized.outputs.body }}") + // Also recognise the deprecated needs.activation.outputs.* forms so that workflows + // using the old syntax still get the sanitized step included during compilation. + if !hasTextUsage { + hasTextUsage = strings.Contains(markdownContent, "${{ needs.activation.outputs.text }}") + } + if !hasTitleUsage { + hasTitleUsage = strings.Contains(markdownContent, "${{ needs.activation.outputs.title }}") + } + if !hasBodyUsage { + hasBodyUsage = strings.Contains(markdownContent, "${{ needs.activation.outputs.body }}") + } + hasUsage := hasTextUsage || hasTitleUsage || hasBodyUsage detectionLog.Printf("Detected usage of sanitized outputs - text: %v, title: %v, body: %v, any: %v", hasTextUsage, hasTitleUsage, hasBodyUsage, hasUsage) diff --git a/pkg/workflow/compute_text_lazy_test.go b/pkg/workflow/compute_text_lazy_test.go index f44e39b77ad..45676dbd619 100644 --- a/pkg/workflow/compute_text_lazy_test.go +++ b/pkg/workflow/compute_text_lazy_test.go @@ -191,6 +191,23 @@ func TestDetectTextOutputUsage(t *testing.T) { content: "Body: \"${{ steps.sanitized.outputs.body }}\"", expectedUsage: true, }, + // Deprecated needs.activation.outputs.* forms must also be detected so that + // workflows not yet migrated still compile correctly. + { + name: "with_deprecated_text_usage", + content: "Content: \"${{ needs.activation.outputs.text }}\"", + expectedUsage: true, + }, + { + name: "with_deprecated_title_usage", + content: "Title: \"${{ needs.activation.outputs.title }}\"", + expectedUsage: true, + }, + { + name: "with_deprecated_body_usage", + content: "Body: \"${{ needs.activation.outputs.body }}\"", + expectedUsage: true, + }, } for _, tt := range tests { diff --git a/pkg/workflow/expression_extraction_fuzz_test.go b/pkg/workflow/expression_extraction_fuzz_test.go index 06e00c7feec..41ffe9ef96d 100644 --- a/pkg/workflow/expression_extraction_fuzz_test.go +++ b/pkg/workflow/expression_extraction_fuzz_test.go @@ -127,7 +127,7 @@ func FuzzRenderExpressions(f *testing.F) { f.Add("Repo: ${{ github.repository }}, Actor: ${{ github.actor }}") f.Add("${{ steps.a.outputs.x || inputs.y }}, ${{ inputs.z }}") - // Deprecated activation output syntax + // Deprecated activation output syntax (kept for backwards-compatibility transformer coverage) f.Add("Content: ${{ needs.activation.outputs.text }}") f.Add("Fallback: ${{ needs.activation.outputs.text || 'default' }}") @@ -245,7 +245,7 @@ func FuzzExtractExpressions(f *testing.F) { f.Add("Repo: ${{ github.repository }}, Actor: ${{ github.actor }}") f.Add("${{ steps.a.outputs.x || inputs.y }}, ${{ inputs.z }}") - // Deprecated activation output syntax + // Deprecated activation output syntax (kept for backwards-compatibility transformer coverage) f.Add("Content: ${{ needs.activation.outputs.text }}") f.Add("Fallback: ${{ needs.activation.outputs.text || 'default' }}") diff --git a/pkg/workflow/expression_extraction_test.go b/pkg/workflow/expression_extraction_test.go index ade8d34a4bb..0ae2ddcf805 100644 --- a/pkg/workflow/expression_extraction_test.go +++ b/pkg/workflow/expression_extraction_test.go @@ -243,9 +243,9 @@ func TestExpressionExtractor_GenerateEnvVarName(t *testing.T) { wantName: "GH_AW_GITHUB_EVENT_ISSUE_NUMBER", }, { - name: "needs output", - content: "needs.activation.outputs.text", - wantName: "GH_AW_NEEDS_ACTIVATION_OUTPUTS_TEXT", + name: "sanitized step outputs", + content: "steps.sanitized.outputs.text", + wantName: "GH_AW_STEPS_SANITIZED_OUTPUTS_TEXT", }, { name: "complex expression with operators", @@ -345,7 +345,7 @@ func TestExpressionExtractor_NoCollisions(t *testing.T) { "github.actor", "github.run_id", "github.event.issue.number", - "needs.activation.outputs.text", + "steps.sanitized.outputs.text", } extractor := NewExpressionExtractor() @@ -511,6 +511,68 @@ Other: ${{ needs.activation.outputs.comment_id }} } } +// TestExpressionExtractor_DeprecatedActivationOutputWarning verifies that extracting +// a deprecated needs.activation.outputs.* expression emits a deprecation warning to +// stderr while still producing the correct (transformed) mapping. +func TestExpressionExtractor_DeprecatedActivationOutputWarning(t *testing.T) { + tests := []struct { + name string + markdown string + deprecated string + modern string + }{ + { + name: "text output emits warning", + markdown: "Content: ${{ needs.activation.outputs.text }}", + deprecated: "needs.activation.outputs.text", + modern: "steps.sanitized.outputs.text", + }, + { + name: "title output emits warning", + markdown: "Title: ${{ needs.activation.outputs.title }}", + deprecated: "needs.activation.outputs.title", + modern: "steps.sanitized.outputs.title", + }, + { + name: "body output emits warning", + markdown: "Body: ${{ needs.activation.outputs.body }}", + deprecated: "needs.activation.outputs.body", + modern: "steps.sanitized.outputs.body", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + extractor := NewExpressionExtractor() + + stderr := captureStderr(func() { + mappings, err := extractor.ExtractExpressions(tt.markdown) + if err != nil { + t.Fatalf("ExtractExpressions() unexpected error: %v", err) + } + // Verify the mapping was produced with the modern expression. + found := false + for _, m := range mappings { + if m.Content == tt.modern { + found = true + break + } + } + if !found { + t.Errorf("expected mapping for %q but not found", tt.modern) + } + }) + + if !strings.Contains(stderr, tt.deprecated) { + t.Errorf("expected deprecation warning mentioning %q in stderr, got: %q", tt.deprecated, stderr) + } + if !strings.Contains(stderr, tt.modern) { + t.Errorf("expected deprecation warning mentioning %q in stderr, got: %q", tt.modern, stderr) + } + }) + } +} + func TestApplyWorkflowDispatchFallbacks(t *testing.T) { tests := []struct { name string From 5687e0e10a68d1ff9f02605c9400995376ff7d36 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:37:09 +0000 Subject: [PATCH 3/3] docs(adr): draft ADR-47264 for backwards-compat deprecated activation outputs detection --- ...deprecated-activation-outputs-detection.md | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 docs/adr/47264-backwards-compat-deprecated-activation-outputs-detection.md diff --git a/docs/adr/47264-backwards-compat-deprecated-activation-outputs-detection.md b/docs/adr/47264-backwards-compat-deprecated-activation-outputs-detection.md new file mode 100644 index 00000000000..f31193c6197 --- /dev/null +++ b/docs/adr/47264-backwards-compat-deprecated-activation-outputs-detection.md @@ -0,0 +1,52 @@ +# ADR-47264: Extend detectTextOutputUsage to Recognise Deprecated needs.activation.outputs Forms + +**Date**: 2026-07-22 +**Status**: Draft +**Deciders**: Unknown (Copilot SWE Agent, pelikhan) + +--- + +### Context + +The workflow compiler automatically rewrites deprecated `${{ needs.activation.outputs.{text,title,body} }}` expressions to their modern `${{ steps.sanitized.outputs.{text,title,body} }}` equivalents. However, the `detectTextOutputUsage` function — which decides whether the sanitized step must be injected into the compiled workflow — only scanned for the modern form. + +Workflows that had not yet been migrated to the modern syntax would silently compile without the sanitized step. This caused runtime failures particularly for `workflow_dispatch`-only triggers, where no content context is provided and the missing step caused undefined outputs. + +The fix must not break any already-migrated workflows, and must preserve the existing deprecation-warning pathway so users are still notified to migrate. + +### Decision + +We will extend `detectTextOutputUsage` to also check for `${{ needs.activation.outputs.text }}`, `${{ needs.activation.outputs.title }}`, and `${{ needs.activation.outputs.body }}` alongside the already-supported modern forms, using short-circuit `if !hasXUsage` guards so that a single `strings.Contains` hit is enough to set the flag. The auto-rewrite that transforms deprecated to modern syntax at the expression level is left untouched. + +### Alternatives Considered + +#### Alternative 1: Reject deprecated syntax with a compile-time error + +Return a compile error (or emit a fatal diagnostic) whenever a deprecated `needs.activation.outputs.*` expression is found, requiring authors to migrate before the workflow compiles. + +Rejected because it is a breaking change for all existing unmigrated workflows. The project's stated intent is a gradual migration; forcing an immediate hard break contradicts that policy and would block legitimate workflows from compiling at all. + +#### Alternative 2: Emit a lint warning and rely on authors to migrate before runtime + +Log a prominent warning, leave `detectTextOutputUsage` as-is (only scanning modern forms), and document that deprecated syntax may produce incorrect compiled output until migrated. + +Rejected because it preserves the silent-failure bug that caused the issue in the first place. Users would see a warning but would still receive a broken compiled workflow, making this behaviour invisible until a runtime failure occurred. + +### Consequences + +#### Positive +- Unmigrated workflows now compile correctly and include the sanitized step, eliminating the silent runtime failure. +- The fix is non-breaking: already-migrated workflows are unaffected because the `if !hasXUsage` guards skip the deprecated-form check once the modern form is found. +- Deprecation warnings (emitted to stderr by `ExpressionExtractor`) are preserved, continuing to guide authors toward the modern syntax. + +#### Negative +- The deprecated `needs.activation.outputs.*` strings are now referenced in two places in the compiler: the auto-rewrite in `ExpressionExtractor` and the new detection guards in `detectTextOutputUsage`. This increases the surface area that must be updated when deprecated syntax support is eventually removed. +- Retaining dual-path detection extends the effective deprecation window, since unmigrated workflows no longer fail visibly and authors have less urgency to migrate. + +#### Neutral +- Three new test cases were added to `TestDetectTextOutputUsage` and a new `TestExpressionExtractor_DeprecatedActivationOutputWarning` suite was introduced, increasing test coverage of the backwards-compatibility layer. +- Existing tests that used the deprecated form in `generateEnvVarName` and `NoCollisions` scenarios were updated to the modern form, reflecting that deprecated syntax is transformed before it reaches those code paths in production. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*