Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion actions/setup/js/create_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions actions/setup/js/create_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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'`,
};
}

Expand Down
10 changes: 5 additions & 5 deletions actions/setup/js/safe_output_topological_sort.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -483,15 +483,15 @@ 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,
body: `Parent: #${rootId}`,
});

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,
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion actions/setup/js/safe_output_unified_handler_manager.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
31 changes: 24 additions & 7 deletions actions/setup/js/temporary_id.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TEMPORARY_ID_CANDIDATE_PATTERN already uses \b and matches any length — no change needed there since the malformed check still delegates to the updated isTemporaryId() (89d3deb).


/**
* @typedef {Object} RepoIssuePair
Expand All @@ -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;
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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'`,
};
}

Expand Down Expand Up @@ -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}` : "";
Expand Down Expand Up @@ -527,6 +543,7 @@ function getCreatedTemporaryId(message) {

module.exports = {
TEMPORARY_ID_PATTERN,
TEMPORARY_ID_CANDIDATE_PATTERN,
generateTemporaryId,
isTemporaryId,
normalizeTemporaryId,
Expand Down
58 changes: 53 additions & 5 deletions actions/setup/js/temporary_id.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
6 changes: 3 additions & 3 deletions actions/setup/js/update_project.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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() : "";
Expand Down Expand Up @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions actions/setup/js/update_project.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
Loading