Accept runtime ${{ ... }} expressions in safe-outputs samples

[dsyme]

Accept runtime ${{ ... }} expressions in safe-outputs samples

PR #37537 · pkg/workflow · patch

---

Problem

safe-outputs.*.samples values are validated at compile-time against the MCP tool's inputSchema. This caused compilation to reject any sample value containing a GitHub Actions runtime expression such as ${{ github.event.inputs.issue_number }} — even though the expression would resolve to a valid type at runtime — making it impossible to wire workflow-dispatch inputs into samples.

---

Approach

During compile-time schema validation, deep-copy each sample map and replace every string value that contains a ${{ ... }} expression with the sentinel placeholder aw_sample (which satisfies common JSON-schema constraints: string, minLength, pattern). Validate the substituted copy only; emit the original sample — with the live expression intact — verbatim into the GH_AW_SAMPLES block of the generated lock file. GitHub Actions then performs its own substitution at runtime.

The original sample map is never mutated.

---

Files changed

File	Change	Impact	Summary
pkg/workflow/samples_validation.go	modified	⭐ high	Introduces substituteRuntimeExpressionsForValidation (compiled regex + aw_sample sentinel) and wires it into validateSamplesForTool so ${{ ... }} expressions are swapped for validation only, leaving the original expression untouched for lock-file emission.
pkg/workflow/samples_validation_test.go	modified	medium	Adds four unit tests: top-level expression fields pass without error; nested arrays/objects are handled; genuine missing-required-field errors still surface; literal values are left untouched by the substitution helper.
pkg/workflow/samples_replay_test.go	modified	medium	Adds TestUseSamplesPreservesRuntimeExpressionsInLockFile — a regression test confirming a ${{ ... }} expression survives compilation and is emitted verbatim into GH_AW_SAMPLES in the generated lock file.
.changeset/patch-samples-runtime-expressions.md	added	low	Patch-level changeset entry documenting the new bypass behaviour for release notes.

---

Key design constraint

Validation uses a substituted copy; the lock file always receives the original sample. This separation ensures:

• Compile-time schema checks still catch real type/required-field errors
• Runtime expressions are never coerced, truncated, or lost in the emitted artifact

---

Breaking changes

None.

---

Commits

SHA	Message
c3b29b3db	Accept runtime ${{ ... }} expressions in safe-outputs samples (#37532)
5a6a39a6b	Potential fix for pull request finding

Generated by PR Description Updater for issue #37537 · 122.1 AIC · ⌖ 14.5 AIC · ⊞ 19.5K · ◷

[Copilot]

Pull request overview

This PR updates safe-outputs sample validation so workflow authors can use GitHub Actions runtime expressions (${{ ... }}) inside safe-outputs.*.samples without failing compile-time JSON-Schema validation, while still preserving the original expressions verbatim in the compiled lock file for runtime substitution.

Changes:

• Substitutes ${{ ... }}-containing string values with a sentinel during schema validation (without mutating the original samples).
• Adds unit tests for top-level and nested substitutions, non-expression validation failures, and helper non-mutating behavior.
• Adds an integration test ensuring runtime expressions survive into the lock file’s GH_AW_SAMPLES payload.

Show a summary per file

File	Description
pkg/workflow/samples_validation.go	Adds runtime-expression substitution during samples schema validation.
pkg/workflow/samples_validation_test.go	Adds unit tests covering runtime-expression substitution and non-mutation guarantees.
pkg/workflow/samples_replay_test.go	Adds integration coverage ensuring expressions are preserved into the lock file env payload.
.changeset/patch-samples-runtime-expressions.md	Documents the patch-level behavioral change in release notes.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 4/4 changed files
• Comments generated: 2

[github-actions]

✅ smoke-ci: safeoutputs CLI comment + comment-memory run (27091718262)

Generated by 🧪 Smoke CI for issue #37537 · ◷

[github-actions]

❌ Design Decision Gate 🏗️ failed during design decision gate check.

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

⚠️ PR Code Quality Reviewer failed during code quality review.

[github-actions]

❌ Test Quality Sentinel failed during test quality analysis.

[github-actions]

Skills-Based Review 🧠

Applied /diagnose and /tdd — the fix cleanly solves the stated item_number case in #37532; three non-blocking observations below.

📋 Key Themes & Highlights

Key Themes

• Boolean field gap: The "aw_sample" sentinel is always a string, so draft: ${{ ... }} and other type: boolean schema fields still fail compile-time validation. This is undocumented in the PR description or the placeholder comment.
• Dead code in fallback: The !ok branch after the type assertion in validateSamplesForTool is unreachable — substituteRuntimeExpressionsForValidation always returns map[string]any when its input is map[string]any.
• Missing boundary test: No test covers boolean-typed fields with runtime expressions, leaving the limitation invisible to future contributors.

Positive Highlights

• ✅ Clean deep-copy approach — original sample is never mutated, which is exactly right for the lock-file contract
• ✅ Solid 4-test suite: bypass, nested structures, non-expression errors still surface, and helper purity
• ✅ Integration test TestUseSamplesPreservesRuntimeExpressionsInLockFile closes the loop end-to-end
• ✅ Non-greedy .*? in the regex is correct; the non-mutation invariant is explicitly verified

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 1.1M · 321.7 AIC · ⌖ 13.8 AIC

[github-actions]

[/diagnose] The "aw_sample" sentinel is always a string, so this approach only works for fields where the schema accepts a string type. Fields like create_pull_request.draft, update_pull_request.draft, update_pull_request.update_branch, and update_project.create_if_missing have type: boolean — substituting them with the string sentinel still fails schema validation. The comment claims the placeholder satisfies "every pattern currently declared" but boolean fields have no pattern, only a strict type requirement.

💡 Example that would still fail

A user writing:

samples:
  - title: "My PR"
    body: "Body"
    branch: "feature"
    draft: ${{ github.event.inputs.is_draft }}

After YAML parsing draft is the string "${{ github.event.inputs.is_draft }}"; after substitution it becomes "aw_sample"; schema validation still fails because "aw_sample" is not a boolean.

To fix, the sentinel logic should inspect the schema type for the field being validated and either skip boolean fields entirely, or use a type-appropriate placeholder per-field. Alternatively, update the comment to document the boolean limitation explicitly so users know this is unsupported.

[github-actions]

[/diagnose] Dead code: substituteRuntimeExpressionsForValidation(stripped) is called with a map[string]any, so the case map[string]any branch always returns a new map[string]any. The type assertion .(map[string]any) can never produce ok == false — the !ok fallback branch is unreachable.

💡 Simplification

Replace the two-step assertion+fallback with a direct call:

stripped := stripSidecarFields(sample, sidecars)
substituted := substituteRuntimeExpressionsForValidation(stripped).(map[string]any)

This removes the dead branch and makes the invariant explicit. If substituteRuntimeExpressionsForValidation is ever refactored to handle non-map inputs at this call site, the compiler will catch the broken assertion rather than silently falling back to unsubstituted data.

[github-actions]

[/tdd] The test suite covers string/number fields with runtime expressions but has no test for type: boolean schema fields (e.g., draft: ${{ github.event.inputs.is_draft }} for create_pull_request). Adding one — even to assert the current failure — prevents this limitation from being silently broken as the schema evolves.

💡 Suggested test

// TestValidateSafeOutputsSamples_BooleanFieldRuntimeExpression documents
// that boolean-typed fields (e.g. draft) do NOT yet support runtime
// expressions, because the string sentinel "aw_sample" fails type: boolean.
// Update this test when boolean field support is added.
func TestValidateSafeOutputsSamples_BooleanFieldRuntimeExpression(t *testing.T) {
    cfg := &SafeOutputsConfig{
        CreatePullRequests: &CreatePullRequestsConfig{
            BaseSafeOutputConfig: BaseSafeOutputConfig{
                Samples: []map[string]any{
                    {
                        "title":  "My PR",
                        "body":   "Body",
                        "branch": "feature",
                        "draft":  "${{ github.event.inputs.is_draft }}",
                    },
                },
            },
        },
    }
    // TODO: this currently fails because aw_sample is not a boolean.
    // Once boolean expression bypass is implemented, flip to check err == nil.
    if err := validateSafeOutputsSamples(cfg); err == nil {
        t.Log("boolean runtime expression bypass now works — update this test")
    }
}