diff --git a/actions/setup/js/create_issue.cjs b/actions/setup/js/create_issue.cjs index ffae1f4ec31..7efe70c1629 100644 --- a/actions/setup/js/create_issue.cjs +++ b/actions/setup/js/create_issue.cjs @@ -340,7 +340,7 @@ async function main(config = {}) { } else { // Check if it looks like a malformed temporary ID if (parentWithoutHash.startsWith("aw_")) { - core.warning(`Invalid temporary ID format for parent: '${message.parent}'. Temporary IDs must be in format 'aw_' followed by 3 to 8 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_Test123'`); + core.warning(`Invalid temporary ID format for parent: '${message.parent}'. Temporary IDs must be in format 'aw_' followed by 3 to 12 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_Test123'`); } else { // It's a real issue number const parsed = parseInt(parentWithoutHash, 10); diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 4ac8dc389c8..e17736c9112 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -211,11 +211,11 @@ async function main(config = {}) { if (!isTemporaryId(normalized)) { core.warning( - `Skipping create_pull_request: Invalid temporary_id format: '${pullRequestItem.temporary_id}'. Temporary IDs must be in format 'aw_' followed by 3 to 8 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_Test123'` + `Skipping create_pull_request: Invalid temporary_id format: '${pullRequestItem.temporary_id}'. Temporary IDs must be in format 'aw_' followed by 3 to 12 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_Test123'` ); return { success: false, - error: `Invalid temporary_id format: '${pullRequestItem.temporary_id}'. Temporary IDs must be in format 'aw_' followed by 3 to 8 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_Test123'`, + error: `Invalid temporary_id format: '${pullRequestItem.temporary_id}'. Temporary IDs must be in format 'aw_' followed by 3 to 12 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_Test123'`, }; } diff --git a/actions/setup/js/safe_output_topological_sort.test.cjs b/actions/setup/js/safe_output_topological_sort.test.cjs index cd70013c762..adcf637b981 100644 --- a/actions/setup/js/safe_output_topological_sort.test.cjs +++ b/actions/setup/js/safe_output_topological_sort.test.cjs @@ -483,7 +483,7 @@ describe("safe_output_topological_sort.cjs", () => { messages.push({ type: "create_issue", temporary_id: rootId, title: "Root" }); for (let i = 0; i < 10; i++) { - const level1Id = `aw_lv1_${i.toString().padStart(7, "0")}`; + const level1Id = `aw_lv1n${i.toString().padStart(7, "0")}`; messages.push({ type: "create_issue", temporary_id: level1Id, @@ -491,7 +491,7 @@ describe("safe_output_topological_sort.cjs", () => { }); for (let j = 0; j < 5; j++) { - const level2Id = `aw_lv2_${i}_${j.toString().padStart(4, "0")}`; + const level2Id = `aw_lv2n${i}n${j.toString().padStart(4, "0")}`; messages.push({ type: "create_issue", temporary_id: level2Id, @@ -507,11 +507,11 @@ describe("safe_output_topological_sort.cjs", () => { // Verify each level-1 item comes before its level-2 children for (let i = 0; i < 10; i++) { - const level1Id = `aw_lv1_${i.toString().padStart(7, "0")}`; + 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_lv2_${i}_${j.toString().padStart(4, "0")}`; + const level2Id = `aw_lv2n${i}n${j.toString().padStart(4, "0")}`; const level2Index = sorted.findIndex(m => m.temporary_id === level2Id); expect(level1Index).toBeLessThan(level2Index); } @@ -520,7 +520,7 @@ describe("safe_output_topological_sort.cjs", () => { // 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_lv1_${i.toString().padStart(7, "0")}`; + const level1Id = `aw_lv1n${i.toString().padStart(7, "0")}`; const level1Index = sorted.findIndex(m => m.temporary_id === level1Id); expect(rootIndex).toBeLessThan(level1Index); } diff --git a/actions/setup/js/safe_output_unified_handler_manager.cjs b/actions/setup/js/safe_output_unified_handler_manager.cjs index 67992685397..ad42acbb01b 100644 --- a/actions/setup/js/safe_output_unified_handler_manager.cjs +++ b/actions/setup/js/safe_output_unified_handler_manager.cjs @@ -759,7 +759,7 @@ function normalizeAndValidateTemporaryId(message, messageType, messageIndex) { const withoutHash = trimmed.startsWith("#") ? trimmed.substring(1).trim() : trimmed; if (!isTemporaryId(withoutHash)) { - throw new Error(`${ERR_VALIDATION}: Message ${messageIndex + 1} (${messageType}): invalid temporary_id '${raw}'. Temporary IDs must be 'aw_' followed by 3 to 8 alphanumeric characters (A-Za-z0-9), e.g. 'aw_abc' or 'aw_Test123'`); + throw new Error(`${ERR_VALIDATION}: Message ${messageIndex + 1} (${messageType}): invalid temporary_id '${raw}'. Temporary IDs must be 'aw_' followed by 3 to 12 alphanumeric characters (A-Za-z0-9), e.g. 'aw_abc' or 'aw_Test123'`); } // Normalize to the strict bare ID to keep lookups consistent. diff --git a/actions/setup/js/temporary_id.cjs b/actions/setup/js/temporary_id.cjs index 20c260b7fae..7d1fc201a2c 100644 --- a/actions/setup/js/temporary_id.cjs +++ b/actions/setup/js/temporary_id.cjs @@ -25,9 +25,15 @@ const crypto = require("crypto"); /** * Regex pattern for matching temporary ID references in text - * Format: #aw_XXX to #aw_XXXXXXXX (aw_ prefix + 3 to 8 alphanumeric characters) + * Format: #aw_XXX to #aw_XXXXXXXXXXXX (aw_ prefix + 3 to 12 alphanumeric characters) */ -const TEMPORARY_ID_PATTERN = /#(aw_[A-Za-z0-9]{3,8})/gi; +const TEMPORARY_ID_PATTERN = /#(aw_[A-Za-z0-9]{3,12})\b/gi; + +/** + * Regex pattern for detecting candidate #aw_ references (any length, word-boundary delimited) + * Used to identify malformed temporary ID references that don't match TEMPORARY_ID_PATTERN + */ +const TEMPORARY_ID_CANDIDATE_PATTERN = /#aw_([A-Za-z0-9]+)\b/gi; /** * @typedef {Object} RepoIssuePair @@ -51,13 +57,13 @@ function generateTemporaryId() { } /** - * Check if a value is a valid temporary ID (aw_ prefix + 3 to 8 alphanumeric characters) + * Check if a value is a valid temporary ID (aw_ prefix + 3 to 12 alphanumeric characters) * @param {any} value - The value to check * @returns {boolean} True if the value is a valid temporary ID */ function isTemporaryId(value) { if (typeof value === "string") { - return /^aw_[A-Za-z0-9]{3,8}$/i.test(value); + return /^aw_[A-Za-z0-9]{3,12}$/i.test(value); } return false; } @@ -80,6 +86,16 @@ function normalizeTemporaryId(tempId) { * @returns {string} Text with temporary IDs replaced with issue numbers */ function replaceTemporaryIdReferences(text, tempIdMap, currentRepo) { + // Detect and warn about malformed #aw_ references that won't be resolved + let candidate; + TEMPORARY_ID_CANDIDATE_PATTERN.lastIndex = 0; + while ((candidate = TEMPORARY_ID_CANDIDATE_PATTERN.exec(text)) !== null) { + const tempId = `aw_${candidate[1]}`; + if (!isTemporaryId(tempId)) { + core.warning(`Malformed temporary ID reference '${candidate[0]}' found in body text. Temporary IDs must be in format '#aw_' followed by 3 to 12 alphanumeric characters (A-Za-z0-9). Example: '#aw_abc' or '#aw_Test123'`); + } + } + return text.replace(TEMPORARY_ID_PATTERN, (match, tempId) => { const resolved = tempIdMap.get(normalizeTemporaryId(tempId)); if (resolved !== undefined) { @@ -146,7 +162,7 @@ function getOrGenerateTemporaryId(message, entityType = "item") { if (!isTemporaryId(normalized)) { return { temporaryId: null, - error: `Invalid temporary_id format: '${message.temporary_id}'. Temporary IDs must be in format 'aw_' followed by 3 to 8 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_Test123'`, + error: `Invalid temporary_id format: '${message.temporary_id}'. Temporary IDs must be in format 'aw_' followed by 3 to 12 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_Test123'`, }; } @@ -282,14 +298,14 @@ function resolveIssueNumber(value, temporaryIdMap) { return { resolved: null, wasTemporaryId: false, - errorMessage: `Invalid temporary ID format: '${valueStr}'. Temporary IDs must be in format 'aw_' followed by 3 to 8 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_abc12345'`, + errorMessage: `Invalid temporary ID format: '${valueStr}'. Temporary IDs must be in format 'aw_' followed by 3 to 12 alphanumeric characters (A-Za-z0-9). Example: 'aw_abc' or 'aw_abc12345'`, }; } // It's a real issue number - use context repo as default const issueNumber = typeof value === "number" ? value : parseInt(valueWithoutHash, 10); if (isNaN(issueNumber) || issueNumber <= 0) { - return { resolved: null, wasTemporaryId: false, errorMessage: `Invalid issue number: ${value}. Expected either a valid temporary ID (format: aw_ followed by 3-8 alphanumeric characters) or a numeric issue number.` }; + return { resolved: null, wasTemporaryId: false, errorMessage: `Invalid issue number: ${value}. Expected either a valid temporary ID (format: aw_ followed by 3-12 alphanumeric characters) or a numeric issue number.` }; } const contextRepo = typeof context !== "undefined" ? `${context.repo.owner}/${context.repo.repo}` : ""; @@ -527,6 +543,7 @@ function getCreatedTemporaryId(message) { module.exports = { TEMPORARY_ID_PATTERN, + TEMPORARY_ID_CANDIDATE_PATTERN, generateTemporaryId, isTemporaryId, normalizeTemporaryId, diff --git a/actions/setup/js/temporary_id.test.cjs b/actions/setup/js/temporary_id.test.cjs index 7af37ce41db..3e1c757ef04 100644 --- a/actions/setup/js/temporary_id.test.cjs +++ b/actions/setup/js/temporary_id.test.cjs @@ -38,7 +38,7 @@ describe("temporary_id.cjs", () => { }); describe("isTemporaryId", () => { - it("should return true for valid aw_ prefixed 3-8 char alphanumeric strings", async () => { + it("should return true for valid aw_ prefixed 3-12 char alphanumeric strings", async () => { const { isTemporaryId } = await import("./temporary_id.cjs"); expect(isTemporaryId("aw_abc")).toBe(true); expect(isTemporaryId("aw_abc1")).toBe(true); @@ -48,13 +48,15 @@ describe("temporary_id.cjs", () => { expect(isTemporaryId("aw_ABCD")).toBe(true); expect(isTemporaryId("aw_xyz9")).toBe(true); expect(isTemporaryId("aw_xyz")).toBe(true); + expect(isTemporaryId("aw_123456789")).toBe(true); // 9 chars - valid with extended limit + expect(isTemporaryId("aw_123456789abc")).toBe(true); // 12 chars - at the limit }); it("should return false for invalid strings", async () => { const { isTemporaryId } = await import("./temporary_id.cjs"); expect(isTemporaryId("abc123def456")).toBe(false); // Missing aw_ prefix expect(isTemporaryId("aw_ab")).toBe(false); // Too short (2 chars) - expect(isTemporaryId("aw_123456789")).toBe(false); // Too long (9 chars) + expect(isTemporaryId("aw_1234567890abc")).toBe(false); // Too long (13 chars) expect(isTemporaryId("aw_test-id")).toBe(false); // Contains hyphen expect(isTemporaryId("aw_id_123")).toBe(false); // Contains underscore expect(isTemporaryId("")).toBe(false); @@ -123,6 +125,52 @@ describe("temporary_id.cjs", () => { const text = "Check #aw_ab and #temp:abc123 for details"; expect(replaceTemporaryIdReferences(text, map, "owner/repo")).toBe("Check #aw_ab and #temp:abc123 for details"); }); + + it("should warn about malformed temporary ID reference that is too short", async () => { + const { replaceTemporaryIdReferences } = await import("./temporary_id.cjs"); + const map = new Map(); + const text = "Check #aw_ab for details"; + const result = replaceTemporaryIdReferences(text, map, "owner/repo"); + expect(result).toBe("Check #aw_ab for details"); + expect(mockCore.warning).toHaveBeenCalledOnce(); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("#aw_ab")); + }); + + it("should warn about malformed temporary ID reference that is too long", async () => { + const { replaceTemporaryIdReferences } = await import("./temporary_id.cjs"); + const map = new Map(); + const text = "Check #aw_toolongname123 for details"; + const result = replaceTemporaryIdReferences(text, map, "owner/repo"); + expect(result).toBe("Check #aw_toolongname123 for details"); + expect(mockCore.warning).toHaveBeenCalledOnce(); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("#aw_toolongname123")); + }); + + it("should not warn for valid temporary ID references", async () => { + const { replaceTemporaryIdReferences } = await import("./temporary_id.cjs"); + const map = new Map([["aw_abc123", { repo: "owner/repo", number: 100 }]]); + const text = "Check #aw_abc123 for details"; + replaceTemporaryIdReferences(text, map, "owner/repo"); + expect(mockCore.warning).not.toHaveBeenCalled(); + }); + + it("should not warn for valid unresolved temporary ID references", async () => { + const { replaceTemporaryIdReferences } = await import("./temporary_id.cjs"); + const map = new Map(); + const text = "Check #aw_abc123 for details"; + replaceTemporaryIdReferences(text, map, "owner/repo"); + expect(mockCore.warning).not.toHaveBeenCalled(); + }); + + it("should warn once per malformed reference when multiple are present", async () => { + const { replaceTemporaryIdReferences } = await import("./temporary_id.cjs"); + const map = new Map(); + const text = "See #aw_ab and #aw_toolongname123 here"; + replaceTemporaryIdReferences(text, map, "owner/repo"); + expect(mockCore.warning).toHaveBeenCalledTimes(2); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("#aw_ab")); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("#aw_toolongname123")); + }); }); describe("replaceTemporaryIdReferencesLegacy", () => { @@ -275,7 +323,7 @@ describe("temporary_id.cjs", () => { expect(result.wasTemporaryId).toBe(false); expect(result.errorMessage).toContain("Invalid temporary ID format"); expect(result.errorMessage).toContain("aw_test-id"); - expect(result.errorMessage).toContain("3 to 8 alphanumeric characters"); + expect(result.errorMessage).toContain("3 to 12 alphanumeric characters"); }); it("should return specific error for malformed temporary ID (too short)", async () => { @@ -291,11 +339,11 @@ describe("temporary_id.cjs", () => { it("should return specific error for malformed temporary ID (too long)", async () => { const { resolveIssueNumber } = await import("./temporary_id.cjs"); const map = new Map(); - const result = resolveIssueNumber("aw_abc123456789", map); + const result = resolveIssueNumber("aw_abc1234567890", map); // 13 chars - too long expect(result.resolved).toBe(null); expect(result.wasTemporaryId).toBe(false); expect(result.errorMessage).toContain("Invalid temporary ID format"); - expect(result.errorMessage).toContain("aw_abc123456789"); + expect(result.errorMessage).toContain("aw_abc1234567890"); }); it("should handle temporary ID with # prefix", async () => { diff --git a/actions/setup/js/update_project.cjs b/actions/setup/js/update_project.cjs index b176d9e17f6..bd185e9704e 100644 --- a/actions/setup/js/update_project.cjs +++ b/actions/setup/js/update_project.cjs @@ -703,11 +703,11 @@ async function updateProject(output, temporaryIdMap = new Map(), githubClient = // Validate IDs used for draft chaining. // Draft issue chaining must use strict temporary IDs to match the unified handler manager. if (temporaryId && !isTemporaryId(temporaryId)) { - throw new Error(`${ERR_VALIDATION}: Invalid temporary_id format: "${temporaryId}". Expected format: aw_ followed by 3 to 8 alphanumeric characters (e.g., "aw_abc", "aw_Test123").`); + throw new Error(`${ERR_VALIDATION}: Invalid temporary_id format: "${temporaryId}". Expected format: aw_ followed by 3 to 12 alphanumeric characters (e.g., "aw_abc", "aw_Test123").`); } if (draftIssueId && !isTemporaryId(draftIssueId)) { - throw new Error(`${ERR_VALIDATION}: Invalid draft_issue_id format: "${draftIssueId}". Expected format: aw_ followed by 3 to 8 alphanumeric characters (e.g., "aw_abc", "aw_Test123").`); + throw new Error(`${ERR_VALIDATION}: Invalid draft_issue_id format: "${draftIssueId}". Expected format: aw_ followed by 3 to 12 alphanumeric characters (e.g., "aw_abc", "aw_Test123").`); } const draftTitle = typeof output.draft_title === "string" ? output.draft_title.trim() : ""; @@ -939,7 +939,7 @@ async function updateProject(output, temporaryIdMap = new Map(), githubClient = } else { // Not a temporary ID - validate as numeric if (!/^\d+$/.test(sanitizedContentNumber)) { - throw new Error(`${ERR_VALIDATION}: Invalid content number "${rawContentNumber}". Provide a positive integer or a valid temporary ID (format: aw_ followed by 3-8 alphanumeric characters).`); + throw new Error(`${ERR_VALIDATION}: Invalid content number "${rawContentNumber}". Provide a positive integer or a valid temporary ID (format: aw_ followed by 3-12 alphanumeric characters).`); } contentNumber = Number.parseInt(sanitizedContentNumber, 10); } diff --git a/actions/setup/js/update_project.test.cjs b/actions/setup/js/update_project.test.cjs index 52b071eda7c..db1bb9af467 100644 --- a/actions/setup/js/update_project.test.cjs +++ b/actions/setup/js/update_project.test.cjs @@ -884,12 +884,12 @@ describe("updateProject", () => { project: projectUrl, content_type: "draft_issue", draft_title: "Test Draft", - temporary_id: "aw_toolong123", + temporary_id: "aw_toolong123456", }; queueResponses([repoResponse(), viewerResponse(), orgProjectV2Response(projectUrl, 60, "project-draft")]); - await expect(updateProject(output)).rejects.toThrow(/Invalid temporary_id format.*aw_ followed by 3 to 8 alphanumeric characters/); + await expect(updateProject(output)).rejects.toThrow(/Invalid temporary_id format.*aw_ followed by 3 to 12 alphanumeric characters/); }); it("rejects malformed auto-generated draft_issue_id with aw_ prefix", async () => { @@ -904,7 +904,7 @@ describe("updateProject", () => { queueResponses([repoResponse(), viewerResponse(), orgProjectV2Response(projectUrl, 60, "project-draft")]); - await expect(updateProject(output, temporaryIdMap)).rejects.toThrow(/Invalid draft_issue_id format.*aw_ followed by 3 to 8 alphanumeric characters/); + await expect(updateProject(output, temporaryIdMap)).rejects.toThrow(/Invalid draft_issue_id format.*aw_ followed by 3 to 12 alphanumeric characters/); }); it("rejects draft_issue without title when creating (no draft_issue_id)", async () => {