Address minor mobile deployment bugs (#745)

* feat: improve deployment tooling

* cr feedback

* for temp testing

* clean build artifacts after deploy

* add deploy source

* uncomment ios commands

* Add tests for minor deployment fixes (#750)

* Add test coverage for deployment scripts and Fastfile

* format

* increase github check to 5 minutes

app/fastlane/DEV.md

@@ -116,6 +116,12 @@ yarn mobile-local-deploy:android  # Deploy Android to Google Play Internal Testi
 
 **Why internal testing?** This provides the same safety as GitHub runner deployments while allowing you to use your local machine for building.
 
+After running a local iOS deploy, reset the Xcode project to avoid committing build artifacts:
+
+```bash
+./scripts/cleanup-ios-build.sh
+```
+
 ### Direct Fastlane Commands (Not Recommended)
 
 ⚠️ **Use the confirmation script above instead of these direct commands.**
@@ -218,7 +224,7 @@ Fastlane requires various secrets to interact with the app stores and sign appli
 | `IOS_P12_PASSWORD` | Password for the p12 certificate file |
 | `IOS_TEAM_ID` | Apple Developer Team ID |
 | `IOS_TEAM_NAME` | Apple Developer Team name |
-| `IOS_TESTFLIGHT_GROUPS` | Comma-separated list of TestFlight groups to distribute the app to |
+| `IOS_TESTFLIGHT_GROUPS` | Comma-separated list of **external** TestFlight groups to distribute the app to |
 
 #### Slack Integration Secrets 📱
 

app/fastlane/Fastfile

@@ -22,6 +22,16 @@ android_has_permissions = false
 
 # Project configuration
 PROJECT_NAME = ENV["IOS_PROJECT_NAME"]
+APP_NAME = ENV["IOS_PROJECT_NAME"] || begin
+  app_json_path = File.expand_path("../app.json", __dir__)
+  if File.exist?(app_json_path)
+    app_config = JSON.parse(File.read(app_json_path))
+    app_config["displayName"] if app_config.is_a?(Hash)
+  end
+rescue JSON::ParserError, Errno::ENOENT
+  UI.important("Could not read app.json or invalid JSON format, using default app name")
+  nil
+end || "MobileApp"
 PROJECT_SCHEME = ENV["IOS_PROJECT_SCHEME"]
 SIGNING_CERTIFICATE = ENV["IOS_SIGNING_CERTIFICATE"]
 
@@ -54,7 +64,7 @@ platform :ios do
     upload_to_testflight(
       api_key: result[:api_key],
       distribute_external: true,
-      # TODO: fix error about the groups not being set correctly, fwiw groups are set in the app store connect
+      # Only external TestFlight groups are valid here
       groups: ENV["IOS_TESTFLIGHT_GROUPS"].split(","),
       changelog: "",
       skip_waiting_for_build_processing: false,
@@ -62,11 +72,12 @@ platform :ios do
 
     # Notify Slack about the new build
     if ENV["SLACK_CHANNEL_ID"]
+      deploy_source = Fastlane::Helpers.is_ci_environment? ? "GitHub Workflow" : "Local Deploy"
       Fastlane::Helpers.upload_file_to_slack(
         file_path: result[:ipa_path],
         channel_id: ENV["SLACK_CHANNEL_ID"],
-        initial_comment: "🍎 iOS v#{package_version} (Build #{result[:build_number]}) deployed to TestFlight",
-        title: "#{PROJECT_NAME}-#{package_version}-#{result[:build_number]}.ipa",
+        initial_comment: "🍎 iOS v#{package_version} (Build #{result[:build_number]}) deployed to TestFlight via #{deploy_source}",
+        title: "#{APP_NAME}-#{package_version}-#{result[:build_number]}.ipa",
       )
     else
       UI.important("Skipping Slack notification: SLACK_CHANNEL_ID not set.")
@@ -262,11 +273,12 @@ platform :android do
 
     # Notify Slack about the new build
     if ENV["SLACK_CHANNEL_ID"]
+      deploy_source = Fastlane::Helpers.is_ci_environment? ? "GitHub Workflow" : "Local Deploy"
       Fastlane::Helpers.upload_file_to_slack(
         file_path: android_aab_path,
         channel_id: ENV["SLACK_CHANNEL_ID"],
-        initial_comment: "🤖 Android v#{package_version} (Build #{version_code}) deployed to #{target_platform}",
-        title: "#{PROJECT_NAME}-#{package_version}-#{version_code}.aab",
+        initial_comment: "🤖 Android v#{package_version} (Build #{version_code}) deployed to #{target_platform} via #{deploy_source}",
+        title: "#{APP_NAME}-#{package_version}-#{version_code}.aab",
       )
     else
       UI.important("Skipping Slack notification: SLACK_CHANNEL_ID not set.")

app/fastlane/helpers/slack.rb

@@ -5,6 +5,7 @@ module Fastlane
       def upload_file_to_slack(file_path:, channel_id:, initial_comment: nil, thread_ts: nil, title: nil)
         slack_token = ENV["SLACK_API_TOKEN"]
         report_error("Missing SLACK_API_TOKEN environment variable.", nil, "Slack Upload Failed") if slack_token.to_s.strip.empty?
+        report_error("Missing SLACK_CHANNEL_ID environment variable.", nil, "Slack Upload Failed") if channel_id.to_s.strip.empty?
         report_error("File not found at path: #{file_path}", nil, "Slack Upload Failed") unless File.exist?(file_path)
 
         file_name = File.basename(file_path)

app/fastlane/test/app_name_test.rb

@@ -0,0 +1,58 @@
+require "minitest/autorun"
+require "json"
+require "tmpdir"
+require "fileutils"
+require_relative "../helpers"
+
+class AppNameTest < Minitest::Test
+  def setup
+    @orig_env = ENV.to_h
+    @tmp = Dir.mktmpdir
+  end
+
+  def teardown
+    FileUtils.remove_entry(@tmp)
+    ENV.clear
+    ENV.update(@orig_env)
+  end
+
+  def write_app_json(content)
+    File.write(File.join(@tmp, "app.json"), content)
+  end
+
+  def evaluate_app_name
+    ENV["IOS_PROJECT_NAME"] || begin
+      app_json_path = File.join(@tmp, "app.json")
+      if File.exist?(app_json_path)
+        app_config = JSON.parse(File.read(app_json_path))
+        app_config["displayName"] if app_config.is_a?(Hash)
+      end
+    rescue JSON::ParserError, Errno::ENOENT
+      Fastlane::UI.ui_object.important("Could not read app.json or invalid JSON format, using default app name")
+      nil
+    end || "MobileApp"
+  end
+
+  def test_env_variable_precedence
+    ENV["IOS_PROJECT_NAME"] = "EnvApp"
+    assert_equal "EnvApp", evaluate_app_name
+  end
+
+  def test_display_name_from_app_json
+    ENV.delete("IOS_PROJECT_NAME")
+    write_app_json({ displayName: "JsonApp" }.to_json)
+    assert_equal "JsonApp", evaluate_app_name
+  end
+
+  def test_default_when_app_json_missing_or_malformed
+    ENV.delete("IOS_PROJECT_NAME")
+    write_app_json("{ invalid json")
+    messages = []
+    ui_obj = Fastlane::UI.ui_object
+    orig = ui_obj.method(:important)
+    ui_obj.define_singleton_method(:important) { |msg| messages << msg }
+    assert_equal "MobileApp", evaluate_app_name
+    assert_includes messages.first, "Could not read app.json"
+    ui_obj.define_singleton_method(:important, orig)
+  end
+end

app/fastlane/test/helpers_test.rb

@@ -328,6 +328,57 @@ class HelpersTest < Minitest::Test
     assert_equal ["MISSING_VAR1", "EMPTY_VAR", "WHITESPACE_VAR"], missing
   end
 
+  def test_upload_file_to_slack_missing_channel
+    ENV["SLACK_API_TOKEN"] = "token"
+    file = Tempfile.new(["artifact", ".txt"])
+    file.write("data")
+    file.close
+
+    assert_raises(FastlaneCore::Interface::FastlaneCommonException) do
+      Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "")
+    end
+  ensure
+    file.unlink
+    ENV.delete("SLACK_API_TOKEN")
+  end
+
+  def test_upload_file_to_slack_missing_token
+    ENV.delete("SLACK_API_TOKEN")
+    file = Tempfile.new(["artifact", ".txt"])
+    file.write("data")
+    file.close
+
+    assert_raises(FastlaneCore::Interface::FastlaneCommonException) do
+      Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "C123")
+    end
+  ensure
+    file.unlink
+  end
+
+  def test_slack_deploy_source_messages
+    file = Tempfile.new(["artifact", ".txt"])
+    file.write("data")
+    file.close
+
+    %w[true nil].each do |ci_value|
+      ENV["CI"] = ci_value == "true" ? "true" : nil
+      captured = nil
+      Fastlane::Helpers.stub(:upload_file_to_slack, ->(**args) { captured = args }) do
+        deploy_source = Fastlane::Helpers.is_ci_environment? ? "GitHub Workflow" : "Local Deploy"
+        Fastlane::Helpers.upload_file_to_slack(
+          file_path: file.path,
+          channel_id: "C123",
+          initial_comment: "Deploy via #{deploy_source}",
+        )
+      end
+      expected = ci_value == "true" ? "GitHub Workflow" : "Local Deploy"
+      assert_includes captured[:initial_comment], expected
+    end
+  ensure
+    file.unlink
+    ENV.delete("CI")
+  end
+
   private
 
   def clear_test_env_vars

app/scripts/cleanup-ios-build.sh

@@ -0,0 +1,30 @@
+#!/bin/bash
+# Reset Xcode project after local fastlane builds
+set -euo pipefail
+
+PROJECT_NAME="${IOS_PROJECT_NAME:-Self}"
+PBXPROJ="ios/${PROJECT_NAME}.xcodeproj/project.pbxproj"
+
+if [ ! -f "$PBXPROJ" ]; then
+  echo "Project file not found: $PBXPROJ" >&2
+  exit 1
+fi
+
+MARKETING_VERSION=$(grep -m1 "MARKETING_VERSION =" "$PBXPROJ" | awk '{print $3}' | tr -d ';')
+CURRENT_VERSION=$(grep -m1 "CURRENT_PROJECT_VERSION =" "$PBXPROJ" | awk '{print $3}' | tr -d ';')
+
+# Validate extracted versions
+if [[ -z "$MARKETING_VERSION" || -z "$CURRENT_VERSION" ]]; then
+  echo "Failed to extract version information from $PBXPROJ" >&2
+  exit 1
+fi
+
+git checkout -- "$PBXPROJ"
+
+if sed --version >/dev/null 2>&1; then
+  sed -i -e "s/\(MARKETING_VERSION = \).*/\1$MARKETING_VERSION;/" -e "s/\(CURRENT_PROJECT_VERSION = \).*/\1$CURRENT_VERSION;/" "$PBXPROJ"
+else
+  sed -i '' -e "s/\(MARKETING_VERSION = \).*/\1$MARKETING_VERSION;/" -e "s/\(CURRENT_PROJECT_VERSION = \).*/\1$CURRENT_VERSION;/" "$PBXPROJ"
+fi
+
+echo "Reset $PBXPROJ"

app/scripts/mobile-deploy-confirm.cjs

@@ -2,7 +2,7 @@
 
 const fs = require('fs');
 const path = require('path');
-const { execSync } = require('child_process');
+let { execSync } = require('child_process');
 
 // Constants
 const DEPLOYMENT_METHODS = {
@@ -414,6 +414,36 @@ function getFastlaneCommands(platform) {
   return commands;
 }
 
+/**
+ * Executes iOS build cleanup script
+ * @param {string} platform - Target platform
+ */
+let performIOSBuildCleanup = function (platform) {
+  // Only run cleanup for iOS deployments
+  if (platform !== PLATFORMS.IOS && platform !== PLATFORMS.BOTH) {
+    return;
+  }
+
+  console.log(`\n${CONSOLE_SYMBOLS.BROOM} Cleaning up iOS build artifacts...`);
+
+  try {
+    const cleanupScript = path.join(__dirname, 'cleanup-ios-build.sh');
+    execSync(`bash "${cleanupScript}"`, {
+      stdio: 'inherit',
+      cwd: __dirname,
+    });
+    console.log(
+      `${CONSOLE_SYMBOLS.SUCCESS} iOS build cleanup completed successfully!`,
+    );
+  } catch (error) {
+    console.error(
+      `${CONSOLE_SYMBOLS.WARNING} iOS build cleanup failed (non-fatal):`,
+      error.message,
+    );
+    // Don't exit on cleanup failure - it's not critical
+  }
+};
+
 /**
  * Executes local fastlane deployment
  * @param {string} platform - Target platform
@@ -423,6 +453,8 @@ async function executeLocalFastlaneDeployment(platform) {
     `\n${CONSOLE_SYMBOLS.ROCKET} Starting local fastlane deployment...`,
   );
 
+  let deploymentSuccessful = false;
+
   try {
     performYarnReinstall();
 
@@ -443,6 +475,7 @@ async function executeLocalFastlaneDeployment(platform) {
       });
     }
 
+    deploymentSuccessful = true;
     console.log(
       `${CONSOLE_SYMBOLS.SUCCESS} Local fastlane deployment completed successfully!`,
     );
@@ -454,7 +487,14 @@ async function executeLocalFastlaneDeployment(platform) {
       `${CONSOLE_SYMBOLS.ERROR} Local fastlane deployment failed:`,
       error.message,
     );
-    process.exit(1);
+  } finally {
+    // Always run cleanup after deployment, regardless of success/failure
+    performIOSBuildCleanup(platform);
+
+    // Only exit with error code if deployment failed
+    if (!deploymentSuccessful) {
+      process.exit(1);
+    }
   }
 }
 
@@ -536,7 +576,20 @@ async function main() {
 }
 
 // Execute main function
-main().catch(error => {
-  console.error(`${CONSOLE_SYMBOLS.ERROR} Error:`, error.message);
-  process.exit(1);
-});
+if (require.main === module) {
+  main().catch(error => {
+    console.error(`${CONSOLE_SYMBOLS.ERROR} Error:`, error.message);
+    process.exit(1);
+  });
+} else {
+  module.exports = {
+    performIOSBuildCleanup,
+    executeLocalFastlaneDeployment,
+    _setExecSync: fn => {
+      execSync = fn;
+    },
+    _setPerformIOSBuildCleanup: fn => {
+      performIOSBuildCleanup = fn;
+    },
+  };
+}

app/scripts/tests/cleanup-ios-build.test.cjs

@@ -0,0 +1,88 @@
+const fs = require('fs');
+const path = require('path');
+const os = require('os');
+const { execSync } = require('child_process');
+const { spawnSync } = require('child_process');
+const { describe, it } = require('node:test');
+const assert = require('node:assert');
+
+const SCRIPT = path.resolve(__dirname, '../cleanup-ios-build.sh');
+
+describe('cleanup-ios-build.sh', () => {
+  it('resets pbxproj and reapplies versions', () => {
+    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'cleanup-test-'));
+    const projectName = 'MyApp';
+    const iosDir = path.join(tmp, 'ios', `${projectName}.xcodeproj`);
+    fs.mkdirSync(iosDir, { recursive: true });
+    const pbxPath = path.join(iosDir, 'project.pbxproj');
+    fs.writeFileSync(
+      pbxPath,
+      'CURRENT_PROJECT_VERSION = 1;\nMARKETING_VERSION = 1.0.0;\n',
+    );
+
+    const cwd = process.cwd();
+    process.chdir(tmp);
+    execSync('git init -q');
+    execSync('git config user.email "test@example.com"');
+    execSync('git config user.name "Test"');
+    execSync(`git add ${pbxPath}`);
+    execSync('git commit -m init -q');
+
+    fs.writeFileSync(
+      pbxPath,
+      'CURRENT_PROJECT_VERSION = 2;\nMARKETING_VERSION = 2.0.0;\nSomeArtifact = 123;\n',
+    );
+
+    execSync(`IOS_PROJECT_NAME=${projectName} bash ${SCRIPT}`);
+    process.chdir(cwd);
+
+    const result = fs.readFileSync(pbxPath, 'utf8');
+    assert(result.includes('CURRENT_PROJECT_VERSION = 2;'));
+    assert(result.includes('MARKETING_VERSION = 2.0.0;'));
+    assert(!result.includes('SomeArtifact'));
+  });
+
+  it('fails when the pbxproj file does not exist', () => {
+    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'cleanup-test-'));
+
+    const result = spawnSync('bash', [SCRIPT], {
+      cwd: tmp,
+      env: { ...process.env, IOS_PROJECT_NAME: 'MissingProject' },
+      encoding: 'utf8',
+    });
+
+    assert.notStrictEqual(result.status, 0);
+    assert(result.stderr.includes('Project file not found'));
+  });
+
+  it('fails when version information cannot be extracted', () => {
+    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'cleanup-test-'));
+    const projectName = 'BadApp';
+    const iosDir = path.join(tmp, 'ios', `${projectName}.xcodeproj`);
+    fs.mkdirSync(iosDir, { recursive: true });
+    const pbxPath = path.join(iosDir, 'project.pbxproj');
+    fs.writeFileSync(
+      pbxPath,
+      'CURRENT_PROJECT_VERSION = ;\nMARKETING_VERSION = ;\n',
+    );
+
+    const cwd = process.cwd();
+    process.chdir(tmp);
+    execSync('git init -q');
+    execSync('git config user.email "test@example.com"');
+    execSync('git config user.name "Test"');
+    execSync(`git add ${pbxPath}`);
+    execSync('git commit -m init -q');
+
+    const result = spawnSync('bash', [SCRIPT], {
+      cwd: tmp,
+      env: { ...process.env, IOS_PROJECT_NAME: projectName },
+      encoding: 'utf8',
+    });
+
+    process.chdir(cwd);
+
+    assert.notStrictEqual(result.status, 0);
+    assert(result.stderr.includes('Failed to extract version information'));
+  });
+});

app/scripts/tests/mobile-deploy-confirm-module.test.cjs

@@ -0,0 +1,48 @@
+const { describe, it } = require('node:test');
+const assert = require('node:assert');
+const child_process = require('child_process');
+
+describe('performIOSBuildCleanup', () => {
+  it('executes cleanup script for ios', () => {
+    const deploy = require('../mobile-deploy-confirm.cjs');
+    let called = null;
+    const original = child_process.execSync;
+    deploy._setExecSync(cmd => {
+      called = cmd;
+    });
+    deploy.performIOSBuildCleanup('ios');
+    deploy._setExecSync(original);
+    assert(called && called.includes('cleanup-ios-build.sh'));
+  });
+
+  it('does nothing for android', () => {
+    const deploy = require('../mobile-deploy-confirm.cjs');
+    let called = false;
+    const original = child_process.execSync;
+    deploy._setExecSync(() => {
+      called = true;
+    });
+    deploy.performIOSBuildCleanup('android');
+    deploy._setExecSync(original);
+    assert.strictEqual(called, false);
+  });
+});
+
+describe('executeLocalFastlaneDeployment', () => {
+  it('invokes cleanup after deployment', async () => {
+    const deploy = require('../mobile-deploy-confirm.cjs');
+    deploy._setExecSync(() => {});
+
+    let cleanupCalled = false;
+    const originalCleanup = deploy.performIOSBuildCleanup;
+    deploy._setPerformIOSBuildCleanup(() => {
+      cleanupCalled = true;
+    });
+
+    await deploy.executeLocalFastlaneDeployment('ios');
+
+    deploy._setPerformIOSBuildCleanup(originalCleanup);
+    deploy._setExecSync(child_process.execSync);
+    assert.strictEqual(cleanupCalled, true);
+  });
+});