Skip to content

[mcp] fix issue where crashlytics isn't detected for ios apps that use project.pbxproj #9515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 25, 2025
+25 −0
Merged

[mcp] fix issue where crashlytics isn't detected for ios apps that use project.pbxproj #9515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1165171
fix issue where crashlytics isn't detected for ios apps that use proj…
aalej Nov 24, 2025
e715e29
update changelog
aalej Nov 24, 2025
bccd465
Merge branch 'master' into aalej_fix-mcp-ioscrashlytics-detect
aalej Nov 25, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fixed issue where MCP server didn't detect if iOS app uses Crashlytics in projects that use `project.pbxproj` (#9515)
17 changes: 17 additions & 0 deletions src/mcp/util/crashlytics/availability.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,23 @@ describe("isCrashlyticsAvailable", () => {
expect(result).to.be.false;
});

it("should return true for an iOS project with Crashlytics in project.pbxproj", async () => {
mockfs({
"/test-dir": {
ios: {
"Project.xcodeproj": {
"project.pbxproj":
"/* Begin PBXBuildFile section */ /* FirebaseCore in Frameworks */ = {isa = PBXBuildFile; /* FirebaseCore */; }; /* FirebaseCrashlytics in Frameworks */ = {isa = PBXBuildFile; /* FirebaseCrashlytics */; }; /* End PBXBuildFile section */",
},
},
},
});

const result = await isCrashlyticsAvailable(mockContext("/test-dir"));

expect(result).to.be.true;
});

it("should return true for a Flutter project with Crashlytics in pubspec.yaml", async () => {
mockfs({
"/test-dir": {
Expand Down
7 changes: 7 additions & 0 deletions src/mcp/util/crashlytics/availability.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* Returns a function that detects whether Crashlytics is available.
*/
export async function isCrashlyticsAvailable(ctx: McpContext): Promise<boolean> {
ctx.host.logger.debug("Looking for whether crashlytics is installed...");

Check warning on line 10 in src/mcp/util/crashlytics/availability.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
return await isCrashlyticsInstalled(ctx);
}

Expand All @@ -22,24 +22,24 @@
!platforms.includes(Platform.ANDROID) &&
!platforms.includes(Platform.IOS)
) {
host.logger.debug("Found no supported Crashlytics platforms.");

Check warning on line 25 in src/mcp/util/crashlytics/availability.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
return false;
}

if (platforms.includes(Platform.FLUTTER) && (await flutterAppUsesCrashlytics(projectDir))) {
host.logger.debug("Found Flutter app using Crashlytics");

Check warning on line 30 in src/mcp/util/crashlytics/availability.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
return true;
}
if (platforms.includes(Platform.ANDROID) && (await androidAppUsesCrashlytics(projectDir))) {
host.logger.debug("Found Android app using Crashlytics");

Check warning on line 34 in src/mcp/util/crashlytics/availability.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
return true;
}
if (platforms.includes(Platform.IOS) && (await iosAppUsesCrashlytics(projectDir))) {
host.logger.debug("Found iOS app using Crashlytics");

Check warning on line 38 in src/mcp/util/crashlytics/availability.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
return true;
}

host.logger.debug(

Check warning on line 42 in src/mcp/util/crashlytics/availability.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
`Found supported platforms ${JSON.stringify(platforms)}, but did not find a Crashlytics dependency.`,
);
return false;
Expand Down Expand Up @@ -80,6 +80,13 @@
return true;
}
}
const xcodeProjectFiles = await detectFiles(appPath, "project.pbxproj");
for (const file of xcodeProjectFiles) {
const content = await fs.readFile(path.join(appPath, file), "utf8");
if (content.includes("Crashlytics")) {
return true;
}
}
Comment on lines +83 to +89
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 24, 2025

Choose a reason for hiding this comment

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

medium

This block of code is nearly identical to the three preceding blocks that check for other file types. This code repetition makes the function less maintainable and can be inefficient as it checks for each file type sequentially.

To improve this, you could refactor the logic to avoid code duplication and improve performance by reading files in parallel. This would make iosAppUsesCrashlytics more concise, performant, and easier to extend.

For example, you could refactor the function to something like this:

async function iosAppUsesCrashlytics(appPath: string): Promise<boolean> {
  const filePatterns = ["Podfile", "Package.swift", "Cartfile*", "project.pbxproj"];

  for (const pattern of filePatterns) {
    const files = await detectFiles(appPath, pattern);
    if (files.length === 0) {
      continue;
    }

    const readPromises = files.map(async (file) => {
      const content = await fs.readFile(path.join(appPath, file), "utf8");
      return content.includes("Crashlytics");
    });

    const results = await Promise.all(readPromises);
    if (results.some((found) => found)) {
      return true;
    }
  }

  return false;
}

While this refactoring is larger than the current change, it would significantly improve the quality of this function.

Copy link
Contributor

@schnecle schnecle Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't love Gemini's approach, but we could concat all of the detected files into a single list and loop over that single list. Not a blocking comment, but might be a nice clean up.

Copy link
Contributor Author

@aalej aalej Nov 25, 2025

Choose a reason for hiding this comment

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

Thanks for the feedback! Going to go ahead and merge this for now, and will work on the clean up in another PR. Just to verify the process, do we want to first detect each file then read through each one? so it would be something like

Detect Podfile
Detect Package.swift
Detect Cartfile*
Detect project.pbxproj
Read Podfile // If using pods, will return after getting all 4 files
Read Package.swift
Read Cartfile*
Read project.pbxproj

I think it would be better if we do something like the current implementation since we can return early once we read the file has Crashlytics

Detect Podfile
Read Podfile // If using pods, returns after getting Podfile
Detect Package.swift
Read Package.swift
Detect Cartfile*
Read Cartfile*
Detect project.pbxproj
Read project.pbxproj

Let me know if I misunderstood anything here.

Copy link
Contributor

@schnecle schnecle Nov 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I was suggesting the first one --

Detect Podfile
Detect Package.swift
Detect Cartfile*
Detect project.pbxproj
Read Podfile // If using pods, will exit here
Read Package.swift
Read Cartfile*
Read project.pbxproj

The file detection is just looking matching file names. It is slightly less efficient in that we are going to try to detect all the different file patterns, but we still have the opportunity to exit early if we find what we are looking for. It would make the code a lot cleaner because it would be something like --

const fileArrays = await Promises.all(
  detectFiles(appPath, "Podfile"), 
  detectFiles(appPath, "Package.swift"),
  detectFiles(appPath, "Cartfile*"),
  detectFiles(appPath, "project.pbxproj"));

const files = fileArrays.flat();

if (files.length === 0) {
  return false;
}

for (const file of files) {
  const content = await fs.readFile(path.join(appPath, file), "utf8");
  if (content.includes("Crashlytics")) {
    return true;
  }
}

return false;

Copy link
Contributor Author

@aalej aalej Nov 26, 2025

Choose a reason for hiding this comment

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

Thanks, this definitely does look cleaner. I will work on this!

return false;
}

Expand Down
Loading