fix: address review comments — fix candidate pattern for hyphens, use shared helper in create_pull_request

Copilot · pelikhan · web-flow · commit bf2a8a1e019c · 2026-04-02T10:20:09.000Z
- TEMPORARY_ID_CANDIDATE_PATTERN now uses [A-Za-z0-9_-]+ (no \b) so #aw_test-id captures the full token 'test-id' instead of just 'test', triggering a warning as expected - create_pull_request.cjs now uses getOrGenerateTemporaryId() shared helper instead of duplicating the validation/fallback logic - Added test for hyphen-containing malformed temp ID reference Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a6bd2d2f-d14b-45b6-b5aa-61ea6b64360c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs
@@ -11,7 +11,7 @@ const { getTrackerID } = require("./get_tracker_id.cjs");
 const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_title.cjs");
 const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs");
 const { getErrorMessage } = require("./error_helpers.cjs");
-const { replaceTemporaryIdReferences, isTemporaryId, generateTemporaryId } = require("./temporary_id.cjs");
+const { replaceTemporaryIdReferences, getOrGenerateTemporaryId } = require("./temporary_id.cjs");
 const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs");
 const { addExpirationToFooter } = require("./ephemerals.cjs");
 const { generateWorkflowIdMarker } = require("./generate_footer.cjs");
@@ -247,30 +247,12 @@ async function main(config = {}) {
 
     const pullRequestItem = message;
 
-    let temporaryId;
-    if (pullRequestItem.temporary_id !== undefined && pullRequestItem.temporary_id !== null) {
-      if (typeof pullRequestItem.temporary_id !== "string") {
-        core.warning(`Skipping create_pull_request: temporary_id must be a string (got ${typeof pullRequestItem.temporary_id})`);
-        return {
-          success: false,
-          error: `temporary_id must be a string (got ${typeof pullRequestItem.temporary_id})`,
-        };
-      }
-
-      const rawTemporaryId = pullRequestItem.temporary_id.trim();
-      const normalized = rawTemporaryId.startsWith("#") ? rawTemporaryId.substring(1).trim() : rawTemporaryId;
-
-      if (!isTemporaryId(normalized)) {
-        // Warn and auto-generate rather than failing - an invalid temporary_id is a minor issue
-        const autoGenerated = generateTemporaryId();
-        core.warning(
-          `Invalid temporary_id format: '${pullRequestItem.temporary_id}'. Temporary IDs must be in format 'aw_' followed by 3 to 12 alphanumeric or underscore characters (A-Za-z0-9_). Example: 'aw_abc' or 'aw_pr_fix'. Using auto-generated ID: '${autoGenerated}'`
-        );
-        temporaryId = autoGenerated;
-      } else {
-        temporaryId = normalized.toLowerCase();
-      }
+    const tempIdResult = getOrGenerateTemporaryId(pullRequestItem, "pull request");
+    if (tempIdResult.error) {
+      core.warning(`Skipping create_pull_request: ${tempIdResult.error}`);
+      return { success: false, error: tempIdResult.error };
     }
+    const temporaryId = tempIdResult.temporaryId;
 
     core.info(`Processing create_pull_request: title=${pullRequestItem.title || "No title"}, bodyLength=${pullRequestItem.body?.length || 0}`);
 
diff --git a/actions/setup/js/temporary_id.cjs b/actions/setup/js/temporary_id.cjs
@@ -30,10 +30,12 @@ const crypto = require("crypto");
 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
+ * Regex pattern for detecting candidate #aw_ references (any alphanumeric, underscore, or hyphen content)
+ * Used to identify malformed temporary ID references that don't match TEMPORARY_ID_PATTERN.
+ * Uses a broader character set (including hyphens) than the valid pattern to capture the full token
+ * and warn about references like #aw_test-id where the hyphen makes the whole token invalid.
  */
-const TEMPORARY_ID_CANDIDATE_PATTERN = /#aw_([A-Za-z0-9_]+)\b/gi;
+const TEMPORARY_ID_CANDIDATE_PATTERN = /#aw_([A-Za-z0-9_-]+)/gi;
 
 /**
  * @typedef {Object} RepoIssuePair
diff --git a/actions/setup/js/temporary_id.test.cjs b/actions/setup/js/temporary_id.test.cjs
@@ -177,6 +177,16 @@ describe("temporary_id.cjs", () => {
       expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("#aw_ab"));
       expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("#aw_toolongname123"));
     });
+
+    it("should warn about malformed temporary ID reference containing a hyphen", async () => {
+      const { replaceTemporaryIdReferences } = await import("./temporary_id.cjs");
+      const map = new Map();
+      const text = "Check #aw_test-id for details";
+      const result = replaceTemporaryIdReferences(text, map, "owner/repo");
+      expect(result).toBe("Check #aw_test-id for details");
+      expect(mockCore.warning).toHaveBeenCalledOnce();
+      expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("#aw_test-id"));
+    });
   });
 
   describe("getOrGenerateTemporaryId", () => {
