Skip to content

Commit f024b63

Browse files
committed
fix: address Copilot review comments
- Replace eval with direct command execution for security - Add platform-specific logic to create-platform-release-pr.sh (extension uses yarn, mobile uses npx with pinned version) - Clarify documentation that require_pr_numbers is ignored for mobile - Add detailed comment explaining merge logic rationale in update-release-changelog.sh (intentionally different from create-platform-release-pr.sh)
1 parent 48d797f commit f024b63

2 files changed

Lines changed: 44 additions & 16 deletions

File tree

.github/scripts/create-platform-release-pr.sh

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -351,16 +351,33 @@ create_changelog_pr() {
351351
# Generate Changelog and Test Plan
352352
echo "Generating changelog for ${platform}.."
353353

354-
# Build the auto-changelog command based on platform and options
355-
CHANGELOG_CMD="yarn auto-changelog update --rc --repo \"${GITHUB_REPOSITORY_URL}\" --currentVersion \"${new_version}\" --autoCategorize --useChangelogEntry --useShortPrLink"
356-
357-
# Add --requirePrNumbers flag if enabled
358-
if [[ "${REQUIRE_PR_NUMBERS}" == "true" ]]; then
359-
CHANGELOG_CMD="${CHANGELOG_CMD} --requirePrNumbers"
354+
# Platform-specific logic: extension uses yarn auto-changelog (project dependency),
355+
# mobile uses npx with pinned version
356+
if [[ "${platform}" == "extension" ]]; then
357+
if [[ "${REQUIRE_PR_NUMBERS}" == "true" ]]; then
358+
yarn auto-changelog update --rc \
359+
--repo "${GITHUB_REPOSITORY_URL}" \
360+
--currentVersion "${new_version}" \
361+
--autoCategorize \
362+
--useChangelogEntry \
363+
--useShortPrLink \
364+
--requirePrNumbers
365+
else
366+
yarn auto-changelog update --rc \
367+
--repo "${GITHUB_REPOSITORY_URL}" \
368+
--currentVersion "${new_version}" \
369+
--autoCategorize \
370+
--useChangelogEntry \
371+
--useShortPrLink
372+
fi
373+
else
374+
# Mobile platform: use npx with pinned version, --requirePrNumbers not applicable
375+
npx @metamask/auto-changelog@4.1.0 update --rc \
376+
--repo "${GITHUB_REPOSITORY_URL}" \
377+
--currentVersion "${new_version}" \
378+
--autoCategorize
360379
fi
361380

362-
eval "${CHANGELOG_CMD}"
363-
364381
# Skip commits.csv for hotfix releases (previous_version_ref is literal "null")
365382
# - When we create a new major/minor release, we fetch all commits included in the release, by fetching the diff between HEAD and previous version reference.
366383
# - When we create a new hotfix release, there are no commits included in the release by default (they will be cherry-picked one by one). So we don't have previous version reference, which is why the value is set to 'null'.

.github/scripts/update-release-changelog.sh

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
# 4. previous_version_ref - Previous version reference (branch/tag/SHA). Defaults to literal "null"
1414
# so that commits.csv generation is skipped, matching hotfix behaviour.
1515
# 5. require_pr_numbers - When "true", only include commits with PR numbers (filters out direct commits).
16-
# Defaults to "false". Only applicable to extension platform.
16+
# Defaults to "false". Only applicable to extension platform; ignored for mobile.
1717
#
1818
# Environment (optional):
1919
# GITHUB_TOKEN - Token for GitHub CLI operations (falls back to gh auth config)
@@ -63,7 +63,10 @@ checkout_or_create_branch() {
6363
else
6464
git checkout "${branch_name}"
6565
fi
66-
# Merge the base branch to include any new commits from the release branch
66+
# Merge the base branch to include any new commits from the release branch.
67+
# This merge logic is intentionally different from heckout_or_create_branch()
68+
# This script runs on every push to the release branch, so the changelog branch may already exist with older content.
69+
# We need to merge the latest release branch commits to ensure auto-changelog sees all new commits.
6770
if [[ -n "${base_branch}" ]]; then
6871
echo "Merging ${base_branch} into ${branch_name} to include new commits..."
6972
git fetch origin "${base_branch}"
@@ -216,14 +219,22 @@ echo "Generating changelog for ${PLATFORM} ${VERSION}.."
216219

217220
# Build the auto-changelog command based on platform and options
218221
if [[ "${PLATFORM}" == "extension" ]]; then
219-
CHANGELOG_CMD="yarn auto-changelog update --rc --repo \"${GITHUB_REPOSITORY_URL}\" --currentVersion \"${VERSION}\" --autoCategorize --useChangelogEntry --useShortPrLink"
220-
221-
# Add --requirePrNumbers flag if enabled
222222
if [[ "${REQUIRE_PR_NUMBERS}" == "true" ]]; then
223-
CHANGELOG_CMD="${CHANGELOG_CMD} --requirePrNumbers"
223+
yarn auto-changelog update --rc \
224+
--repo "${GITHUB_REPOSITORY_URL}" \
225+
--currentVersion "${VERSION}" \
226+
--autoCategorize \
227+
--useChangelogEntry \
228+
--useShortPrLink \
229+
--requirePrNumbers
230+
else
231+
yarn auto-changelog update --rc \
232+
--repo "${GITHUB_REPOSITORY_URL}" \
233+
--currentVersion "${VERSION}" \
234+
--autoCategorize \
235+
--useChangelogEntry \
236+
--useShortPrLink
224237
fi
225-
226-
eval "${CHANGELOG_CMD}"
227238
else
228239
npx @metamask/auto-changelog@4.1.0 update --rc --repo "${GITHUB_REPOSITORY_URL}" --currentVersion "${VERSION}" --autoCategorize
229240
fi

0 commit comments

Comments
 (0)