From f5f6d7d83803c9b6880fd50d7687314fe1074ab3 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Tue, 14 Apr 2026 11:35:23 +1000 Subject: [PATCH] cleanup: remove 7 dead CJS scripts and their tests Remove unused JavaScript files that have zero references from production code (not in HANDLER_MAP, not required by other .cjs files, not referenced in Go code or compiled workflows): Dead scripts removed: - add_copilot_reviewer.cjs (+ test) - demo_enhanced_errors.cjs - expiration_helpers.cjs - markdown_transformer.cjs (+ test) - safe_output_topological_sort.cjs (+ test) - safe_outputs_mcp_client.cjs (+ test) - update_runner.cjs (+ test) Total: 7 source files + 5 test files = 12 files removed. --- actions/setup/js/add_copilot_reviewer.cjs | 54 -- .../setup/js/add_copilot_reviewer.test.cjs | 215 ------ actions/setup/js/demo_enhanced_errors.cjs | 59 -- actions/setup/js/expiration_helpers.cjs | 28 - actions/setup/js/markdown_transformer.cjs | 84 -- .../setup/js/markdown_transformer.test.cjs | 487 ------------ .../setup/js/safe_output_topological_sort.cjs | 288 ------- .../js/safe_output_topological_sort.test.cjs | 731 ------------------ actions/setup/js/safe_outputs_mcp_client.cjs | 135 ---- .../setup/js/safe_outputs_mcp_client.test.cjs | 105 --- actions/setup/js/update_runner.cjs | 449 ----------- actions/setup/js/update_runner.test.cjs | 608 --------------- 12 files changed, 3243 deletions(-) delete mode 100644 actions/setup/js/add_copilot_reviewer.cjs delete mode 100644 actions/setup/js/add_copilot_reviewer.test.cjs delete mode 100755 actions/setup/js/demo_enhanced_errors.cjs delete mode 100644 actions/setup/js/expiration_helpers.cjs delete mode 100644 actions/setup/js/markdown_transformer.cjs delete mode 100644 actions/setup/js/markdown_transformer.test.cjs delete mode 100644 actions/setup/js/safe_output_topological_sort.cjs delete mode 100644 actions/setup/js/safe_output_topological_sort.test.cjs delete mode 100644 actions/setup/js/safe_outputs_mcp_client.cjs delete mode 100644 actions/setup/js/safe_outputs_mcp_client.test.cjs delete mode 100644 actions/setup/js/update_runner.cjs delete mode 100644 actions/setup/js/update_runner.test.cjs diff --git a/actions/setup/js/add_copilot_reviewer.cjs b/actions/setup/js/add_copilot_reviewer.cjs deleted file mode 100644 index 5c3a701260e..00000000000 --- a/actions/setup/js/add_copilot_reviewer.cjs +++ /dev/null @@ -1,54 +0,0 @@ -// @ts-check -/// - -const { getErrorMessage } = require("./error_helpers.cjs"); -const { ERR_CONFIG, ERR_NOT_FOUND, ERR_VALIDATION } = require("./error_codes.cjs"); -const { COPILOT_REVIEWER_BOT } = require("./constants.cjs"); - -/** - * Add Copilot as a reviewer to a pull request. - * - * Runs in github-script context. Requires `PR_NUMBER` environment variable. - */ -async function main() { - const prNumberStr = process.env.PR_NUMBER?.trim(); - - if (!prNumberStr) { - core.setFailed(`${ERR_CONFIG}: PR_NUMBER environment variable is required but not set`); - return; - } - - const prNumber = parseInt(prNumberStr, 10); - if (isNaN(prNumber) || prNumber <= 0) { - core.setFailed(`${ERR_VALIDATION}: Invalid PR_NUMBER: ${prNumberStr}. Must be a positive integer.`); - return; - } - - core.info(`Adding Copilot as reviewer to PR #${prNumber}`); - - const { owner, repo } = context.repo; - try { - await github.rest.pulls.requestReviewers({ - owner, - repo, - pull_number: prNumber, - reviewers: [COPILOT_REVIEWER_BOT], - }); - - core.info(`Successfully added Copilot as reviewer to PR #${prNumber}`); - - await core.summary - .addRaw( - `## Copilot Reviewer Added - -Successfully added Copilot as a reviewer to PR #${prNumber}.` - ) - .write(); - } catch (error) { - const errorMessage = getErrorMessage(error); - core.error(`Failed to add Copilot as reviewer: ${errorMessage}`); - core.setFailed(`${ERR_NOT_FOUND}: Failed to add Copilot as reviewer to PR #${prNumber}: ${errorMessage}`); - } -} - -module.exports = { main }; diff --git a/actions/setup/js/add_copilot_reviewer.test.cjs b/actions/setup/js/add_copilot_reviewer.test.cjs deleted file mode 100644 index bb83ffe435a..00000000000 --- a/actions/setup/js/add_copilot_reviewer.test.cjs +++ /dev/null @@ -1,215 +0,0 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; -const { ERR_CONFIG } = require("./error_codes.cjs"); - -// Mock the global objects that GitHub Actions provides -const mockCore = { - debug: vi.fn(), - info: vi.fn(), - notice: vi.fn(), - warning: vi.fn(), - error: vi.fn(), - setFailed: vi.fn(), - setOutput: vi.fn(), - summary: { - addRaw: vi.fn().mockReturnThis(), - write: vi.fn().mockResolvedValue(), - }, -}; - -const mockGithub = { - rest: { - pulls: { - requestReviewers: vi.fn().mockResolvedValue({}), - }, - }, -}; - -const mockContext = { - eventName: "pull_request", - repo: { - owner: "testowner", - repo: "testrepo", - }, - payload: { - pull_request: { - number: 123, - }, - }, -}; - -// Set up global mocks before importing the module -global.core = mockCore; -global.github = mockGithub; -global.context = mockContext; - -describe("add_copilot_reviewer", () => { - beforeEach(() => { - // Reset all mocks before each test - vi.clearAllMocks(); - vi.resetModules(); // Reset module cache to allow fresh imports - - // Clear environment variables - delete process.env.PR_NUMBER; - - // Reset context to default - global.context = { - eventName: "pull_request", - repo: { - owner: "testowner", - repo: "testrepo", - }, - payload: { - pull_request: { - number: 123, - }, - }, - }; - }); - - // Helper function to run the script with main() call - async function runScript() { - const { main } = await import("./add_copilot_reviewer.cjs?" + Date.now()); - await main(); - } - - it("should fail when PR_NUMBER is not set", async () => { - delete process.env.PR_NUMBER; - - await runScript(); - - expect(mockCore.setFailed).toHaveBeenCalledWith(`${ERR_CONFIG}: PR_NUMBER environment variable is required but not set`); - expect(mockGithub.rest.pulls.requestReviewers).not.toHaveBeenCalled(); - }); - - it("should fail when PR_NUMBER is empty", async () => { - process.env.PR_NUMBER = " "; - - await runScript(); - - expect(mockCore.setFailed).toHaveBeenCalledWith(`${ERR_CONFIG}: PR_NUMBER environment variable is required but not set`); - expect(mockGithub.rest.pulls.requestReviewers).not.toHaveBeenCalled(); - }); - - it("should fail when PR_NUMBER is not a valid number", async () => { - process.env.PR_NUMBER = "not-a-number"; - - await runScript(); - - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Invalid PR_NUMBER")); - expect(mockGithub.rest.pulls.requestReviewers).not.toHaveBeenCalled(); - }); - - it("should fail when PR_NUMBER is zero", async () => { - process.env.PR_NUMBER = "0"; - - await runScript(); - - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Invalid PR_NUMBER")); - expect(mockGithub.rest.pulls.requestReviewers).not.toHaveBeenCalled(); - }); - - it("should fail when PR_NUMBER is negative", async () => { - process.env.PR_NUMBER = "-1"; - - await runScript(); - - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Invalid PR_NUMBER")); - expect(mockGithub.rest.pulls.requestReviewers).not.toHaveBeenCalled(); - }); - - it("should add copilot as reviewer when PR_NUMBER is valid", async () => { - process.env.PR_NUMBER = "456"; - - await runScript(); - - expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledWith({ - owner: "testowner", - repo: "testrepo", - pull_number: 456, - reviewers: ["copilot-pull-request-reviewer[bot]"], - }); - expect(mockCore.info).toHaveBeenCalledWith("Successfully added Copilot as reviewer to PR #456"); - expect(mockCore.summary.addRaw).toHaveBeenCalled(); - expect(mockCore.summary.write).toHaveBeenCalled(); - }); - - it("should handle API errors gracefully", async () => { - process.env.PR_NUMBER = "123"; - mockGithub.rest.pulls.requestReviewers.mockRejectedValueOnce(new Error("API Error")); - - await runScript(); - - expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("Failed to add Copilot as reviewer")); - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to add Copilot as reviewer")); - }); - - it("should trim whitespace from PR_NUMBER", async () => { - process.env.PR_NUMBER = " 789 "; - - await runScript(); - - expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledWith({ - owner: "testowner", - repo: "testrepo", - pull_number: 789, - reviewers: ["copilot-pull-request-reviewer[bot]"], - }); - }); - - it("should log initial info message when starting", async () => { - process.env.PR_NUMBER = "100"; - - await runScript(); - - expect(mockCore.info).toHaveBeenCalledWith("Adding Copilot as reviewer to PR #100"); - }); - - it("should truncate float PR_NUMBER to integer", async () => { - process.env.PR_NUMBER = "5.9"; - - await runScript(); - - expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledWith(expect.objectContaining({ pull_number: 5 })); - }); - - it("should include PR number in failure message on API error", async () => { - process.env.PR_NUMBER = "321"; - mockGithub.rest.pulls.requestReviewers.mockRejectedValueOnce(new Error("Rate limited")); - - await runScript(); - - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("321")); - }); - - it("should include ERR_VALIDATION in message for non-numeric PR_NUMBER", async () => { - const { ERR_VALIDATION } = require("./error_codes.cjs"); - process.env.PR_NUMBER = "abc"; - - await runScript(); - - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining(ERR_VALIDATION)); - }); - - it("should call core.error before core.setFailed on API error", async () => { - process.env.PR_NUMBER = "555"; - const apiError = new Error("Forbidden"); - mockGithub.rest.pulls.requestReviewers.mockRejectedValueOnce(apiError); - - const callOrder = /** @type {string[]} */ []; - mockCore.error.mockImplementation(() => callOrder.push("error")); - mockCore.setFailed.mockImplementation(() => callOrder.push("setFailed")); - - await runScript(); - - expect(callOrder).toEqual(["error", "setFailed"]); - }); - - it("should write summary with PR number on success", async () => { - process.env.PR_NUMBER = "42"; - - await runScript(); - - expect(mockCore.summary.addRaw).toHaveBeenCalledWith(expect.stringContaining("42")); - expect(mockCore.summary.write).toHaveBeenCalled(); - }); -}); diff --git a/actions/setup/js/demo_enhanced_errors.cjs b/actions/setup/js/demo_enhanced_errors.cjs deleted file mode 100755 index 30807c773f5..00000000000 --- a/actions/setup/js/demo_enhanced_errors.cjs +++ /dev/null @@ -1,59 +0,0 @@ -#!/usr/bin/env node -// @ts-check - -/** - * Demonstration of Enhanced MCP Error Messages - * - * This script demonstrates how the enhanced error messages provide - * actionable guidance when tools are called with missing parameters. - * - * NOTE: This is a demonstration script that only uses "body" as a string literal - * in examples. No sanitization is needed as no user-provided content is processed. - */ - -const { generateEnhancedErrorMessage } = require("./mcp_enhanced_errors.cjs"); -const tools = require("./safe_outputs_tools.json"); -// SEC-004: No sanitize needed - demo script with string literals only - -console.log("=".repeat(80)); -console.log("Enhanced MCP Error Messages - Demonstration"); -console.log("=".repeat(80)); -console.log(); - -// Demonstrate error for add_comment without item_number -const addCommentTool = tools.find(t => t.name === "add_comment"); -if (addCommentTool) { - console.log("Example 1: Missing 'item_number' parameter in add_comment"); - console.log("-".repeat(80)); - const error = generateEnhancedErrorMessage(["item_number"], addCommentTool.name, addCommentTool.inputSchema); - console.log(error); - console.log(); -} - -// Demonstrate error for create_issue without title and body -const createIssueTool = tools.find(t => t.name === "create_issue"); -if (createIssueTool) { - console.log("Example 2: Missing 'title' and 'body' parameters in create_issue"); - console.log("-".repeat(80)); - const error = generateEnhancedErrorMessage(["title", "body"], createIssueTool.name, createIssueTool.inputSchema); - console.log(error); - console.log(); -} - -// Demonstrate error for assign_milestone with missing parameters -const assignMilestoneTool = tools.find(t => t.name === "assign_milestone"); -if (assignMilestoneTool) { - console.log("Example 3: Missing required parameters in assign_milestone"); - console.log("-".repeat(80)); - const error = generateEnhancedErrorMessage(["issue_number", "milestone_number"], assignMilestoneTool.name, assignMilestoneTool.inputSchema); - console.log(error); - console.log(); -} - -console.log("=".repeat(80)); -console.log("Benefits of Enhanced Error Messages:"); -console.log(" 1. Clear indication of which parameters are missing"); -console.log(" 2. Description of what each parameter represents"); -console.log(" 3. Example showing correct usage format"); -console.log(" 4. Helps agents self-correct without human intervention"); -console.log("=".repeat(80)); diff --git a/actions/setup/js/expiration_helpers.cjs b/actions/setup/js/expiration_helpers.cjs deleted file mode 100644 index 7820e6bb2e1..00000000000 --- a/actions/setup/js/expiration_helpers.cjs +++ /dev/null @@ -1,28 +0,0 @@ -// @ts-check -/// - -const { createExpirationLine } = require("./ephemerals.cjs"); - -/** - * Add expiration checkbox with XML comment to body lines if expires is set - * @param {string[]} bodyLines - Array of body lines to append to - * @param {string} envVarName - Name of the environment variable containing expires hours (e.g., "GH_AW_DISCUSSION_EXPIRES") - * @param {string} entityType - Type of entity for logging (e.g., "Discussion", "Issue", "Pull Request") - * @returns {void} - */ -function addExpirationComment(bodyLines, envVarName, entityType) { - const expiresEnv = process.env[envVarName]; - if (expiresEnv) { - const expiresHours = parseInt(expiresEnv, 10); - if (!isNaN(expiresHours) && expiresHours > 0) { - const expirationDate = new Date(); - expirationDate.setHours(expirationDate.getHours() + expiresHours); - bodyLines.push(createExpirationLine(expirationDate)); - core.info(`${entityType} will expire on ${expirationDate.toISOString()} (${expiresHours} hours)`); - } - } -} - -module.exports = { - addExpirationComment, -}; diff --git a/actions/setup/js/markdown_transformer.cjs b/actions/setup/js/markdown_transformer.cjs deleted file mode 100644 index 0f99324da00..00000000000 --- a/actions/setup/js/markdown_transformer.cjs +++ /dev/null @@ -1,84 +0,0 @@ -// @ts-check - -/** - * Transform markdown by increasing header levels by 1 - * (# becomes ##, ## becomes ###, etc.) - * - * This transformer is used to adjust the markdown generated by Copilot CLI - * conversation.md when displaying it in GitHub Actions step summaries. - * - * Rules: - * - Only transforms ATX-style headers (# prefix) - * - Preserves whitespace and indentation - * - Does not transform headers in code blocks - * - Does not transform headers in inline code - * - Maximum header level is 6 (h6) - * - Uses regex for efficient processing - * - * @param {string} markdown - Markdown content to transform - * @returns {string} Transformed markdown with increased header levels - */ -function increaseHeaderLevel(markdown) { - if (!markdown || typeof markdown !== "string") { - return markdown || ""; - } - - // Normalize line endings to \n for consistent processing - const normalizedMarkdown = markdown.replace(/\r\n/g, "\n"); - - // Split into lines for processing - const lines = normalizedMarkdown.split("\n"); - const result = []; - let inCodeBlock = false; - let codeBlockFence = ""; - - for (const line of lines) { - // Track code block state (fenced code blocks with ``` or ~~~) - const fenceMatch = line.match(/^(\s*)(`{3,}|~{3,})/); - if (fenceMatch) { - if (!inCodeBlock) { - // Starting a code block - inCodeBlock = true; - codeBlockFence = fenceMatch[2][0]; // Get the fence character (` or ~) - } else if (line.includes(codeBlockFence.repeat(3))) { - // Ending a code block (must have at least 3 fence chars) - inCodeBlock = false; - codeBlockFence = ""; - } - result.push(line); - continue; - } - - // If we're in a code block, don't transform - if (inCodeBlock) { - result.push(line); - continue; - } - - // Check if this line is an ATX-style header (# prefix) - // Pattern: optional whitespace, 1-6 #, at least one space, then content - const headerMatch = line.match(/^(\s*)(#{1,6})(\s+.*)$/); - if (headerMatch) { - const indent = headerMatch[1]; - const hashes = headerMatch[2]; - const content = headerMatch[3]; - - // Only increase if we're below h6 (6 hashes) - if (hashes.length < 6) { - result.push(`${indent}#${hashes}${content}`); - } else { - // Already at max level, keep as-is - result.push(line); - } - } else { - // Not a header, keep as-is - result.push(line); - } - } - - return result.join("\n"); -} - -module.exports = { - increaseHeaderLevel, -}; diff --git a/actions/setup/js/markdown_transformer.test.cjs b/actions/setup/js/markdown_transformer.test.cjs deleted file mode 100644 index fa675abceb7..00000000000 --- a/actions/setup/js/markdown_transformer.test.cjs +++ /dev/null @@ -1,487 +0,0 @@ -import { describe, it, expect } from "vitest"; - -describe("markdown_transformer.cjs", () => { - let markdownTransformer; - - beforeEach(async () => { - markdownTransformer = await import("./markdown_transformer.cjs"); - }); - - describe("increaseHeaderLevel", () => { - it("should transform h1 to h2", () => { - const input = "# Header 1"; - const expected = "## Header 1"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should transform h2 to h3", () => { - const input = "## Header 2"; - const expected = "### Header 2"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should transform h3 to h4", () => { - const input = "### Header 3"; - const expected = "#### Header 3"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should transform h4 to h5", () => { - const input = "#### Header 4"; - const expected = "##### Header 4"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should transform h5 to h6", () => { - const input = "##### Header 5"; - const expected = "###### Header 5"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should not transform h6 (max level)", () => { - const input = "###### Header 6"; - const expected = "###### Header 6"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should transform multiple headers in sequence", () => { - const input = `# Title -## Section -### Subsection -#### Detail`; - const expected = `## Title -### Section -#### Subsection -##### Detail`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should preserve non-header lines", () => { - const input = `# Header - -This is a paragraph. - -## Another Header - -More text here.`; - const expected = `## Header - -This is a paragraph. - -### Another Header - -More text here.`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should not transform headers in fenced code blocks (backticks)", () => { - const input = `# Real Header - -\`\`\` -# Fake Header -## Another Fake -\`\`\` - -## Real Header 2`; - const expected = `## Real Header - -\`\`\` -# Fake Header -## Another Fake -\`\`\` - -### Real Header 2`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should not transform headers in fenced code blocks (tildes)", () => { - const input = `# Real Header - -~~~ -# Fake Header -## Another Fake -~~~ - -## Real Header 2`; - const expected = `## Real Header - -~~~ -# Fake Header -## Another Fake -~~~ - -### Real Header 2`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should not transform headers in code blocks with language specifier", () => { - const input = `# Real Header - -\`\`\`markdown -# Fake Header in Markdown -## Another Fake -\`\`\` - -## Real Header 2`; - const expected = `## Real Header - -\`\`\`markdown -# Fake Header in Markdown -## Another Fake -\`\`\` - -### Real Header 2`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle nested code blocks correctly", () => { - const input = `# Header - -\`\`\` -Outer code block -# Not a header -\`\`\` - -## Section - -\`\`\` -Another code block -### Still not a header -\`\`\``; - const expected = `## Header - -\`\`\` -Outer code block -# Not a header -\`\`\` - -### Section - -\`\`\` -Another code block -### Still not a header -\`\`\``; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should preserve indentation in headers", () => { - const input = `# Header - ## Indented Header - ### More Indented`; - const expected = `## Header - ### Indented Header - #### More Indented`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle headers with special characters", () => { - const input = `# Header with **bold** -## Header with *italic* -### Header with \`code\` -#### Header with [link](url)`; - const expected = `## Header with **bold** -### Header with *italic* -#### Header with \`code\` -##### Header with [link](url)`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle headers with trailing spaces", () => { - const input = "# Header \n## Section "; - const expected = "## Header \n### Section "; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle headers with multiple spaces after #", () => { - const input = "# Header\n## Section"; - const expected = "## Header\n### Section"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should not transform lines without space after #", () => { - const input = "#No space\n## Valid Header"; - const expected = "#No space\n### Valid Header"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should not transform # in middle of line", () => { - const input = "This is # not a header\n# This is a header"; - const expected = "This is # not a header\n## This is a header"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle empty string", () => { - expect(markdownTransformer.increaseHeaderLevel("")).toBe(""); - }); - - it("should handle null input", () => { - expect(markdownTransformer.increaseHeaderLevel(null)).toBe(""); - }); - - it("should handle undefined input", () => { - expect(markdownTransformer.increaseHeaderLevel(undefined)).toBe(""); - }); - - it("should handle markdown with no headers", () => { - const input = "Just some text\nWith multiple lines\nBut no headers"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(input); - }); - - it("should handle mixed ATX headers and content", () => { - const input = `# Main Title - -Paragraph with some content. - -## Section 1 - -Some text here. - -### Subsection 1.1 - -More content. - -## Section 2 - -Final section.`; - const expected = `## Main Title - -Paragraph with some content. - -### Section 1 - -Some text here. - -#### Subsection 1.1 - -More content. - -### Section 2 - -Final section.`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle headers with emojis", () => { - const input = "# 🚀 Header\n## 📝 Section"; - const expected = "## 🚀 Header\n### 📝 Section"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle headers with numbers", () => { - const input = "# 1. First Header\n## 2.1 Second Header"; - const expected = "## 1. First Header\n### 2.1 Second Header"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle complex real-world example", () => { - const input = `# Conversation Summary - -## Initialization - -Model: gpt-4 - -## Turn 1 - -### User - -Hello, world! - -### Assistant - -Hi there! - -## Turn 2 - -### User - -\`\`\`javascript -# This is not a header -const x = 1; -\`\`\` - -### Assistant - -\`\`\` -# Also not a header -## Nope -\`\`\` - -## Final Summary - -All done!`; - - const expected = `## Conversation Summary - -### Initialization - -Model: gpt-4 - -### Turn 1 - -#### User - -Hello, world! - -#### Assistant - -Hi there! - -### Turn 2 - -#### User - -\`\`\`javascript -# This is not a header -const x = 1; -\`\`\` - -#### Assistant - -\`\`\` -# Also not a header -## Nope -\`\`\` - -### Final Summary - -All done!`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle code blocks with longer fences", () => { - const input = `# Header - -\`\`\`\` -# Not a header -\`\`\`\` - -## Section`; - const expected = `## Header - -\`\`\`\` -# Not a header -\`\`\`\` - -### Section`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle indented code blocks", () => { - const input = `# Header - - \`\`\` - # Not a header - \`\`\` - -## Section`; - const expected = `## Header - - \`\`\` - # Not a header - \`\`\` - -### Section`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle Windows line endings", () => { - const input = "# Header\r\n## Section\r\n"; - const result = markdownTransformer.increaseHeaderLevel(input); - // Line endings are normalized to \n - expect(result).toBe("## Header\n### Section\n"); - }); - - it("should handle headers at different nesting levels", () => { - const input = `# H1 -## H2 -### H3 -#### H4 -##### H5 -###### H6 -##### H5 again -#### H4 again -### H3 again -## H2 again -# H1 again`; - const expected = `## H1 -### H2 -#### H3 -##### H4 -###### H5 -###### H6 -###### H5 again -##### H4 again -#### H3 again -### H2 again -## H1 again`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle headers with HTML entities", () => { - const input = "# Header & Content\n## Section <tag>"; - const expected = "## Header & Content\n### Section <tag>"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should not transform setext-style headers", () => { - const input = `Header 1 -======== - -Header 2 --------- - -# ATX Header`; - // Setext headers are not transformed, only ATX - const expected = `Header 1 -======== - -Header 2 --------- - -## ATX Header`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle consecutive code blocks", () => { - const input = `# Header - -\`\`\` -Block 1 -# Not a header -\`\`\` - -\`\`\` -Block 2 -## Also not a header -\`\`\` - -## Section`; - const expected = `## Header - -\`\`\` -Block 1 -# Not a header -\`\`\` - -\`\`\` -Block 2 -## Also not a header -\`\`\` - -### Section`; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle empty headers", () => { - const input = "# \n## Section"; - const expected = "## \n### Section"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - - it("should handle headers with only whitespace after #", () => { - const input = "# \n## Section"; - const expected = "## \n### Section"; - expect(markdownTransformer.increaseHeaderLevel(input)).toBe(expected); - }); - }); -}); diff --git a/actions/setup/js/safe_output_topological_sort.cjs b/actions/setup/js/safe_output_topological_sort.cjs deleted file mode 100644 index 1198d516fdd..00000000000 --- a/actions/setup/js/safe_output_topological_sort.cjs +++ /dev/null @@ -1,288 +0,0 @@ -// @ts-check -/// - -/** - * Topological Sort for Safe Output Tool Calls - * - * This module provides topological sorting of safe output messages based on - * temporary ID dependencies. Messages that create entities without referencing - * temporary IDs are processed first, followed by messages that depend on them. - * - * This enables resolution of all temporary IDs in a single pass for acyclic - * dependency graphs (graphs without loops). - */ - -const { extractTemporaryIdReferences, getCreatedTemporaryId } = require("./temporary_id.cjs"); - -/** - * Build a dependency graph for safe output messages - * Returns: - * - dependencies: Map of message index -> Set of message indices it depends on - * - providers: Map of temporary ID -> message index that creates it - * - * @param {Array} messages - Array of safe output messages - * @returns {{dependencies: Map>, providers: Map}} - */ -function buildDependencyGraph(messages) { - /** @type {Map>} */ - const dependencies = new Map(); - - /** @type {Map} */ - const providers = new Map(); - - // First pass: identify which messages create which temporary IDs - for (let i = 0; i < messages.length; i++) { - const message = messages[i]; - const createdId = getCreatedTemporaryId(message); - - if (createdId !== null) { - if (providers.has(createdId)) { - // Duplicate temporary ID - this is a problem - // We'll let the handler deal with this, but note it - if (typeof core !== "undefined") { - core.warning(`Duplicate temporary_id '${createdId}' at message indices ${providers.get(createdId)} and ${i}. ` + `Only the first occurrence will be used.`); - } - } else { - providers.set(createdId, i); - } - } - - // Initialize dependencies set for this message - dependencies.set(i, new Set()); - } - - // Second pass: identify dependencies - for (let i = 0; i < messages.length; i++) { - const message = messages[i]; - const referencedIds = extractTemporaryIdReferences(message); - - // For each temporary ID this message references, find the provider - for (const tempId of referencedIds) { - const providerIndex = providers.get(tempId); - - if (providerIndex !== undefined) { - // This message depends on the provider message - const deps = dependencies.get(i); - if (deps) { - deps.add(providerIndex); - } - } - // If no provider, the temp ID might be from a previous step or be unresolved - // We don't add a dependency in this case - } - } - - return { dependencies, providers }; -} - -/** - * Detect cycles in the dependency graph using iterative mark-and-sweep algorithm - * Returns an array of message indices that form a cycle, or empty array if no cycle - * - * @param {Map>} dependencies - Dependency graph - * @returns {Array} Indices of messages forming a cycle, or empty array - */ -function detectCycle(dependencies) { - const WHITE = 0; // Not visited - const GRAY = 1; // Visiting (on stack) - const BLACK = 2; // Visited (completed) - - const colors = new Map(); - const parent = new Map(); - - // Initialize all nodes as WHITE - for (const node of dependencies.keys()) { - colors.set(node, WHITE); - parent.set(node, null); - } - - // Try to find cycle starting from each WHITE node - for (const startNode of dependencies.keys()) { - if (colors.get(startNode) !== WHITE) { - continue; - } - - // Use a stack for iterative DFS - // Each stack entry: [node, iterator, isReturning] - /** @type {Array<[number, any, boolean]>} */ - const stack = [[startNode, null, false]]; - - while (stack.length > 0) { - const entry = stack.pop(); - if (!entry) continue; - - const [node, depsIterator, isReturning] = entry; - - if (isReturning) { - // Returning from exploring this node - mark as BLACK - colors.set(node, BLACK); - continue; - } - - // Mark node as GRAY (being explored) - colors.set(node, GRAY); - - // Push node again to mark BLACK when we return - stack.push([node, null, true]); - - // Get dependencies for this node - const deps = dependencies.get(node) || new Set(); - - // Process each dependency - for (const dep of deps) { - const depColor = colors.get(dep); - - if (depColor === WHITE) { - // Not visited yet - explore it - parent.set(dep, node); - stack.push([dep, null, false]); - } else if (depColor === GRAY) { - // Found a back edge - cycle detected! - // Reconstruct the cycle from dep to current node - const cycle = [dep]; - let current = node; - while (current !== dep && current !== null) { - cycle.unshift(current); - current = parent.get(current); - } - return cycle; - } - // If BLACK, it's already fully explored - no cycle through this path - } - } - } - - return []; -} - -/** - * Perform topological sort on messages using Kahn's algorithm - * Messages without dependencies come first, followed by their dependents - * - * @param {Array} messages - Array of safe output messages - * @param {Map>} dependencies - Dependency graph - * @returns {Array} Array of message indices in topologically sorted order - */ -function topologicalSort(messages, dependencies) { - // Calculate in-degree (number of dependencies) for each message - const inDegree = new Map(); - for (let i = 0; i < messages.length; i++) { - const deps = dependencies.get(i) || new Set(); - inDegree.set(i, deps.size); - } - - // Queue of messages with no dependencies - const queue = []; - for (let i = 0; i < messages.length; i++) { - if (inDegree.get(i) === 0) { - queue.push(i); - } - } - - const sorted = []; - - while (queue.length > 0) { - // Process nodes in order of appearance for stability - // This preserves the original order when there are no dependencies - const node = queue.shift(); - if (node !== undefined) { - sorted.push(node); - - // Find all messages that depend on this one - for (const [other, deps] of dependencies.entries()) { - if (deps.has(node)) { - // Reduce in-degree - const currentDegree = inDegree.get(other); - if (currentDegree !== undefined) { - inDegree.set(other, currentDegree - 1); - - // If all dependencies satisfied, add to queue - if (inDegree.get(other) === 0) { - queue.push(other); - } - } - } - } - } - } - - // If sorted.length < messages.length, there's a cycle - if (sorted.length < messages.length) { - const unsorted = []; - for (let i = 0; i < messages.length; i++) { - if (!sorted.includes(i)) { - unsorted.push(i); - } - } - - if (typeof core !== "undefined") { - core.warning(`Topological sort incomplete: ${sorted.length}/${messages.length} messages sorted. ` + `Messages ${unsorted.join(", ")} may be part of a dependency cycle.`); - } - } - - return sorted; -} - -/** - * Sort safe output messages in topological order based on temporary ID dependencies - * Messages that don't reference temporary IDs are processed first, followed by - * messages that depend on them. This enables single-pass resolution of temporary IDs. - * - * If a cycle is detected, the original order is preserved and a warning is logged. - * - * @param {Array} messages - Array of safe output messages - * @returns {Array} Messages in topologically sorted order - */ -function sortSafeOutputMessages(messages) { - if (!Array.isArray(messages) || messages.length === 0) { - return messages; - } - - // Build dependency graph - const { dependencies, providers } = buildDependencyGraph(messages); - - if (typeof core !== "undefined") { - const messagesWithDeps = Array.from(dependencies.entries()).filter(([_, deps]) => deps.size > 0); - core.info(`Dependency analysis: ${providers.size} message(s) create temporary IDs, ` + `${messagesWithDeps.length} message(s) have dependencies`); - } - - // Check for cycles - const cycle = detectCycle(dependencies); - if (cycle.length > 0) { - if (typeof core !== "undefined") { - const cycleMessages = cycle.map(i => { - const msg = messages[i]; - const tempId = getCreatedTemporaryId(msg); - return `${i} (${msg.type}${tempId ? `, creates ${tempId}` : ""})`; - }); - core.warning(`Dependency cycle detected in safe output messages: ${cycleMessages.join(" -> ")}. ` + `Temporary IDs may not resolve correctly. Messages will be processed in original order.`); - } - // Return original order if there's a cycle - return messages; - } - - // Perform topological sort - const sortedIndices = topologicalSort(messages, dependencies); - - // Reorder messages according to sorted indices - const sortedMessages = sortedIndices.map(i => messages[i]); - - if (typeof core !== "undefined" && sortedIndices.length > 0) { - // Check if order changed - const orderChanged = sortedIndices.some((idx, i) => idx !== i); - if (orderChanged) { - core.info(`Topological sort reordered ${messages.length} message(s) to resolve temporary ID dependencies. ` + `New order: [${sortedIndices.join(", ")}]`); - } else { - core.info(`Topological sort: Messages already in optimal order (no reordering needed)`); - } - } - - return sortedMessages; -} - -module.exports = { - buildDependencyGraph, - detectCycle, - topologicalSort, - sortSafeOutputMessages, -}; diff --git a/actions/setup/js/safe_output_topological_sort.test.cjs b/actions/setup/js/safe_output_topological_sort.test.cjs deleted file mode 100644 index adcf637b981..00000000000 --- a/actions/setup/js/safe_output_topological_sort.test.cjs +++ /dev/null @@ -1,731 +0,0 @@ -// @ts-check -import { describe, it, expect, beforeEach, vi } from "vitest"; - -// Mock core for logging -const mockCore = { - info: vi.fn(), - warning: vi.fn(), - debug: vi.fn(), -}; -global.core = mockCore; - -describe("safe_output_topological_sort.cjs", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - describe("buildDependencyGraph", () => { - it("should build graph with simple dependency", async () => { - const { buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_ddd111", title: "Parent" }, - { type: "add_comment", issue_number: "aw_ddd111", body: "Comment" }, - ]; - - const { dependencies, providers } = buildDependencyGraph(messages); - - expect(providers.size).toBe(1); - expect(providers.get("aw_ddd111")).toBe(0); - - expect(dependencies.get(0).size).toBe(0); // Message 0 has no dependencies - expect(dependencies.get(1).size).toBe(1); // Message 1 depends on message 0 - expect(dependencies.get(1).has(0)).toBe(true); - }); - - it("should build graph with chain of dependencies", async () => { - const { buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_ddd111", title: "First" }, - { type: "create_issue", temporary_id: "aw_eee222", body: "Ref #aw_ddd111" }, - { type: "create_issue", temporary_id: "aw_fff333", body: "Ref #aw_eee222" }, - ]; - - const { dependencies, providers } = buildDependencyGraph(messages); - - expect(providers.size).toBe(3); - - expect(dependencies.get(0).size).toBe(0); // No dependencies - expect(dependencies.get(1).size).toBe(1); // Depends on 0 - expect(dependencies.get(1).has(0)).toBe(true); - expect(dependencies.get(2).size).toBe(1); // Depends on 1 - expect(dependencies.get(2).has(1)).toBe(true); - }); - - it("should handle multiple dependencies", async () => { - const { buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_ddd111", title: "Issue 1" }, - { type: "create_issue", temporary_id: "aw_eee222", title: "Issue 2" }, - { - type: "create_issue", - temporary_id: "aw_fff333", - body: "See #aw_ddd111 and #aw_eee222", - }, - ]; - - const { dependencies, providers } = buildDependencyGraph(messages); - - expect(dependencies.get(2).size).toBe(2); - expect(dependencies.get(2).has(0)).toBe(true); - expect(dependencies.get(2).has(1)).toBe(true); - }); - - it("should warn on duplicate temporary IDs", async () => { - const { buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_abc111", title: "First" }, - { type: "create_issue", temporary_id: "aw_abc111", title: "Second" }, - ]; - - const { providers } = buildDependencyGraph(messages); - - expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Duplicate temporary_id 'aw_abc111'")); - - // Verify only the first occurrence is used as provider - expect(providers.get("aw_abc111")).toBe(0); - expect(providers.size).toBe(1); - }); - - it("should handle messages without temporary IDs", async () => { - const { buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", title: "No temp ID" }, - { type: "add_comment", issue_number: 123, body: "Regular issue" }, - ]; - - const { dependencies, providers } = buildDependencyGraph(messages); - - expect(providers.size).toBe(0); - expect(dependencies.get(0).size).toBe(0); - expect(dependencies.get(1).size).toBe(0); - }); - }); - - describe("detectCycle", () => { - it("should detect simple cycle", async () => { - const { detectCycle } = await import("./safe_output_topological_sort.cjs"); - - // Create a cycle: 0 -> 1 -> 0 - const dependencies = new Map([ - [0, new Set([1])], - [1, new Set([0])], - ]); - - const cycle = detectCycle(dependencies); - - expect(cycle.length).toBeGreaterThan(0); - }); - - it("should detect complex cycle", async () => { - const { detectCycle } = await import("./safe_output_topological_sort.cjs"); - - // Create a cycle: 0 -> 1 -> 2 -> 0 - const dependencies = new Map([ - [0, new Set([1])], - [1, new Set([2])], - [2, new Set([0])], - ]); - - const cycle = detectCycle(dependencies); - - expect(cycle.length).toBeGreaterThan(0); - }); - - it("should return empty array for acyclic graph", async () => { - const { detectCycle } = await import("./safe_output_topological_sort.cjs"); - - // Acyclic: 0 -> 1 -> 2 - const dependencies = new Map([ - [0, new Set()], - [1, new Set([0])], - [2, new Set([1])], - ]); - - const cycle = detectCycle(dependencies); - - expect(cycle.length).toBe(0); - }); - - it("should handle disconnected components", async () => { - const { detectCycle } = await import("./safe_output_topological_sort.cjs"); - - // Two separate chains: 0 -> 1 and 2 -> 3 - const dependencies = new Map([ - [0, new Set()], - [1, new Set([0])], - [2, new Set()], - [3, new Set([2])], - ]); - - const cycle = detectCycle(dependencies); - - expect(cycle.length).toBe(0); - }); - }); - - describe("topologicalSort", () => { - it("should sort messages with simple dependency", async () => { - const { topologicalSort, buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "add_comment", issue_number: "aw_ddd111", body: "Comment" }, - { type: "create_issue", temporary_id: "aw_ddd111", title: "Parent" }, - ]; - - const { dependencies } = buildDependencyGraph(messages); - const sorted = topologicalSort(messages, dependencies); - - // Message 1 (create_issue) should come before message 0 (add_comment) - expect(sorted).toEqual([1, 0]); - }); - - it("should preserve original order when no dependencies", async () => { - const { topologicalSort, buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_ddd111", title: "Issue 1" }, - { type: "create_issue", temporary_id: "aw_eee222", title: "Issue 2" }, - { type: "create_issue", temporary_id: "aw_fff333", title: "Issue 3" }, - ]; - - const { dependencies } = buildDependencyGraph(messages); - const sorted = topologicalSort(messages, dependencies); - - expect(sorted).toEqual([0, 1, 2]); - }); - - it("should sort dependency chain correctly", async () => { - const { topologicalSort, buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_fff333", body: "Ref #aw_eee222" }, - { type: "create_issue", temporary_id: "aw_eee222", body: "Ref #aw_ddd111" }, - { type: "create_issue", temporary_id: "aw_ddd111", title: "First" }, - ]; - - const { dependencies } = buildDependencyGraph(messages); - const sorted = topologicalSort(messages, dependencies); - - // Should be: message 2 (first), then 1 (second), then 0 (third) - expect(sorted).toEqual([2, 1, 0]); - }); - - it("should handle multiple independent messages", async () => { - const { topologicalSort, buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_ddd111", title: "Independent 1" }, - { type: "add_comment", issue_number: "aw_ddd111", body: "Comment on 1" }, - { type: "create_issue", temporary_id: "aw_eee222", title: "Independent 2" }, - { type: "add_comment", issue_number: "aw_eee222", body: "Comment on 2" }, - ]; - - const { dependencies } = buildDependencyGraph(messages); - const sorted = topologicalSort(messages, dependencies); - - // Creates should come before their comments - expect(sorted.indexOf(0)).toBeLessThan(sorted.indexOf(1)); // Issue 1 before comment on 1 - expect(sorted.indexOf(2)).toBeLessThan(sorted.indexOf(3)); // Issue 2 before comment on 2 - }); - - it("should handle complex dependency graph", async () => { - const { topologicalSort, buildDependencyGraph } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_aaa111", title: "Parent" }, - { type: "create_issue", temporary_id: "aw_bbb111", body: "Parent: #aw_aaa111" }, - { type: "create_issue", temporary_id: "aw_ccc222", body: "Parent: #aw_aaa111" }, - { type: "link_sub_issue", parent_issue_number: "aw_aaa111", sub_issue_number: "aw_bbb111" }, - { type: "link_sub_issue", parent_issue_number: "aw_aaa111", sub_issue_number: "aw_ccc222" }, - ]; - - const { dependencies } = buildDependencyGraph(messages); - const sorted = topologicalSort(messages, dependencies); - - // Parent must come first - expect(sorted[0]).toBe(0); - // Children must come after parent - const childIndices = [sorted.indexOf(1), sorted.indexOf(2)]; - expect(Math.min(...childIndices)).toBeGreaterThan(0); - // Links must come after all creates - expect(sorted.indexOf(3)).toBeGreaterThan(Math.max(...childIndices)); - expect(sorted.indexOf(4)).toBeGreaterThan(Math.max(...childIndices)); - }); - }); - - describe("sortSafeOutputMessages", () => { - it("should return empty array for empty input", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const sorted = sortSafeOutputMessages([]); - - expect(sorted).toEqual([]); - }); - - it("should return original messages for non-array input", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const input = null; - const sorted = sortSafeOutputMessages(input); - - expect(sorted).toBe(input); - }); - - it("should sort messages without temporary IDs first", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "add_comment", issue_number: "aw_ddd111", body: "Comment" }, - { type: "create_issue", temporary_id: "aw_ddd111", title: "Issue" }, - { type: "create_issue", title: "No temp ID" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Messages without dependencies should come first - expect(sorted[0].type).toBe("create_issue"); - expect(sorted[0].title).toBe("Issue"); - expect(sorted[1].type).toBe("create_issue"); - expect(sorted[1].title).toBe("No temp ID"); - expect(sorted[2].type).toBe("add_comment"); - }); - - it("should handle cross-references between issues, PRs, and discussions", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_pull_request", temporary_id: "aw_fedcba", body: "Fixes #aw_ddd111" }, - { type: "create_issue", temporary_id: "aw_ddd111", title: "Bug report" }, - { type: "create_discussion", temporary_id: "aw_abcdef", body: "See #aw_fedcba" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Issue should come first, then PR (which references issue), then discussion (which references PR) - expect(sorted[0].type).toBe("create_issue"); - expect(sorted[1].type).toBe("create_pull_request"); - expect(sorted[2].type).toBe("create_discussion"); - }); - - it("should return original order when cycle is detected", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_ddd111", body: "See #aw_eee222" }, - { type: "create_issue", temporary_id: "aw_eee222", body: "See #aw_ddd111" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Should return original order due to cycle - expect(sorted).toEqual(messages); - expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Dependency cycle detected")); - }); - - it("should log info about reordering", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "add_comment", issue_number: "aw_ddd111", body: "Comment" }, - { type: "create_issue", temporary_id: "aw_ddd111", title: "Issue" }, - ]; - - sortSafeOutputMessages(messages); - - expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Topological sort reordered")); - }); - - it("should log info when order doesn't change", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_issue", temporary_id: "aw_ddd111", title: "Issue" }, - { type: "add_comment", issue_number: "aw_ddd111", body: "Comment" }, - ]; - - sortSafeOutputMessages(messages); - - expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("already in optimal order")); - }); - - it("should handle complex real-world scenario", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - // Simulate a real workflow: create parent issue, create sub-issues, link them, add comments - const messages = [ - { type: "add_comment", issue_number: "aw_aaa111", body: "Status update" }, - { type: "link_sub_issue", parent_issue_number: "aw_aaa111", sub_issue_number: "aw_ccc222" }, - { type: "create_issue", temporary_id: "aw_bbb111", title: "Sub-task 1", body: "Parent: #aw_aaa111" }, - { type: "create_issue", temporary_id: "aw_aaa111", title: "Epic" }, - { type: "link_sub_issue", parent_issue_number: "aw_aaa111", sub_issue_number: "aw_bbb111" }, - { type: "create_issue", temporary_id: "aw_ccc222", title: "Sub-task 2", body: "Parent: #aw_aaa111" }, - { type: "add_comment", issue_number: "aw_bbb111", body: "Work started" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Verify parent is created first - const parentIndex = sorted.findIndex(m => m.temporary_id === "aw_aaa111"); - expect(parentIndex).toBe(0); - - // Verify children come after parent - const child1Index = sorted.findIndex(m => m.temporary_id === "aw_bbb111"); - const child2Index = sorted.findIndex(m => m.temporary_id === "aw_ccc222"); - expect(child1Index).toBeGreaterThan(parentIndex); - expect(child2Index).toBeGreaterThan(parentIndex); - - // Verify links come after all creates - const link1Index = sorted.findIndex(m => m.type === "link_sub_issue" && m.sub_issue_number === "aw_bbb111"); - const link2Index = sorted.findIndex(m => m.type === "link_sub_issue" && m.sub_issue_number === "aw_ccc222"); - expect(link1Index).toBeGreaterThan(child1Index); - expect(link2Index).toBeGreaterThan(child2Index); - - // Verify comments come after their targets - const parentCommentIndex = sorted.findIndex(m => m.type === "add_comment" && m.issue_number === "aw_aaa111"); - const child1CommentIndex = sorted.findIndex(m => m.type === "add_comment" && m.issue_number === "aw_bbb111"); - expect(parentCommentIndex).toBeGreaterThan(parentIndex); - expect(child1CommentIndex).toBeGreaterThan(child1Index); - }); - - it("should handle messages referencing external (already resolved) temp IDs", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - // Message references a temp ID that's not created in this batch - // (might be from a previous step) - const messages = [ - { type: "create_issue", temporary_id: "aw_abc1234", title: "New Issue" }, - { type: "add_comment", issue_number: "aw_def9876", body: "Comment on external" }, - { type: "add_comment", issue_number: "aw_abc1234", body: "Comment on new" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // New issue should come before its comment - expect(sorted[0].temporary_id).toBe("aw_abc1234"); - expect(sorted[2].issue_number).toBe("aw_abc1234"); - - // External reference can be anywhere (no dependency in this batch) - // It should appear but we don't enforce ordering relative to unrelated items - expect(sorted.some(m => m.issue_number === "aw_def9876")).toBe(true); - }); - - it("should handle create_project with item_url dependency on create_issue", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_project", title: "My Project", item_url: "aw_abc123" }, - { type: "create_issue", temporary_id: "aw_abc123", title: "Issue" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Issue should come before project since project depends on it - expect(sorted[0].type).toBe("create_issue"); - expect(sorted[1].type).toBe("create_project"); - }); - - it("should handle create_project with item_url dependency (URL format)", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_project", title: "My Project", item_url: "https://github.com/owner/repo/issues/aw_abc123" }, - { type: "create_issue", temporary_id: "aw_abc123", title: "Issue" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Issue should come before project - expect(sorted[0].type).toBe("create_issue"); - expect(sorted[1].type).toBe("create_project"); - }); - - it("should handle create_project with item_url dependency (URL with # prefix)", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_project", title: "My Project", item_url: "https://github.com/owner/repo/issues/#aw_abc123" }, - { type: "create_issue", temporary_id: "aw_abc123", title: "Issue" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Issue should come before project - expect(sorted[0].type).toBe("create_issue"); - expect(sorted[1].type).toBe("create_project"); - }); - - it("should handle update_project with content_number dependency on create_issue", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "update_project", project: "https://github.com/orgs/org/projects/1", content_type: "issue", content_number: "aw_abc123" }, - { type: "create_issue", temporary_id: "aw_abc123", title: "Issue" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Issue should come before update_project since update_project depends on it - expect(sorted[0].type).toBe("create_issue"); - expect(sorted[1].type).toBe("update_project"); - }); - - it("should handle large graphs with many dependencies", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - // Create a large tree: 1 root, 10 level-1 children, each with 5 level-2 children - const messages = []; - const rootId = "aw_root00000000"; - messages.push({ type: "create_issue", temporary_id: rootId, title: "Root" }); - - for (let i = 0; i < 10; i++) { - const level1Id = `aw_lv1n${i.toString().padStart(7, "0")}`; - messages.push({ - type: "create_issue", - temporary_id: level1Id, - body: `Parent: #${rootId}`, - }); - - for (let j = 0; j < 5; j++) { - const level2Id = `aw_lv2n${i}n${j.toString().padStart(4, "0")}`; - messages.push({ - type: "create_issue", - temporary_id: level2Id, - body: `Parent: #${level1Id}`, - }); - } - } - - const sorted = sortSafeOutputMessages(messages); - - // Verify root comes first - expect(sorted[0].temporary_id).toBe(rootId); - - // Verify each level-1 item comes before its level-2 children - for (let i = 0; i < 10; i++) { - const level1Id = `aw_lv1n${i.toString().padStart(7, "0")}`; - const level1Index = sorted.findIndex(m => m.temporary_id === level1Id); - - for (let j = 0; j < 5; j++) { - const level2Id = `aw_lv2n${i}n${j.toString().padStart(4, "0")}`; - const level2Index = sorted.findIndex(m => m.temporary_id === level2Id); - expect(level1Index).toBeLessThan(level2Index); - } - } - - // Verify root comes before all level-1 items - const rootIndex = sorted.findIndex(m => m.temporary_id === rootId); - for (let i = 0; i < 10; i++) { - const level1Id = `aw_lv1n${i.toString().padStart(7, "0")}`; - const level1Index = sorted.findIndex(m => m.temporary_id === level1Id); - expect(rootIndex).toBeLessThan(level1Index); - } - }); - - it("should handle deeply nested linear dependencies", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - // Create a chain: A -> B -> C -> D -> E -> F - const messages = []; - const ids = ["aw_aaa111", "aw_bbb222", "aw_ccc333", "aw_ddd444", "aw_eee555", "aw_fff666"]; - - // Add in reverse order to test sorting - for (let i = ids.length - 1; i >= 0; i--) { - messages.push({ - type: "create_issue", - temporary_id: ids[i], - body: i > 0 ? `Depends on #${ids[i - 1]}` : "First issue", - }); - } - - const sorted = sortSafeOutputMessages(messages); - - // Verify they're sorted in dependency order - for (let i = 0; i < ids.length; i++) { - expect(sorted[i].temporary_id).toBe(ids[i]); - } - }); - - it("should handle multiple disconnected dependency chains", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - // Chain 1: A -> B - { type: "create_issue", temporary_id: "aw_aaa111", title: "A" }, - { type: "create_issue", temporary_id: "aw_bbb222", body: "Ref #aw_aaa111" }, - - // Chain 2: C -> D - { type: "create_issue", temporary_id: "aw_ccc333", title: "C" }, - { type: "create_issue", temporary_id: "aw_ddd444", body: "Ref #aw_ccc333" }, - - // Chain 3: E -> F - { type: "create_issue", temporary_id: "aw_eee555", title: "E" }, - { type: "create_issue", temporary_id: "aw_fff666", body: "Ref #aw_eee555" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Within each chain, verify dependency order - const aIndex = sorted.findIndex(m => m.temporary_id === "aw_aaa111"); - const bIndex = sorted.findIndex(m => m.temporary_id === "aw_bbb222"); - expect(aIndex).toBeLessThan(bIndex); - - const cIndex = sorted.findIndex(m => m.temporary_id === "aw_ccc333"); - const dIndex = sorted.findIndex(m => m.temporary_id === "aw_ddd444"); - expect(cIndex).toBeLessThan(dIndex); - - const eIndex = sorted.findIndex(m => m.temporary_id === "aw_eee555"); - const fIndex = sorted.findIndex(m => m.temporary_id === "aw_fff666"); - expect(eIndex).toBeLessThan(fIndex); - }); - - it("should handle diamond dependency pattern", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - // Diamond: A -> B, A -> C, B -> D, C -> D - const messages = [ - { type: "create_issue", temporary_id: "aw_ddd444", body: "Needs #aw_bbb222 and #aw_ccc333" }, - { type: "create_issue", temporary_id: "aw_bbb222", body: "Child of #aw_aaa111" }, - { type: "create_issue", temporary_id: "aw_aaa111", title: "Root" }, - { type: "create_issue", temporary_id: "aw_ccc333", body: "Child of #aw_aaa111" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - const aIndex = sorted.findIndex(m => m.temporary_id === "aw_aaa111"); - const bIndex = sorted.findIndex(m => m.temporary_id === "aw_bbb222"); - const cIndex = sorted.findIndex(m => m.temporary_id === "aw_ccc333"); - const dIndex = sorted.findIndex(m => m.temporary_id === "aw_ddd444"); - - // A must come first - expect(aIndex).toBe(0); - - // B and C must come after A - expect(bIndex).toBeGreaterThan(aIndex); - expect(cIndex).toBeGreaterThan(aIndex); - - // D must come after both B and C - expect(dIndex).toBeGreaterThan(bIndex); - expect(dIndex).toBeGreaterThan(cIndex); - }); - - it("should handle messages with multiple temporary ID references in body", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { - type: "create_issue", - temporary_id: "aw_fff666", - body: "Blocks #aw_aaa111, #aw_bbb222, and #aw_ccc333", - }, - { type: "create_issue", temporary_id: "aw_aaa111", title: "First" }, - { type: "create_issue", temporary_id: "aw_bbb222", title: "Second" }, - { type: "create_issue", temporary_id: "aw_ccc333", title: "Third" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // All referenced IDs must come before the message that references them - const fIndex = sorted.findIndex(m => m.temporary_id === "aw_fff666"); - const aIndex = sorted.findIndex(m => m.temporary_id === "aw_aaa111"); - const bIndex = sorted.findIndex(m => m.temporary_id === "aw_bbb222"); - const cIndex = sorted.findIndex(m => m.temporary_id === "aw_ccc333"); - - expect(fIndex).toBeGreaterThan(aIndex); - expect(fIndex).toBeGreaterThan(bIndex); - expect(fIndex).toBeGreaterThan(cIndex); - }); - - it("should handle empty messages array gracefully", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const sorted = sortSafeOutputMessages([]); - - expect(sorted).toEqual([]); - }); - - it("should handle single message", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [{ type: "create_issue", temporary_id: "aw_aaa111", title: "Solo" }]; - - const sorted = sortSafeOutputMessages(messages); - - expect(sorted).toEqual(messages); - }); - - it("should preserve message object references", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const msg1 = { type: "create_issue", temporary_id: "aw_aaa111", title: "First" }; - const msg2 = { type: "create_issue", temporary_id: "aw_bbb222", body: "Ref #aw_aaa111" }; - - const sorted = sortSafeOutputMessages([msg2, msg1]); - - // Objects should be the same references, not copies - expect(sorted[0]).toBe(msg1); - expect(sorted[1]).toBe(msg2); - }); - - it("should handle cycle with 3 nodes", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - // Cycle: A -> B -> C -> A - const messages = [ - { type: "create_issue", temporary_id: "aw_aaa111", body: "Needs #aw_ccc333" }, - { type: "create_issue", temporary_id: "aw_bbb222", body: "Needs #aw_aaa111" }, - { type: "create_issue", temporary_id: "aw_ccc333", body: "Needs #aw_bbb222" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Should preserve original order when cycle detected - expect(sorted).toEqual(messages); - expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Dependency cycle detected")); - }); - - it("should handle messages with no type field", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { temporary_id: "aw_aaa111", title: "No type" }, - { type: "create_issue", temporary_id: "aw_bbb222", title: "Has type" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // Should handle gracefully without crashing - expect(sorted.length).toBe(2); - }); - - it("should handle mixed types (issues, PRs, discussions, comments)", async () => { - const { sortSafeOutputMessages } = await import("./safe_output_topological_sort.cjs"); - - const messages = [ - { type: "create_pull_request", temporary_id: "aw_aaa111", title: "PR" }, - { type: "add_comment", issue_number: "aw_aaa111", body: "Comment on PR" }, - { type: "create_discussion", temporary_id: "aw_bbb222", body: "See PR #aw_aaa111" }, - { type: "create_issue", temporary_id: "aw_ccc333", body: "Related to #aw_bbb222" }, - ]; - - const sorted = sortSafeOutputMessages(messages); - - // PR comes first - expect(sorted[0].type).toBe("create_pull_request"); - // Comment on PR comes after PR - const prIndex = sorted.findIndex(m => m.type === "create_pull_request"); - const commentIndex = sorted.findIndex(m => m.type === "add_comment"); - expect(commentIndex).toBeGreaterThan(prIndex); - // Discussion referencing PR comes after PR - const discussionIndex = sorted.findIndex(m => m.type === "create_discussion"); - expect(discussionIndex).toBeGreaterThan(prIndex); - // Issue referencing discussion comes after discussion - const issueIndex = sorted.findIndex(m => m.type === "create_issue"); - expect(issueIndex).toBeGreaterThan(discussionIndex); - }); - }); -}); diff --git a/actions/setup/js/safe_outputs_mcp_client.cjs b/actions/setup/js/safe_outputs_mcp_client.cjs deleted file mode 100644 index 59a0c74e959..00000000000 --- a/actions/setup/js/safe_outputs_mcp_client.cjs +++ /dev/null @@ -1,135 +0,0 @@ -// @ts-check - -const { spawn } = require("child_process"); -const path = require("path"); -const serverPath = path.join("/tmp/gh-aw/safeoutputs/mcp-server.cjs"); -const { GH_AW_SAFE_OUTPUTS_TOOL_CALLS } = process.env; -function parseJsonl(input) { - if (!input) return []; - return input - .split(/\r?\n/) - .map(l => l.trim()) - .filter(Boolean) - .map(line => JSON.parse(line)); -} -const toolCalls = parseJsonl(GH_AW_SAFE_OUTPUTS_TOOL_CALLS); -const child = spawn(process.execPath, [serverPath], { - stdio: ["pipe", "pipe", "pipe"], - env: process.env, -}); -let stdoutBuffer = Buffer.alloc(0); -const pending = new Map(); -let nextId = 1; -function writeMessage(obj) { - const json = JSON.stringify(obj); - const message = json + "\n"; - child.stdin.write(message); -} -function sendRequest(method, params) { - const id = nextId++; - const req = { jsonrpc: "2.0", id, method, params }; - return new Promise((resolve, reject) => { - pending.set(id, { resolve, reject }); - writeMessage(req); - // simple timeout - const to = setTimeout(() => { - if (pending.has(id)) { - pending.delete(id); - reject(new Error(`Request timed out: ${method}`)); - } - }, 5000); - // wrap resolve to clear timeout - const origResolve = resolve; - resolve = value => { - clearTimeout(to); - origResolve(value); - }; - }); -} - -function handleMessage(msg) { - if (msg.method && !msg.id) { - console.error("<- notification", msg.method, msg.params || ""); - return; - } - if (msg.id !== undefined && (msg.result !== undefined || msg.error !== undefined)) { - const waiter = pending.get(msg.id); - if (waiter) { - pending.delete(msg.id); - if (msg.error) waiter.reject(new Error(msg.error.message || JSON.stringify(msg.error))); - else waiter.resolve(msg.result); - } else { - console.error("<- response with unknown id", msg.id); - } - return; - } - console.error("<- unexpected message", msg); -} - -child.stdout.on("data", chunk => { - stdoutBuffer = Buffer.concat([stdoutBuffer, chunk]); - while (true) { - const newlineIndex = stdoutBuffer.indexOf("\n"); - if (newlineIndex === -1) break; - - const line = stdoutBuffer.slice(0, newlineIndex).toString("utf8").replace(/\r$/, ""); - stdoutBuffer = stdoutBuffer.slice(newlineIndex + 1); - - if (line.trim() === "") continue; // Skip empty lines - - let parsed = null; - try { - parsed = JSON.parse(line); - } catch (e) { - console.error("Failed to parse server message", e); - continue; - } - handleMessage(parsed); - } -}); -child.stderr.on("data", d => { - process.stderr.write("[server] " + d.toString()); -}); -child.on("exit", (code, sig) => { - console.error("server exited", code, sig); -}); - -(async () => { - try { - console.error("Starting MCP client -> spawning server at", serverPath); - const init = await sendRequest("initialize", { - clientInfo: { name: "mcp-stdio-client", version: "0.1.0" }, - protocolVersion: "2024-11-05", - }); - console.error("initialize ->", init); - const toolsList = await sendRequest("tools/list", {}); - console.error("tools/list ->", toolsList); - for (const toolCall of toolCalls) { - const { type, ...args } = toolCall; - console.error("Calling tool:", type, args); - try { - const res = await sendRequest("tools/call", { - name: type, - arguments: args, - }); - console.error("tools/call ->", res); - } catch (err) { - console.error("tools/call error for", type, err); - } - } - - // Clean up: give server a moment to flush, then exit - setTimeout(() => { - try { - child.kill(); - } catch (e) {} - process.exit(0); - }, 200); - } catch (e) { - console.error("Error in MCP client:", e); - try { - child.kill(); - } catch (e) {} - process.exit(1); - } -})(); diff --git a/actions/setup/js/safe_outputs_mcp_client.test.cjs b/actions/setup/js/safe_outputs_mcp_client.test.cjs deleted file mode 100644 index 90601eac816..00000000000 --- a/actions/setup/js/safe_outputs_mcp_client.test.cjs +++ /dev/null @@ -1,105 +0,0 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; -describe("safe_outputs_mcp_client.cjs", () => { - (describe("JSONL parsing", () => { - (it("should parse simple JSONL input", () => { - const result = '{"key":"value1"}\n{"key":"value2"}' - .split(/\r?\n/) - .map(l => l.trim()) - .filter(Boolean) - .map(line => JSON.parse(line)); - (expect(result).toHaveLength(2), expect(result[0]).toEqual({ key: "value1" }), expect(result[1]).toEqual({ key: "value2" })); - }), - it("should handle empty input", () => { - const parseJsonl = input => - input - ? input - .split(/\r?\n/) - .map(l => l.trim()) - .filter(Boolean) - .map(line => JSON.parse(line)) - : []; - (expect(parseJsonl("")).toEqual([]), expect(parseJsonl(null)).toEqual([]), expect(parseJsonl(void 0)).toEqual([])); - }), - it("should skip empty lines", () => { - const result = '{"key":"value1"}\n\n\n{"key":"value2"}\n' - .split(/\r?\n/) - .map(l => l.trim()) - .filter(Boolean) - .map(line => JSON.parse(line)); - expect(result).toHaveLength(2); - }), - it("should handle Windows line endings", () => { - const result = '{"key":"value1"}\r\n{"key":"value2"}\r\n' - .split(/\r?\n/) - .map(l => l.trim()) - .filter(Boolean) - .map(line => JSON.parse(line)); - expect(result).toHaveLength(2); - }), - it("should handle whitespace in lines", () => { - const result = ' {"key":"value1"} \n {"key":"value2"} ' - .split(/\r?\n/) - .map(l => l.trim()) - .filter(Boolean) - .map(line => JSON.parse(line)); - (expect(result).toHaveLength(2), expect(result[0]).toEqual({ key: "value1" })); - })); - }), - describe("message structure", () => { - (it("should create valid JSON-RPC request", () => { - const request = { jsonrpc: "2.0", id: 1, method: "test_method", params: { arg: "value" } }; - (expect(request).toHaveProperty("jsonrpc", "2.0"), expect(request).toHaveProperty("id", 1), expect(request).toHaveProperty("method", "test_method"), expect(request).toHaveProperty("params")); - }), - it("should handle notification messages (no id)", () => { - const isNotification = msg => msg.method && !msg.id; - (expect(isNotification({ method: "notify", params: {} })).toBe(!0), expect(isNotification({ jsonrpc: "2.0", id: 1, method: "request", params: {} })).toBe(!1)); - }), - it("should identify response messages", () => { - const isResponse = msg => void 0 !== msg.id && (void 0 !== msg.result || void 0 !== msg.error); - (expect(isResponse({ id: 1, result: { data: "test" } })).toBe(!0), expect(isResponse({ id: 2, error: { message: "error" } })).toBe(!0), expect(isResponse({ id: 3, method: "test" })).toBe(!1)); - })); - }), - describe("error handling", () => { - (it("should handle JSON parse errors gracefully", () => { - const parseLine = line => { - try { - return { success: !0, data: JSON.parse(line) }; - } catch (e) { - return { success: !1, error: e.message }; - } - }; - (expect(parseLine('{"key":"value"}').success).toBe(!0), expect(parseLine("{invalid json}").success).toBe(!1)); - }), - it("should handle error responses", () => { - const handleResponse = (msg, pending) => (msg.error ? new Error(msg.error.message || JSON.stringify(msg.error)) : msg.result), - errorResult = handleResponse({ id: 1, error: { message: "test error" } }); - (expect(errorResult).toBeInstanceOf(Error), expect(errorResult.message).toBe("test error")); - const successResult = handleResponse({ id: 2, result: { data: "success" } }); - expect(successResult).toEqual({ data: "success" }); - })); - }), - describe("server path validation", () => { - it("should construct valid server path", () => { - const serverPath = require("path").join("/tmp/gh-aw/safeoutputs/mcp-server.cjs"); - (expect(serverPath).toContain("mcp-server.cjs"), expect(serverPath).toContain("safeoutputs")); - }); - }), - describe("buffer handling", () => { - (it("should handle line extraction from buffer", () => { - let buffer = Buffer.from('{"key":"value"}\n{"key2":"value2"}'); - const newlineIndex = buffer.indexOf("\n"); - expect(newlineIndex).toBeGreaterThan(-1); - const line = buffer.slice(0, newlineIndex).toString("utf8"), - remaining = buffer.slice(newlineIndex + 1); - (expect(line).toBe('{"key":"value"}'), expect(remaining.toString()).toBe('{"key2":"value2"}')); - }), - it("should handle buffer without newline", () => { - const newlineIndex = Buffer.from('{"key":"value"}').indexOf("\n"); - expect(newlineIndex).toBe(-1); - }), - it("should handle empty buffer", () => { - const buffer = Buffer.alloc(0); - expect(buffer.length).toBe(0); - })); - })); -}); diff --git a/actions/setup/js/update_runner.cjs b/actions/setup/js/update_runner.cjs deleted file mode 100644 index 68d0721bf4e..00000000000 --- a/actions/setup/js/update_runner.cjs +++ /dev/null @@ -1,449 +0,0 @@ -// @ts-check -/// - -/** - * Shared update runner for safe-output scripts (update_issue, update_pull_request, etc.) - * - * This module depends on GitHub Actions environment globals provided by actions/github-script: - * - core: @actions/core module for logging and outputs - * - github: @octokit/rest instance for GitHub API calls - * - context: GitHub Actions context with event payload and repository info - * - * @module update_runner - */ - -const { loadAgentOutput } = require("./load_agent_output.cjs"); -const { generateStagedPreview } = require("./staged_preview.cjs"); -const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_title.cjs"); -const { getErrorMessage } = require("./error_helpers.cjs"); -const { sanitizeContent } = require("./sanitize_content.cjs"); - -/** - * @typedef {Object} UpdateRunnerConfig - * @property {string} itemType - Type of item in agent output (e.g., "update_issue", "update_pull_request") - * @property {string} displayName - Human-readable name (e.g., "issue", "pull request") - * @property {string} displayNamePlural - Human-readable plural name (e.g., "issues", "pull requests") - * @property {string} numberField - Field name for explicit number (e.g., "issue_number", "pull_request_number") - * @property {string} outputNumberKey - Output key for number (e.g., "issue_number", "pull_request_number") - * @property {string} outputUrlKey - Output key for URL (e.g., "issue_url", "pull_request_url") - * @property {(eventName: string, payload: any) => boolean} isValidContext - Function to check if context is valid - * @property {(payload: any) => number|undefined} getContextNumber - Function to get number from context payload - * @property {boolean} supportsStatus - Whether this type supports status updates - * @property {boolean} supportsOperation - Whether this type supports operation (append/prepend/replace) - * @property {(item: any, index: number) => string} renderStagedItem - Function to render item for staged preview - * @property {(github: any, context: any, targetNumber: number, updateData: any, handlerConfig?: any) => Promise} executeUpdate - Function to execute the update API call - * @property {(result: any) => string} getSummaryLine - Function to generate summary line for an updated item - * @property {any} [handlerConfig] - Optional handler configuration passed to executeUpdate - */ - -/** - * Resolve the target number for an update operation - * @param {Object} params - Resolution parameters - * @param {string} params.updateTarget - Target configuration ("triggering", "*", or explicit number) - * @param {any} params.item - Update item with optional explicit number field - * @param {string} params.numberField - Field name for explicit number - * @param {boolean} params.isValidContext - Whether current context is valid - * @param {number|undefined} params.contextNumber - Number from triggering context - * @param {string} params.displayName - Display name for error messages - * @returns {{success: true, number: number} | {success: false, error: string}} - */ -function resolveTargetNumber(params) { - const { updateTarget, item, numberField, isValidContext, contextNumber, displayName } = params; - - if (updateTarget === "*") { - // For target "*", we need an explicit number from the update item - const explicitNumber = item[numberField]; - if (explicitNumber) { - const parsed = parseInt(explicitNumber, 10); - if (isNaN(parsed) || parsed <= 0) { - return { success: false, error: `Invalid ${numberField} specified: ${explicitNumber}` }; - } - return { success: true, number: parsed }; - } else { - return { success: false, error: `Target is "*" but no ${numberField} specified in update item` }; - } - } else if (updateTarget && updateTarget !== "triggering") { - // Explicit number specified in target - const parsed = parseInt(updateTarget, 10); - if (isNaN(parsed) || parsed <= 0) { - return { success: false, error: `Invalid ${displayName} number in target configuration: ${updateTarget}` }; - } - return { success: true, number: parsed }; - } else { - // Default behavior: use triggering context - if (isValidContext && contextNumber) { - return { success: true, number: contextNumber }; - } - return { success: false, error: `Could not determine ${displayName} number` }; - } -} - -/** - * Build update data based on allowed fields and provided values - * @param {Object} params - Build parameters - * @param {any} params.item - Update item with field values - * @param {boolean} params.canUpdateStatus - Whether status updates are allowed - * @param {boolean} params.canUpdateTitle - Whether title updates are allowed - * @param {boolean} params.canUpdateBody - Whether body updates are allowed - * @param {boolean} [params.canUpdateLabels] - Whether label updates are allowed - * @param {boolean} params.supportsStatus - Whether this type supports status - * @returns {{hasUpdates: boolean, updateData: any, logMessages: string[]}} - */ -function buildUpdateData(params) { - const { item, canUpdateStatus, canUpdateTitle, canUpdateBody, canUpdateLabels, supportsStatus } = params; - - /** @type {any} */ - const updateData = {}; - let hasUpdates = false; - const logMessages = []; - - // Handle status update (only for types that support it, like issues) - if (supportsStatus && canUpdateStatus && item.status !== undefined) { - if (item.status === "open" || item.status === "closed") { - updateData.state = item.status; - hasUpdates = true; - logMessages.push(`Will update status to: ${item.status}`); - } else { - logMessages.push(`Invalid status value: ${item.status}. Must be 'open' or 'closed'`); - } - } - - // Handle title update - let titleForDedup = null; - if (canUpdateTitle && item.title !== undefined) { - const trimmedTitle = typeof item.title === "string" ? item.title.trim() : ""; - if (trimmedTitle.length > 0) { - updateData.title = trimmedTitle; - titleForDedup = trimmedTitle; - hasUpdates = true; - logMessages.push(`Will update title to: ${trimmedTitle}`); - } else { - logMessages.push("Invalid title value: must be a non-empty string"); - } - } - - // Handle body update (with title deduplication) - if (canUpdateBody && item.body !== undefined) { - if (typeof item.body === "string") { - let processedBody = item.body; - - // If we're updating the title at the same time, remove duplicate title from body - if (titleForDedup) { - processedBody = removeDuplicateTitleFromDescription(titleForDedup, processedBody); - } - - // Sanitize content to prevent injection attacks - processedBody = sanitizeContent(processedBody); - - updateData.body = processedBody; - hasUpdates = true; - logMessages.push(`Will update body (length: ${processedBody.length})`); - } else { - logMessages.push("Invalid body value: must be a string"); - } - } - - // Handle labels update - if (canUpdateLabels && item.labels !== undefined) { - if (Array.isArray(item.labels)) { - updateData.labels = item.labels; - hasUpdates = true; - logMessages.push(`Will update labels to: ${item.labels.join(", ")}`); - } else { - logMessages.push("Invalid labels value: must be an array"); - } - } - - return { hasUpdates, updateData, logMessages }; -} - -/** - * Run the update workflow with the provided configuration - * @param {UpdateRunnerConfig} config - Configuration for the update runner - * @returns {Promise} Array of updated items or undefined - */ -async function runUpdateWorkflow(config) { - const { - itemType, - displayName, - displayNamePlural, - numberField, - outputNumberKey, - outputUrlKey, - isValidContext, - getContextNumber, - supportsStatus, - supportsOperation, - renderStagedItem, - executeUpdate, - getSummaryLine, - handlerConfig = {}, - } = config; - - // Check if we're in staged mode - const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; - - const result = loadAgentOutput(); - if (!result.success) { - return; - } - - // Find all update items - const updateItems = result.items.filter(/** @param {any} item */ item => item.type === itemType); - if (updateItems.length === 0) { - core.info(`No ${itemType} items found in agent output`); - return; - } - - core.info(`Found ${updateItems.length} ${itemType} item(s)`); - - // If in staged mode, emit 🎭 Staged Mode Preview via generateStagedPreview - if (isStaged) { - await generateStagedPreview({ - title: `Update ${displayNamePlural.charAt(0).toUpperCase() + displayNamePlural.slice(1)}`, - description: `The following ${displayName} updates would be applied if staged mode was disabled:`, - items: updateItems, - renderItem: renderStagedItem, - }); - return; - } - - // Get the configuration from handler config object - const updateTarget = handlerConfig.target || "triggering"; - const canUpdateStatus = handlerConfig.allow_status === true; - const canUpdateTitle = handlerConfig.allow_title === true; - const canUpdateBody = handlerConfig.allow_body === true; - const canUpdateLabels = handlerConfig.allow_labels === true; - - core.info(`Update target configuration: ${updateTarget}`); - if (supportsStatus) { - core.info(`Can update status: ${canUpdateStatus}, title: ${canUpdateTitle}, body: ${canUpdateBody}, labels: ${canUpdateLabels}`); - } else { - core.info(`Can update title: ${canUpdateTitle}, body: ${canUpdateBody}, labels: ${canUpdateLabels}`); - } - - // Check context validity - const contextIsValid = isValidContext(context.eventName, context.payload); - const contextNumber = getContextNumber(context.payload); - - // Validate context based on target configuration - if (updateTarget === "triggering" && !contextIsValid) { - core.info(`Target is "triggering" but not running in ${displayName} context, skipping ${displayName} update`); - return; - } - - const updatedItems = []; - - // Process each update item - for (let i = 0; i < updateItems.length; i++) { - const updateItem = updateItems[i]; - core.info(`Processing ${itemType} item ${i + 1}/${updateItems.length}`); - - // Resolve target number - const targetResult = resolveTargetNumber({ - updateTarget, - item: updateItem, - numberField, - isValidContext: contextIsValid, - contextNumber, - displayName, - }); - - if (!targetResult.success) { - core.info(targetResult.error); - continue; - } - - const targetNumber = targetResult.number; - core.info(`Updating ${displayName} #${targetNumber}`); - - // Build update data - const { hasUpdates, updateData, logMessages } = buildUpdateData({ - item: updateItem, - canUpdateStatus, - canUpdateTitle, - canUpdateBody, - canUpdateLabels, - supportsStatus, - }); - - // Log all messages - for (const msg of logMessages) { - core.info(msg); - } - - // Handle body operation for types that support it (like PRs with append/prepend) - if (supportsOperation && canUpdateBody && updateItem.body !== undefined && typeof updateItem.body === "string") { - // The body was already added by buildUpdateData, but we need to handle operations - // This will be handled by the executeUpdate function for PR-specific logic - updateData._operation = updateItem.operation || "append"; - updateData._rawBody = updateItem.body; - } - - if (!hasUpdates) { - core.info("No valid updates to apply for this item"); - continue; - } - - try { - // Execute the update using the provided function - const updatedItem = await executeUpdate(github, context, targetNumber, updateData, handlerConfig); - core.info(`Updated ${displayName} #${updatedItem.number}: ${updatedItem.html_url}`); - updatedItems.push(updatedItem); - - // Set output for the last updated item (for backward compatibility) - if (i === updateItems.length - 1) { - core.setOutput(outputNumberKey, updatedItem.number); - core.setOutput(outputUrlKey, updatedItem.html_url); - } - } catch (error) { - core.error(`✗ Failed to update ${displayName} #${targetNumber}: ${getErrorMessage(error)}`); - throw error; - } - } - - // Write summary for all updated items - if (updatedItems.length > 0) { - let summaryContent = `\n\n## Updated ${displayNamePlural.charAt(0).toUpperCase() + displayNamePlural.slice(1)}\n`; - for (const item of updatedItems) { - summaryContent += getSummaryLine(item); - } - await core.summary.addRaw(summaryContent).write(); - } - - core.info(`Successfully updated ${updatedItems.length} ${displayName}(s)`); - return updatedItems; -} - -/** - * @typedef {Object} RenderStagedItemConfig - * @property {string} entityName - Display name for the entity (e.g., "Issue", "Pull Request") - * @property {string} numberField - Field name for the target number (e.g., "issue_number", "pull_request_number") - * @property {string} targetLabel - Label for the target (e.g., "Target Issue:", "Target PR:") - * @property {string} currentTargetText - Text when targeting current entity (e.g., "Current issue", "Current pull request") - * @property {boolean} [includeOperation=false] - Whether to include operation field for body updates - */ - -/** - * Create a render function for staged preview items - * @param {RenderStagedItemConfig} config - Configuration for the renderer - * @returns {(item: any, index: number) => string} Render function - */ -function createRenderStagedItem(config) { - const { entityName, numberField, targetLabel, currentTargetText, includeOperation = false } = config; - - return function renderStagedItem(item, index) { - let content = `#### ${entityName} Update ${index + 1}\n`; - if (item[numberField]) { - content += `**${targetLabel}** #${item[numberField]}\n\n`; - } else { - content += `**Target:** ${currentTargetText}\n\n`; - } - - if (item.title !== undefined) { - content += `**New Title:** ${item.title}\n\n`; - } - if (item.body !== undefined) { - if (includeOperation) { - const operation = item.operation || "append"; - content += `**Operation:** ${operation}\n`; - content += `**Body Content:**\n${item.body}\n\n`; - } else { - content += `**New Body:**\n${item.body}\n\n`; - } - } - if (item.status !== undefined) { - content += `**New Status:** ${item.status}\n\n`; - } - return content; - }; -} - -/** - * @typedef {Object} SummaryLineConfig - * @property {string} entityPrefix - Prefix for the summary line (e.g., "Issue", "PR") - */ - -/** - * Create a summary line generator function - * @param {SummaryLineConfig} config - Configuration for the summary generator - * @returns {(item: any) => string} Summary line generator function - */ -function createGetSummaryLine(config) { - const { entityPrefix } = config; - - return function getSummaryLine(item) { - return `- ${entityPrefix} #${item.number}: [${item.title}](${item.html_url})\n`; - }; -} - -/** - * @typedef {Object} UpdateHandlerConfig - * @property {string} itemType - Type of item in agent output (e.g., "update_issue") - * @property {string} displayName - Human-readable name (e.g., "issue") - * @property {string} displayNamePlural - Human-readable plural name (e.g., "issues") - * @property {string} numberField - Field name for explicit number (e.g., "issue_number") - * @property {string} outputNumberKey - Output key for number (e.g., "issue_number") - * @property {string} outputUrlKey - Output key for URL (e.g., "issue_url") - * @property {string} entityName - Display name for entity (e.g., "Issue", "Pull Request") - * @property {string} entityPrefix - Prefix for summary lines (e.g., "Issue", "PR") - * @property {string} targetLabel - Label for target in staged preview (e.g., "Target Issue:") - * @property {string} currentTargetText - Text for current target (e.g., "Current issue") - * @property {boolean} supportsStatus - Whether this type supports status updates - * @property {boolean} supportsOperation - Whether this type supports operation (append/prepend/replace) - * @property {(eventName: string, payload: any) => boolean} isValidContext - Function to check if context is valid - * @property {(payload: any) => number|undefined} getContextNumber - Function to get number from context payload - * @property {(github: any, context: any, targetNumber: number, updateData: any) => Promise} executeUpdate - Function to execute the update API call - */ - -/** - * Create an update handler from configuration - * This factory function eliminates boilerplate by generating all the - * render functions, summary line generators, and the main handler - * @param {UpdateHandlerConfig} config - Handler configuration - * @returns {() => Promise} Main handler function - */ -function createUpdateHandler(config) { - // Create render function for staged preview - const renderStagedItem = createRenderStagedItem({ - entityName: config.entityName, - numberField: config.numberField, - targetLabel: config.targetLabel, - currentTargetText: config.currentTargetText, - includeOperation: config.supportsOperation, - }); - - // Create summary line generator - const getSummaryLine = createGetSummaryLine({ - entityPrefix: config.entityPrefix, - }); - - // Return the main handler function - return async function main(handlerConfig = {}) { - return await runUpdateWorkflow({ - itemType: config.itemType, - displayName: config.displayName, - displayNamePlural: config.displayNamePlural, - numberField: config.numberField, - outputNumberKey: config.outputNumberKey, - outputUrlKey: config.outputUrlKey, - isValidContext: config.isValidContext, - getContextNumber: config.getContextNumber, - supportsStatus: config.supportsStatus, - supportsOperation: config.supportsOperation, - renderStagedItem, - executeUpdate: config.executeUpdate, - getSummaryLine, - handlerConfig, // Pass handler config to the runner - }); - }; -} - -module.exports = { - runUpdateWorkflow, - resolveTargetNumber, - buildUpdateData, - createRenderStagedItem, - createGetSummaryLine, - createUpdateHandler, -}; diff --git a/actions/setup/js/update_runner.test.cjs b/actions/setup/js/update_runner.test.cjs deleted file mode 100644 index 14f8199eadb..00000000000 --- a/actions/setup/js/update_runner.test.cjs +++ /dev/null @@ -1,608 +0,0 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; - -describe("update_runner.cjs", () => { - let helpers; - - beforeEach(async () => { - helpers = await import("./update_runner.cjs"); - }); - - describe("resolveTargetNumber", () => { - it("should resolve explicit number from item with wildcard target", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "*", - item: { issue_number: 123 }, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(true); - expect(result.number).toBe(123); - }); - - it("should fail with wildcard target but no explicit number", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "*", - item: {}, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(false); - expect(result.error).toContain('Target is "*"'); - expect(result.error).toContain("issue_number"); - }); - - it("should fail with wildcard target and invalid number", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "*", - item: { issue_number: "invalid" }, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(false); - expect(result.error).toContain("Invalid issue_number"); - }); - - it("should fail with wildcard target and zero number", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "*", - item: { issue_number: 0 }, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(false); - // 0 is falsy, so it's treated as "no number specified" - expect(result.error).toContain("issue_number"); - }); - - it("should resolve explicit target number", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "789", - item: {}, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(true); - expect(result.number).toBe(789); - }); - - it("should fail with invalid explicit target", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "invalid", - item: {}, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(false); - expect(result.error).toContain("Invalid issue number"); - }); - - it("should fail with zero explicit target", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "0", - item: {}, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(false); - expect(result.error).toContain("Invalid issue number"); - }); - - it("should resolve from triggering context", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "triggering", - item: {}, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(true); - expect(result.number).toBe(456); - }); - - it("should resolve from triggering context when target is empty", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "", - item: {}, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(true); - expect(result.number).toBe(456); - }); - - it("should fail when triggering but no context number", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "triggering", - item: {}, - numberField: "issue_number", - isValidContext: true, - contextNumber: undefined, - displayName: "issue", - }); - expect(result.success).toBe(false); - expect(result.error).toContain("Could not determine issue number"); - }); - - it("should fail when triggering but context is invalid", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "triggering", - item: {}, - numberField: "issue_number", - isValidContext: false, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(false); - expect(result.error).toContain("Could not determine issue number"); - }); - - it("should work with pull_request_number field", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "*", - item: { pull_request_number: 999 }, - numberField: "pull_request_number", - isValidContext: true, - contextNumber: 456, - displayName: "pull request", - }); - expect(result.success).toBe(true); - expect(result.number).toBe(999); - }); - - it("should handle string number in item", () => { - const result = helpers.resolveTargetNumber({ - updateTarget: "*", - item: { issue_number: "123" }, - numberField: "issue_number", - isValidContext: true, - contextNumber: 456, - displayName: "issue", - }); - expect(result.success).toBe(true); - expect(result.number).toBe(123); - }); - }); - - describe("buildUpdateData", () => { - it("should include status when allowed and valid", () => { - const result = helpers.buildUpdateData({ - item: { status: "closed" }, - canUpdateStatus: true, - canUpdateTitle: false, - canUpdateBody: false, - supportsStatus: true, - }); - expect(result.hasUpdates).toBe(true); - expect(result.updateData.state).toBe("closed"); - expect(result.logMessages).toContain("Will update status to: closed"); - }); - - it("should include status open when allowed and valid", () => { - const result = helpers.buildUpdateData({ - item: { status: "open" }, - canUpdateStatus: true, - canUpdateTitle: false, - canUpdateBody: false, - supportsStatus: true, - }); - expect(result.hasUpdates).toBe(true); - expect(result.updateData.state).toBe("open"); - }); - - it("should reject invalid status value", () => { - const result = helpers.buildUpdateData({ - item: { status: "invalid" }, - canUpdateStatus: true, - canUpdateTitle: false, - canUpdateBody: false, - supportsStatus: true, - }); - expect(result.hasUpdates).toBe(false); - expect(result.updateData.state).toBeUndefined(); - expect(result.logMessages.some(m => m.includes("Invalid status value"))).toBe(true); - }); - - it("should skip status when not allowed", () => { - const result = helpers.buildUpdateData({ - item: { status: "closed" }, - canUpdateStatus: false, - canUpdateTitle: false, - canUpdateBody: false, - supportsStatus: true, - }); - expect(result.hasUpdates).toBe(false); - expect(result.updateData.state).toBeUndefined(); - }); - - it("should skip status when not supported", () => { - const result = helpers.buildUpdateData({ - item: { status: "closed" }, - canUpdateStatus: true, - canUpdateTitle: false, - canUpdateBody: false, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(false); - expect(result.updateData.state).toBeUndefined(); - }); - - it("should include title when allowed and valid", () => { - const result = helpers.buildUpdateData({ - item: { title: "New Title" }, - canUpdateStatus: false, - canUpdateTitle: true, - canUpdateBody: false, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(true); - expect(result.updateData.title).toBe("New Title"); - expect(result.logMessages).toContain("Will update title to: New Title"); - }); - - it("should trim title whitespace", () => { - const result = helpers.buildUpdateData({ - item: { title: " Trimmed Title " }, - canUpdateStatus: false, - canUpdateTitle: true, - canUpdateBody: false, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(true); - expect(result.updateData.title).toBe("Trimmed Title"); - }); - - it("should reject empty title", () => { - const result = helpers.buildUpdateData({ - item: { title: " " }, - canUpdateStatus: false, - canUpdateTitle: true, - canUpdateBody: false, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(false); - expect(result.updateData.title).toBeUndefined(); - expect(result.logMessages.some(m => m.includes("Invalid title value"))).toBe(true); - }); - - it("should reject non-string title", () => { - const result = helpers.buildUpdateData({ - item: { title: 123 }, - canUpdateStatus: false, - canUpdateTitle: true, - canUpdateBody: false, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(false); - expect(result.updateData.title).toBeUndefined(); - }); - - it("should skip title when not allowed", () => { - const result = helpers.buildUpdateData({ - item: { title: "New Title" }, - canUpdateStatus: false, - canUpdateTitle: false, - canUpdateBody: false, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(false); - expect(result.updateData.title).toBeUndefined(); - }); - - it("should include body when allowed and valid", () => { - const result = helpers.buildUpdateData({ - item: { body: "New body content" }, - canUpdateStatus: false, - canUpdateTitle: false, - canUpdateBody: true, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(true); - expect(result.updateData.body).toBe("New body content"); - expect(result.logMessages.some(m => m.includes("Will update body"))).toBe(true); - }); - - it("should include empty string body", () => { - const result = helpers.buildUpdateData({ - item: { body: "" }, - canUpdateStatus: false, - canUpdateTitle: false, - canUpdateBody: true, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(true); - expect(result.updateData.body).toBe(""); - }); - - it("should reject non-string body", () => { - const result = helpers.buildUpdateData({ - item: { body: 123 }, - canUpdateStatus: false, - canUpdateTitle: false, - canUpdateBody: true, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(false); - expect(result.updateData.body).toBeUndefined(); - expect(result.logMessages.some(m => m.includes("Invalid body value"))).toBe(true); - }); - - it("should skip body when not allowed", () => { - const result = helpers.buildUpdateData({ - item: { body: "New body" }, - canUpdateStatus: false, - canUpdateTitle: false, - canUpdateBody: false, - supportsStatus: false, - }); - expect(result.hasUpdates).toBe(false); - expect(result.updateData.body).toBeUndefined(); - }); - - it("should handle multiple fields", () => { - const result = helpers.buildUpdateData({ - item: { title: "New Title", body: "New body", status: "closed" }, - canUpdateStatus: true, - canUpdateTitle: true, - canUpdateBody: true, - supportsStatus: true, - }); - expect(result.hasUpdates).toBe(true); - expect(result.updateData.title).toBe("New Title"); - expect(result.updateData.body).toBe("New body"); - expect(result.updateData.state).toBe("closed"); - expect(result.logMessages.length).toBe(3); - }); - - it("should return false hasUpdates when no valid updates", () => { - const result = helpers.buildUpdateData({ - item: {}, - canUpdateStatus: true, - canUpdateTitle: true, - canUpdateBody: true, - supportsStatus: true, - }); - expect(result.hasUpdates).toBe(false); - expect(Object.keys(result.updateData).length).toBe(0); - }); - - it("should not include fields when item value is undefined", () => { - const result = helpers.buildUpdateData({ - item: { title: undefined, body: undefined, status: undefined }, - canUpdateStatus: true, - canUpdateTitle: true, - canUpdateBody: true, - supportsStatus: true, - }); - expect(result.hasUpdates).toBe(false); - }); - }); - - describe("createRenderStagedItem", () => { - it("should render issue update with explicit number", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Issue", - numberField: "issue_number", - targetLabel: "Target Issue:", - currentTargetText: "Current issue", - includeOperation: false, - }); - - const result = render({ issue_number: 123, title: "New Title" }, 0); - - expect(result).toContain("#### Issue Update 1"); - expect(result).toContain("**Target Issue:** #123"); - expect(result).toContain("**New Title:** New Title"); - }); - - it("should render issue update without explicit number", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Issue", - numberField: "issue_number", - targetLabel: "Target Issue:", - currentTargetText: "Current issue", - includeOperation: false, - }); - - const result = render({ title: "New Title" }, 0); - - expect(result).toContain("#### Issue Update 1"); - expect(result).toContain("**Target:** Current issue"); - expect(result).toContain("**New Title:** New Title"); - }); - - it("should render PR update with operation field", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Pull Request", - numberField: "pull_request_number", - targetLabel: "Target PR:", - currentTargetText: "Current pull request", - includeOperation: true, - }); - - const result = render({ pull_request_number: 456, body: "New body content", operation: "prepend" }, 0); - - expect(result).toContain("### Pull Request Update 1"); - expect(result).toContain("**Target PR:** #456"); - expect(result).toContain("**Operation:** prepend"); - expect(result).toContain("**Body Content:**\nNew body content"); - }); - - it("should render body without operation when includeOperation is false", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Issue", - numberField: "issue_number", - targetLabel: "Target Issue:", - currentTargetText: "Current issue", - includeOperation: false, - }); - - const result = render({ body: "New body content" }, 0); - - expect(result).toContain("**New Body:**\nNew body content"); - expect(result).not.toContain("**Operation:"); - }); - - it("should render status when present", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Issue", - numberField: "issue_number", - targetLabel: "Target Issue:", - currentTargetText: "Current issue", - includeOperation: false, - }); - - const result = render({ status: "closed" }, 0); - - expect(result).toContain("**New Status:** closed"); - }); - - it("should use default operation when includeOperation is true but operation not specified", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Pull Request", - numberField: "pull_request_number", - targetLabel: "Target PR:", - currentTargetText: "Current pull request", - includeOperation: true, - }); - - const result = render({ body: "New body content" }, 0); - - expect(result).toContain("**Operation:** append"); - expect(result).toContain("**Body Content:**\nNew body content"); - }); - - it("should increment index correctly", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Issue", - numberField: "issue_number", - targetLabel: "Target Issue:", - currentTargetText: "Current issue", - includeOperation: false, - }); - - const result = render({ title: "Title" }, 4); - - expect(result).toContain("#### Issue Update 5"); - }); - }); - - describe("createGetSummaryLine", () => { - it("should generate issue summary line", () => { - const getSummaryLine = helpers.createGetSummaryLine({ - entityPrefix: "Issue", - }); - - const result = getSummaryLine({ - number: 123, - title: "Test Issue", - html_url: "https://github.com/owner/repo/issues/123", - }); - - expect(result).toBe("- Issue #123: [Test Issue](https://github.com/owner/repo/issues/123)\n"); - }); - - it("should generate PR summary line", () => { - const getSummaryLine = helpers.createGetSummaryLine({ - entityPrefix: "PR", - }); - - const result = getSummaryLine({ - number: 456, - title: "Test PR", - html_url: "https://github.com/owner/repo/pull/456", - }); - - expect(result).toBe("- PR #456: [Test PR](https://github.com/owner/repo/pull/456)\n"); - }); - - it("should handle special characters in title", () => { - const getSummaryLine = helpers.createGetSummaryLine({ - entityPrefix: "Issue", - }); - - const result = getSummaryLine({ - number: 789, - title: "Fix [bug] with chars", - html_url: "https://github.com/owner/repo/issues/789", - }); - - expect(result).toBe("- Issue #789: [Fix [bug] with chars](https://github.com/owner/repo/issues/789)\n"); - }); - }); - - describe("Default operation behavior", () => { - it("should default to append when operation is not specified in renderStagedItem", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Pull Request", - numberField: "pull_request_number", - targetLabel: "Target PR:", - currentTargetText: "Current pull request", - includeOperation: true, - }); - - const result = render({ body: "New body content" }, 0); - - // Should show "append" as the default operation - expect(result).toContain("**Operation:** append"); - }); - - it("should respect explicit append operation in renderStagedItem", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Pull Request", - numberField: "pull_request_number", - targetLabel: "Target PR:", - currentTargetText: "Current pull request", - includeOperation: true, - }); - - const result = render({ body: "New body content", operation: "append" }, 0); - - expect(result).toContain("**Operation:** append"); - }); - - it("should respect explicit prepend operation in renderStagedItem", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Pull Request", - numberField: "pull_request_number", - targetLabel: "Target PR:", - currentTargetText: "Current pull request", - includeOperation: true, - }); - - const result = render({ body: "New body content", operation: "prepend" }, 0); - - expect(result).toContain("**Operation:** prepend"); - }); - - it("should respect explicit replace operation in renderStagedItem", () => { - const render = helpers.createRenderStagedItem({ - entityName: "Pull Request", - numberField: "pull_request_number", - targetLabel: "Target PR:", - currentTargetText: "Current pull request", - includeOperation: true, - }); - - const result = render({ body: "New body content", operation: "replace" }, 0); - - expect(result).toContain("**Operation:** replace"); - }); - }); -});