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
25 changes: 16 additions & 9 deletions .github/workflows/release-shared.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,26 @@ jobs:
--run-number "${{ github.run_number }}" \
>> "$GITHUB_OUTPUT"

- name: Resolve IDB cache inputs
id: idb_inputs
run: |
set -euo pipefail
PINNED_IDB_COMMIT=$(grep '^IDB_GIT_REF=' scripts/build.sh | sed -E 's/^IDB_GIT_REF="\$\{IDB_GIT_REF:-([^}]*)\}"$/\1/')
PATCH_HASH=$(find patches/idb -name '*.patch' -type f | sort | xargs shasum -a 256 | shasum -a 256 | awk '{print $1}')
echo "idb_commit=$PINNED_IDB_COMMIT" >> "$GITHUB_OUTPUT"
echo "patch_hash=$PATCH_HASH" >> "$GITHUB_OUTPUT"

Comment on lines +55 to +63

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add error handling and validation for cache input extraction.

The extraction logic is fragile and lacks validation:

  1. The grep command will fail silently if the pattern changes in scripts/build.sh.
  2. No validation that PINNED_IDB_COMMIT is non-empty after extraction.
  3. If no patch files exist, PATCH_HASH will be the hash of empty input, which may not be the intended behaviour.

Consider adding validation after extraction to fail fast with a clear error message if the inputs are invalid.

🛡️ Proposed fix to add validation
       - name: Resolve IDB cache inputs
         id: idb_inputs
         run: |
           set -euo pipefail
           PINNED_IDB_COMMIT=$(grep '^IDB_GIT_REF=' scripts/build.sh | sed -E 's/^IDB_GIT_REF="\$\{IDB_GIT_REF:-([^}]*)\}"$/\1/')
+          if [[ -z "$PINNED_IDB_COMMIT" ]]; then
+            echo "❌ Error: Failed to extract IDB_GIT_REF from scripts/build.sh"
+            exit 1
+          fi
           PATCH_HASH=$(find patches/idb -name '*.patch' -type f | sort | xargs shasum -a 256 | shasum -a 256 | awk '{print $1}')
+          if [[ -z "$PATCH_HASH" ]]; then
+            echo "❌ Error: Failed to compute patch hash"
+            exit 1
+          fi
           echo "idb_commit=$PINNED_IDB_COMMIT" >> "$GITHUB_OUTPUT"
           echo "patch_hash=$PATCH_HASH" >> "$GITHUB_OUTPUT"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Resolve IDB cache inputs
id: idb_inputs
run: |
set -euo pipefail
PINNED_IDB_COMMIT=$(grep '^IDB_GIT_REF=' scripts/build.sh | sed -E 's/^IDB_GIT_REF="\$\{IDB_GIT_REF:-([^}]*)\}"$/\1/')
PATCH_HASH=$(find patches/idb -name '*.patch' -type f | sort | xargs shasum -a 256 | shasum -a 256 | awk '{print $1}')
echo "idb_commit=$PINNED_IDB_COMMIT" >> "$GITHUB_OUTPUT"
echo "patch_hash=$PATCH_HASH" >> "$GITHUB_OUTPUT"
- name: Resolve IDB cache inputs
id: idb_inputs
run: |
set -euo pipefail
PINNED_IDB_COMMIT=$(grep '^IDB_GIT_REF=' scripts/build.sh | sed -E 's/^IDB_GIT_REF="\$\{IDB_GIT_REF:-([^}]*)\}"$/\1/')
if [[ -z "$PINNED_IDB_COMMIT" ]]; then
echo "❌ Error: Failed to extract IDB_GIT_REF from scripts/build.sh"
exit 1
fi
PATCH_HASH=$(find patches/idb -name '*.patch' -type f | sort | xargs shasum -a 256 | shasum -a 256 | awk '{print $1}')
if [[ -z "$PATCH_HASH" ]]; then
echo "❌ Error: Failed to compute patch hash"
exit 1
fi
echo "idb_commit=$PINNED_IDB_COMMIT" >> "$GITHUB_OUTPUT"
echo "patch_hash=$PATCH_HASH" >> "$GITHUB_OUTPUT"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/release-shared.yml around lines 55 - 63, The extraction
step (id idb_inputs) must validate PINNED_IDB_COMMIT (from scripts/build.sh via
grep) and PATCH_HASH (from patches/idb) and fail fast with clear messages: after
computing PINNED_IDB_COMMIT ensure it is non-empty and if empty print a
descriptive error and exit 1; for PATCH_HASH first detect whether any files
exist under patches/idb and if none either fail with a clear error or set a
deterministic sentinel (but do not silently use the hash of empty input); ensure
grep failures are surfaced (keep set -euo pipefail and avoid swallowing grep
errors) and only write idb_commit/patch_hash to GITHUB_OUTPUT when they pass
validation so the workflow fails early with actionable logs.

- name: Restore IDB repository cache
uses: actions/cache/restore@v4
with:
path: idb_checkout
key: idb-repo-${{ runner.os }}-dummy
restore-keys: |
idb-repo-${{ runner.os }}-
key: idb-repo-${{ runner.os }}-${{ steps.idb_inputs.outputs.idb_commit }}-${{ steps.idb_inputs.outputs.patch_hash }}

- name: Check IDB repository freshness
id: idb_check
run: |
set -euo pipefail
PINNED_IDB_COMMIT=$(grep '^IDB_GIT_REF=' scripts/build.sh | sed -E 's/^IDB_GIT_REF="\$\{IDB_GIT_REF:-([^}]*)\}"$/\1/')
PINNED_IDB_COMMIT="${{ steps.idb_inputs.outputs.idb_commit }}"
echo "idb_commit=$PINNED_IDB_COMMIT" >> "$GITHUB_OUTPUT"

if [ -d "idb_checkout/.git" ]; then
Expand All @@ -81,13 +88,13 @@ jobs:
uses: actions/cache/restore@v4
with:
path: build_products/Frameworks
key: frameworks-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}
key: frameworks-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}-${{ steps.idb_inputs.outputs.patch_hash }}

- name: Restore XCFramework cache
uses: actions/cache/restore@v4
with:
path: build_products/XCFrameworks
key: xcframeworks-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}
key: xcframeworks-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}-${{ steps.idb_inputs.outputs.patch_hash }}

- name: Check XCFramework outputs
id: xcframework_check
Expand Down Expand Up @@ -195,21 +202,21 @@ jobs:
uses: actions/cache/save@v4
with:
path: idb_checkout
key: idb-repo-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}
key: idb-repo-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}-${{ steps.idb_inputs.outputs.patch_hash }}

- name: Save runtime Frameworks cache
if: steps.idb_check.outputs.needs_setup == 'true' || steps.xcframework_check.outputs.needs_build == 'true'
uses: actions/cache/save@v4
with:
path: build_products/Frameworks
key: frameworks-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}
key: frameworks-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}-${{ steps.idb_inputs.outputs.patch_hash }}

- name: Save XCFramework cache
if: steps.idb_check.outputs.needs_setup == 'true' || steps.xcframework_check.outputs.needs_build == 'true'
uses: actions/cache/save@v4
with:
path: build_products/XCFrameworks
key: xcframeworks-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}
key: xcframeworks-${{ runner.os }}-${{ steps.idb_check.outputs.idb_commit }}-${{ steps.idb_inputs.outputs.patch_hash }}

- name: Prepare - Clean Swift build environment
run: |
Expand Down
24 changes: 24 additions & 0 deletions scripts/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,24 @@ function verify_macho_has_arch() {
fi
}

function verify_fbsimulatorcontrol_has_xctest_client_type_patch() {
local binary_path="$1"

if [[ ! -f "$binary_path" ]]; then
echo "❌ Error: FBSimulatorControl binary not found for accessibility patch verification: $binary_path"
exit 1
fi

local client_type_references
client_type_references=$(strings -a "$binary_path" | grep -F "setClientType:" || true)
if [[ -z "$client_type_references" ]]; then
echo "❌ Error: FBSimulatorControl is missing the XCTest accessibility client type patch"
echo " Expected to find selector reference 'setClientType:' in: $binary_path"
echo " Rebuild IDB frameworks after applying patches/idb/accessibility-client-type-for-xctest.patch."
exit 1
fi
}

# Function to invoke xcodebuild, optionally with xcpretty
function invoke_xcodebuild() {
local arguments=("$@")
Expand Down Expand Up @@ -575,6 +593,9 @@ function verify_xcframework_inputs() {
fi
verify_macho_has_arch "${framework_binary}" "arm64"
verify_macho_has_arch "${framework_binary}" "x86_64"
if [[ "${framework_name}" == "FBSimulatorControl" ]]; then
verify_fbsimulatorcontrol_has_xctest_client_type_patch "${framework_binary}"
fi
done

print_success "XCFramework inputs include arm64 and x86_64 slices"
Expand Down Expand Up @@ -609,6 +630,9 @@ function verify_release_architectures() {
fi
verify_macho_has_arch "${framework_binary}" "arm64"
verify_macho_has_arch "${framework_binary}" "x86_64"
if [[ "${framework_name}" == "FBSimulatorControl" ]]; then
verify_fbsimulatorcontrol_has_xctest_client_type_patch "${framework_binary}"
fi
done

print_success "Release artifacts include arm64 and x86_64 slices"
Expand Down
Loading