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
15 changes: 14 additions & 1 deletion actions/setup/js/create_discussion.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const HANDLER_TYPE = "create_discussion";

const { getTrackerID } = require("./get_tracker_id.cjs");
const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs");
const { replaceTemporaryIdReferences } = require("./temporary_id.cjs");
const { generateTemporaryId, isTemporaryId, normalizeTemporaryId, getOrGenerateTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs");
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

generateTemporaryId, isTemporaryId, and normalizeTemporaryId are imported here but not referenced anywhere in create_discussion.cjs (only getOrGenerateTemporaryId and replaceTemporaryIdReferences are used). Removing unused imports will make it clearer which temporary-id utilities this handler actually depends on.

Suggested change
const { generateTemporaryId, isTemporaryId, normalizeTemporaryId, getOrGenerateTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs");
const { getOrGenerateTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs");

Copilot uses AI. Check for mistakes.
const { parseAllowedRepos, getDefaultTargetRepo, validateRepo, parseRepoSlug } = require("./repo_helpers.cjs");
const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_title.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");
Expand Down Expand Up @@ -460,6 +460,19 @@ async function main(config = {}) {
const categoryId = resolvedCategory.id;
core.info(`Using category: ${resolvedCategory.name} (${resolvedCategory.matchType})`);

// Get or generate the temporary ID for this discussion
const tempIdResult = getOrGenerateTemporaryId(message, "discussion");
if (tempIdResult.error) {
core.warning(`Skipping discussion: ${tempIdResult.error}`);
return {
success: false,
error: tempIdResult.error,
};
}
// At this point, temporaryId is guaranteed to be a string (not null)
const temporaryId = /** @type {string} */ tempIdResult.temporaryId;
core.info(`Processing create_discussion: title=${message.title}, bodyLength=${message.body?.length ?? 0}, temporaryId=${temporaryId}, repo=${qualifiedItemRepo}`);

// Build labels array (merge config labels with item-specific labels)
const discussionLabels = [...labels, ...(Array.isArray(item.labels) ? item.labels : [])]
.filter(Boolean)
Expand Down
39 changes: 13 additions & 26 deletions actions/setup/js/create_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs");
const { generateFooterWithMessages } = require("./messages_footer.cjs");
const { generateWorkflowIdMarker } = require("./generate_footer.cjs");
const { getTrackerID } = require("./get_tracker_id.cjs");
const { generateTemporaryId, isTemporaryId, normalizeTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs");
const { generateTemporaryId, isTemporaryId, normalizeTemporaryId, getOrGenerateTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs");
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

generateTemporaryId is imported from temporary_id.cjs but is no longer used after switching to getOrGenerateTemporaryId(). Consider removing it from the destructured require to keep the dependency list accurate.

Suggested change
const { generateTemporaryId, isTemporaryId, normalizeTemporaryId, getOrGenerateTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs");
const { isTemporaryId, normalizeTemporaryId, getOrGenerateTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs");

Copilot uses AI. Check for mistakes.
const { parseAllowedRepos, getDefaultTargetRepo, validateRepo, parseRepoSlug } = require("./repo_helpers.cjs");
const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_title.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");
Expand Down Expand Up @@ -309,31 +309,16 @@ async function main(config = {}) {
}

// Get or generate the temporary ID for this issue
let temporaryId = generateTemporaryId();
if (message.temporary_id !== undefined && message.temporary_id !== null) {
if (typeof message.temporary_id !== "string") {
const error = `temporary_id must be a string (got ${typeof message.temporary_id})`;
core.warning(`Skipping issue: ${error}`);
return {
success: false,
error,
};
}

const rawTemporaryId = message.temporary_id.trim();
const normalized = rawTemporaryId.startsWith("#") ? rawTemporaryId.substring(1).trim() : rawTemporaryId;

if (!isTemporaryId(normalized)) {
const 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'`;
core.warning(`Skipping issue: ${error}`);
return {
success: false,
error,
};
}

temporaryId = normalized.toLowerCase();
const tempIdResult = getOrGenerateTemporaryId(message, "issue");
if (tempIdResult.error) {
core.warning(`Skipping issue: ${tempIdResult.error}`);
return {
success: false,
error: tempIdResult.error,
};
}
// At this point, temporaryId is guaranteed to be a string (not null)
const temporaryId = /** @type {string} */ tempIdResult.temporaryId;
core.info(`Processing create_issue: title=${message.title}, bodyLength=${message.body?.length ?? 0}, temporaryId=${temporaryId}, repo=${qualifiedItemRepo}`);

// Resolve parent: check if it's a temporary ID reference
Expand Down Expand Up @@ -500,7 +485,9 @@ async function main(config = {}) {
createdIssues.push({ ...issue, _repo: qualifiedItemRepo });

// Store the mapping of temporary_id -> {repo, number}
temporaryIdMap.set(normalizeTemporaryId(temporaryId), { repo: qualifiedItemRepo, number: issue.number });
// temporaryId is guaranteed to be non-null because we checked tempIdResult.error above
const normalizedTempId = normalizeTemporaryId(String(temporaryId));
temporaryIdMap.set(normalizedTempId, { repo: qualifiedItemRepo, number: issue.number });
core.info(`Stored temporary ID mapping: ${temporaryId} -> ${qualifiedItemRepo}#${issue.number}`);

// Track issue for copilot assignment if needed
Expand Down
14 changes: 13 additions & 1 deletion actions/setup/js/create_project.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

const { loadAgentOutput } = require("./load_agent_output.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");
const { normalizeTemporaryId, isTemporaryId } = require("./temporary_id.cjs");
const { normalizeTemporaryId, isTemporaryId, generateTemporaryId, getOrGenerateTemporaryId } = require("./temporary_id.cjs");
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

generateTemporaryId is imported from temporary_id.cjs but isn’t used in this module anymore (temporary IDs come from getOrGenerateTemporaryId()). Consider removing it from the destructured require to avoid confusion about the source of temporary IDs.

Suggested change
const { normalizeTemporaryId, isTemporaryId, generateTemporaryId, getOrGenerateTemporaryId } = require("./temporary_id.cjs");
const { normalizeTemporaryId, isTemporaryId, getOrGenerateTemporaryId } = require("./temporary_id.cjs");

Copilot uses AI. Check for mistakes.

/**
* Log detailed GraphQL error information
Expand Down Expand Up @@ -343,6 +343,18 @@ async function main(config = {}, githubClient = null) {
try {
let { title, owner, owner_type, item_url } = message;

// Get or generate the temporary ID for this project
const tempIdResult = getOrGenerateTemporaryId(message, "project");
if (tempIdResult.error) {
core.warning(`Skipping project: ${tempIdResult.error}`);
return {
success: false,
error: tempIdResult.error,
};
}
// At this point, temporaryId is guaranteed to be a string (not null)
const temporaryId = /** @type {string} */ tempIdResult.temporaryId;
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

create_project now auto-generates a temporaryId via getOrGenerateTemporaryId() when message.temporary_id is missing, but that generated ID isn’t written back onto message.temporary_id and isn’t included in the non-staged return value. As a result, the unified handler manager can’t persist the project mapping (it keys off message.temporary_id) and the caller never receives the auto-generated ID, which contradicts the create_project tool contract (“auto-generated and returned”). Consider setting message.temporary_id to the generated value and/or returning it (e.g. temporaryId) so downstream mapping and follow-up update_project calls work.

Suggested change
const temporaryId = /** @type {string} */ tempIdResult.temporaryId;
const temporaryId = /** @type {string} */ tempIdResult.temporaryId;
// Persist the generated temporary ID back onto the message so downstream
// handlers that key off message.temporary_id can use it.
message.temporary_id = temporaryId;

Copilot uses AI. Check for mistakes.

// Resolve temporary ID in item_url if present
if (item_url && typeof item_url === "string") {
// Check if item_url contains a temporary ID (either as URL or plain ID)
Expand Down
1 change: 0 additions & 1 deletion actions/setup/js/create_project_status_update.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,6 @@ async function main(config = {}, githubClient = null) {
previewInfo: {
projectUrl: effectiveProjectUrl,
status,
title,
},
};
}
Expand Down
43 changes: 43 additions & 0 deletions actions/setup/js/temporary_id.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,48 @@ function replaceTemporaryIdReferencesLegacy(text, tempIdMap) {
});
}

/**
* Validate and process a temporary_id from a message
* Auto-generates a temporary ID if not provided, or validates and normalizes if provided
*
* @param {Object} message - The message object that may contain a temporary_id field
* @param {string} entityType - Type of entity (e.g., "issue", "discussion", "project") for error messages
* @returns {{temporaryId: string, error: null} | {temporaryId: null, error: string}} Result with temporaryId or error
*/
function getOrGenerateTemporaryId(message, entityType = "item") {
// Auto-generate if not provided
Comment on lines +99 to +108
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

In getOrGenerateTemporaryId(), the entityType parameter is documented as being used for error messages but it’s not referenced anywhere in the function. Either remove the parameter from the signature/JSDoc, or incorporate it into the returned error strings so the documentation matches behavior.

Copilot uses AI. Check for mistakes.
if (message.temporary_id === undefined || message.temporary_id === null) {
return {
temporaryId: generateTemporaryId(),
error: null,
};
}

// Validate type
if (typeof message.temporary_id !== "string") {
return {
temporaryId: null,
error: `temporary_id must be a string (got ${typeof message.temporary_id})`,
};
}

// Normalize and validate format
const rawTemporaryId = message.temporary_id.trim();
const normalized = rawTemporaryId.startsWith("#") ? rawTemporaryId.substring(1).trim() : rawTemporaryId;

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'`,
};
}

return {
temporaryId: normalized.toLowerCase(),
error: null,
};
}
Comment on lines +107 to +139
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

New helper getOrGenerateTemporaryId() is a behavior change (auto-generate, validate, normalize) but there are no unit tests covering its main branches (missing temporary_id, non-string, invalid format, leading #, normalization). Adding targeted tests in temporary_id.test.cjs would help prevent regressions as more handlers adopt this helper.

Copilot uses AI. Check for mistakes.

/**
* Load the temporary ID map from environment variable
* Supports both old format (temporary_id -> number) and new format (temporary_id -> {repo, number})
Expand Down Expand Up @@ -470,6 +512,7 @@ module.exports = {
generateTemporaryId,
isTemporaryId,
normalizeTemporaryId,
getOrGenerateTemporaryId,
replaceTemporaryIdReferences,
replaceTemporaryIdReferencesLegacy,
loadTemporaryIdMap,
Expand Down
8 changes: 4 additions & 4 deletions pkg/workflow/docker_api_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@ func TestCollectDockerImages_APIProxyForEnginesWithLLMGateway(t *testing.T) {
expectAPIProxy bool
}{
{
name: "Claude engine does not include api-proxy image (supportsLLMGateway: false)",
name: "Claude engine includes api-proxy image (supportsLLMGateway: 10000)",
engine: "claude",
expectAPIProxy: false,
expectAPIProxy: true,
},
{
name: "Copilot engine does not include api-proxy image (supportsLLMGateway: false)",
engine: "copilot",
expectAPIProxy: false,
},
{
name: "Codex engine does not include api-proxy image (supportsLLMGateway: false)",
name: "Codex engine includes api-proxy image (supportsLLMGateway: 10001)",
engine: "codex",
expectAPIProxy: false,
expectAPIProxy: true,
},
}

Expand Down
Loading