Skip to content

Format {files} placeholders as inline code in JS template rendering#34011

Merged
pelikhan merged 7 commits into
mainfrom
copilot/update-rendertemplate-files-handling
May 22, 2026
Merged

Format {files} placeholders as inline code in JS template rendering#34011
pelikhan merged 7 commits into
mainfrom
copilot/update-rendertemplate-files-handling

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 22, 2026

{files} values were rendered as raw comma-separated text, which made multi-file output harder to scan. This change adds dedicated {files} rendering so filenames are consistently markdown-formatted and unsafe input is rejected.

  • Template rendering behavior

    • Added special handling for the files expression in actions/setup/js/messages_core.cjs.
    • Treats files as a comma-separated list, trims entries, drops empty segments, and renders each filename as an inline code span.
  • Input safety

    • Rejects any filename containing backticks to prevent malformed markdown/injection in rendered output.
    • Error now includes the offending filename for fast diagnosis.
  • Coverage updates

    • Extended actions/setup/js/messages_core.test.cjs with focused cases for:
      • standard comma-separated file lists,
      • backtick-containing filename rejection,
      • empty files input behavior,
      • file-template rendering path (renderTemplateFromFile).
renderTemplate("Changed files: {files}", {
  files: "a.txt,b/c.md, docs/readme.md ",
});
// => "Changed files: `a.txt`, `b/c.md`, `docs/readme.md`"

Copilot AI and others added 3 commits May 22, 2026 13:32
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Handle files placeholder formatting in JS template renderer Format {files} placeholders as inline code in JS template rendering May 22, 2026
Copilot AI requested a review from pelikhan May 22, 2026 13:42
Comment thread actions/setup/js/messages_core.cjs Outdated
Comment on lines +92 to +95
const invalidFile = files.find(file => file.includes("`"));
if (invalidFile) {
throw new Error(`Invalid {files} value: filename "${invalidFile}" contains backticks`);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot do not throw, replace with redacted. Handle case where file names have been pre wrapped in back ticks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in commit 7a8f39b.

{files} rendering now avoids throwing, supports filenames that are already wrapped in backticks, and redacts unsafe entries as `redacted`.

Copilot AI and others added 3 commits May 22, 2026 13:51
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan May 22, 2026 13:54
@pelikhan pelikhan marked this pull request as ready for review May 22, 2026 13:58
Copilot AI review requested due to automatic review settings May 22, 2026 13:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates JS template rendering so {files} placeholders are rendered as inline-code filenames (improving readability of multi-file output) and updates action pin/lock data for docker/metadata-action@v6.

Changes:

  • Added special {files} handling in actions/setup/js/messages_core.cjs to split comma-separated file lists and wrap each filename in inline code.
  • Added/extended vitest cases around {files} rendering (including empty input and backtick-containing entries) and file-based template rendering.
  • Updated action pin/lock data to include docker/metadata-action@v6 and updated the pinned SHA in the release workflow lock.
Show a summary per file
File Description
actions/setup/js/messages_core.cjs Implements special {files} placeholder rendering logic.
actions/setup/js/messages_core.test.cjs Adds tests covering {files} formatting behavior and render-from-file path.
pkg/workflow/data/action_pins.json Adds docker/metadata-action@v6 pin entry.
pkg/actionpins/data/action_pins.json Adds docker/metadata-action@v6 pin entry.
.github/aw/actions-lock.json Adds docker/metadata-action@v6 pin entry for GitHub Actions locking.
.github/workflows/release.lock.yml Updates the pinned docker/metadata-action SHA used in the release workflow.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

actions/setup/js/messages_core.cjs:101

  • PR description says backtick-containing filenames are rejected and the error includes the offending filename, but the implementation/test suite currently silently replaces those entries with redacted. Please align either the behavior (throw with a helpful message) or the PR description/tests so consumers know whether this is a hard failure or a soft-sanitize.
          if (!normalized || normalized.includes("`")) {
            normalized = "redacted";
          }
          return `\`${normalized}\``;
  • Files reviewed: 6/6 changed files
  • Comments generated: 2

Comment thread actions/setup/js/messages_core.cjs Outdated
Comment on lines +87 to +91
if (key === "files") {
const files = String(value)
.split(",")
.map(file => file.trim())
.filter(Boolean);
Comment thread actions/setup/js/messages_core.test.cjs Outdated
Comment on lines +86 to +90
it("should specially format files placeholder as backticked filenames", async () => {
const { renderTemplate } = await import("./messages_core.cjs?" + Date.now());
const result = renderTemplate("Changed files: {files}", { files: "a.txt,b/c.md, docs/readme.md " });
expect(result).toBe("Changed files: `a.txt`, `b/c.md`, `docs/readme.md`");
});
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot move logic into a helper "renderFilesList" and apply to call sites instead

@github-actions
Copy link
Copy Markdown
Contributor

@copilot review all comments and address the unresolved review feedback.
Please extract renderFilesList and update the call sites so this blocker can move forward.

Generated by 👨‍🍳 PR Sous Chef · ● 2.7M ·

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 22, 2026

@copilot move logic into a helper "renderFilesList" and apply to call sites instead

Updated in commit d79ea31.

@pelikhan pelikhan merged commit 579a75f into main May 22, 2026
@pelikhan pelikhan deleted the copilot/update-rendertemplate-files-handling branch May 22, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants