Conversation
- Import isTruthy from is_truthy.cjs instead of duplicating it locally
- Add require('./shim.cjs') to ensure core is always available
- Remove all 'if (typeof core !== "undefined")' guards (unnecessary with shim)
- Simplify main() by removing verbose ASCII separators and redundant logging
- Remove unused ERR_VALIDATION import
- Update test to import isTruthy from is_truthy.cjs and remove duplicate isTruthy tests (already covered by is_truthy.test.cjs)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors render_template.cjs to rely on the shared isTruthy helper and a common shim.cjs-based globals setup, reducing duplicated logic and noisy logging while keeping the rendering behavior the same.
Changes:
- Replaced the local
isTruthyimplementation inrender_template.cjswith an import fromis_truthy.cjsand addedshim.cjsinitialization. - Simplified
render_template.cjslogging by removing verbose/debug-only output andtypeof coreguards. - Updated
render_template.test.cjsto remove the inlinedisTruthyextraction/tests and use the shared helper.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| actions/setup/js/render_template.cjs | Imports shared isTruthy, initializes core via shim, and streamlines logging in template rendering + main flow. |
| actions/setup/js/render_template.test.cjs | Removes duplicate isTruthy testing/extraction and aligns the test with the shared helper usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { isTruthy } = require("./is_truthy.cjs"), | ||
| renderTemplateScript = fs.readFileSync(path.join(__dirname, "render_template.cjs"), "utf8"), | ||
| renderMarkdownTemplateMatch = renderTemplateScript.match(/function renderMarkdownTemplate\(markdown\)\s*{[\s\S]*?return result;[\s\S]*?}/); | ||
| if (!isTruthyMatch || !renderMarkdownTemplateMatch) throw new Error("Could not extract functions from render_template.cjs"); | ||
| const isTruthy = eval(`(${isTruthyMatch[0]})`), | ||
| renderMarkdownTemplate = eval(`(${renderMarkdownTemplateMatch[0]})`); | ||
| (describe("isTruthy", () => { | ||
| (it("should return false for empty string", () => { | ||
| expect(isTruthy("")).toBe(!1); | ||
| if (!renderMarkdownTemplateMatch) throw new Error("Could not extract renderMarkdownTemplate function from render_template.cjs"); | ||
| const renderMarkdownTemplate = eval(`(${renderMarkdownTemplateMatch[0]})`); |
There was a problem hiding this comment.
The test still reads render_template.cjs as text and evals the extracted renderMarkdownTemplate, which means it does not exercise the real module loading path (including the new require("./is_truthy.cjs") and require("./shim.cjs")). A broken import path or runtime require error in render_template.cjs would not be caught by this test. Consider requiring ./render_template.cjs directly and using the exported renderMarkdownTemplate so the test validates the actual runtime wiring.
See below for a potential fix:
{ renderMarkdownTemplate } = require("./render_template.cjs");
This PR simplifies
render_template.cjsto improve clarity and consistency while preserving all functionality.Files Simplified
actions/setup/js/render_template.cjs— eliminated duplicateisTruthy, removed verbose guards, simplified loggingactions/setup/js/render_template.test.cjs— aligned with new import pattern, removed duplicateisTruthytestsImprovements Made
Eliminated Duplicate
isTruthyrender_template.cjsdefined its own copy ofisTruthy(with extra debug logging) whileis_truthy.cjsalready exists as a shared module. Bothinterpolate_prompt.cjsandfuzz_template_substitution_harness.cjsalready import fromis_truthy.cjs. This fix closes the DRY violation.Before:
After:
Removed Unnecessary
typeof coreGuardsAdded
require("./shim.cjs")(consistent withsubstitute_placeholders.cjs) and removed ~15if (typeof core !== "undefined")guards. The shim ensurescoreis always available, so these guards were defensive noise.Simplified
main()LoggingRemoved ASCII art separators (
========================================), per-block body previews, first/last 200 character dumps, and error type logging — keeping only the actionable diagnostic messages.Removed Unused Import
ERR_VALIDATIONwas imported but never used.Changes Based On
Recent changes from commit
3be658504—fix: \{\\{\#if ...}} in runtime-imported markdown incorrectly becomes an unresolvable placeholder (#22170)Testing
npx vitest run actions/setup/js/render_template.test.cjs)make lint-cjs)make fmt-cjs)Review Focus
Please verify:
isTruthyfromis_truthy.cjshas identical behavior to the removed local copy (it does — same logic, just without the extra debug logging).cjsfiles likesubstitute_placeholders.cjsNote
**🔒 Integrity filter blocked 2 items**
The following items were blocked because they don't meet the GitHub integrity level.
search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".{{#if ...}}in runtime-imported markdown incorrectly becomes an unresolvable placeholder #22170pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".To allow these resources, lower
min-integrityin your GitHub frontmatter: