Skip to content

fix: skip boolean/null literal wrapping in runtime imports #25831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Apr 11, 2026
+19 −16
Merged

fix: skip boolean/null literal wrapping in runtime imports #25831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8284fb5
Initial plan
Copilot Apr 11, 2026
1b179a0
fix: skip boolean/null literal wrapping in runtime imports (#25800)
Copilot Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions actions/setup/js/runtime_import.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,20 +650,20 @@ function wrapExpressionsInTemplateConditionals(content) {
return match;
}

// Boolean/null literals are self-evaluating — the template renderer's isTruthy()
// handles them directly. Wrapping them would create __GH_AW_TRUE__/__GH_AW_FALSE__/__GH_AW_NULL__
// placeholders that cannot be resolved at runtime (no corresponding env var is set),
// causing the placeholder validator to flag them as unsubstituted.
if (trimmed === "true" || trimmed === "false" || trimmed === "null") {
return match;
}
Comment on lines +653 to +659
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The early-return covers bare {{#if true}}/false/null, but {{#if ${{ true }} }} (already-wrapped input) will still be converted by extractAndReplacePlaceholders() into {{#if __GH_AW_TRUE__ }} and then fail placeholder validation because there’s no substitution mapping at runtime. Consider also special-casing true/false/null in the extraction step (or unwrapping them back to literals) so wrapped literals don’t produce __GH_AW_TRUE__/__GH_AW_FALSE__/__GH_AW_NULL__ placeholders.

Copilot uses AI. Check for mistakes.

// Only wrap expressions that look like GitHub Actions expressions
// GitHub Actions expressions typically start with a letter and contain dots
// (e.g., github.actor, github.event.issue.number) or specific keywords (true, false, null).
// (e.g., github.actor, github.event.issue.number).
// Expressions starting with non-alphabetic characters (e.g., "...") are NOT GitHub expressions.
const looksLikeGitHubExpr =
(/^[a-zA-Z]/.test(trimmed) && trimmed.includes(".")) ||
trimmed === "true" ||
trimmed === "false" ||
trimmed === "null" ||
trimmed.startsWith("github.") ||
trimmed.startsWith("needs.") ||
trimmed.startsWith("steps.") ||
trimmed.startsWith("env.") ||
trimmed.startsWith("inputs.");
(/^[a-zA-Z]/.test(trimmed) && trimmed.includes(".")) || trimmed.startsWith("github.") || trimmed.startsWith("needs.") || trimmed.startsWith("steps.") || trimmed.startsWith("env.") || trimmed.startsWith("inputs.");

if (!looksLikeGitHubExpr) {
// Not a GitHub Actions expression, leave as-is
Expand Down
15 changes: 9 additions & 6 deletions actions/setup/js/runtime_import.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1498,14 +1498,17 @@ describe("runtime_import", () => {
it("should wrap {{#if steps.foo.outputs.bar}}", () => {
expect(wrapExpressionsInTemplateConditionals("{{#if steps.foo.outputs.bar}}body{{/if}}")).toBe("{{#if ${{ steps.foo.outputs.bar }} }}body{{/if}}");
});
it("should wrap {{#if true}}", () => {
expect(wrapExpressionsInTemplateConditionals("{{#if true}}body{{/if}}")).toBe("{{#if ${{ true }} }}body{{/if}}");
});

describe("boolean/null literals — must be left unchanged for direct isTruthy() evaluation", () => {
it("should leave {{#if true}} unchanged", () => {
expect(wrapExpressionsInTemplateConditionals("{{#if true}}body{{/if}}")).toBe("{{#if true}}body{{/if}}");
});
it("should wrap {{#if false}}", () => {
expect(wrapExpressionsInTemplateConditionals("{{#if false}}body{{/if}}")).toBe("{{#if ${{ false }} }}body{{/if}}");
it("should leave {{#if false}} unchanged", () => {
expect(wrapExpressionsInTemplateConditionals("{{#if false}}body{{/if}}")).toBe("{{#if false}}body{{/if}}");
});
it("should wrap {{#if null}}", () => {
expect(wrapExpressionsInTemplateConditionals("{{#if null}}body{{/if}}")).toBe("{{#if ${{ null }} }}body{{/if}}");
it("should leave {{#if null}} unchanged", () => {
expect(wrapExpressionsInTemplateConditionals("{{#if null}}body{{/if}}")).toBe("{{#if null}}body{{/if}}");
});
});

Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This updates unit expectations for wrapExpressionsInTemplateConditionals(), but there isn’t an end-to-end assertion that processRuntimeImport() leaves {{#if true}}/false/null unchanged (and does not introduce any __GH_AW_...__ placeholders) after the full wrap→extract pipeline. Adding an integration test alongside the existing processRuntimeImport — expression wrapping integration cases would help prevent regressions.

Suggested change
describe("boolean/null literals — full wrap→extract pipeline must remain unchanged", () => {
const getExtractedContent = (extracted) => {
if (typeof extracted === "string") {
return extracted;
}
return extracted?.content ?? extracted?.processedContent ?? extracted?.template ?? extracted?.result;
};
it.each([
"{{#if true}}body{{/if}}",
"{{#if false}}body{{/if}}",
"{{#if null}}body{{/if}}",
])("should preserve %s and avoid placeholder insertion", (input) => {
const wrapped = wrapExpressionsInTemplateConditionals(input);
const extracted = extractAndReplacePlaceholders(wrapped);
const output = getExtractedContent(extracted);
expect(output).toBe(input);
expect(output).not.toContain("__GH_AW_");
expect(JSON.stringify(extracted)).not.toContain("__GH_AW_");
});
});

Copilot uses AI. Check for mistakes.
Expand Down
Loading