Skip to content

When calculating app target, attempt to autodetect from advertised capabilities if not specified in url#649

Merged
andrewmcgivery merged 1 commit intomainfrom
am/apps_detect_mcp
Feb 17, 2026
Merged

When calculating app target, attempt to autodetect from advertised capabilities if not specified in url#649
andrewmcgivery merged 1 commit intomainfrom
am/apps_detect_mcp

Conversation

@andrewmcgivery
Copy link
Contributor

When calculating app target, attempt to autodetect from advertised capabilities if not specified in url.

Note this PR also updated rmcp to latest (0.14 -> 0.16)

Changelogs:

@andrewmcgivery andrewmcgivery requested a review from a team as a code owner February 17, 2026 21:47
@apollo-librarian
Copy link

apollo-librarian bot commented Feb 17, 2026

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: 634494c18d5ef97d1cdef995
Build Logs: View logs

@github-actions
Copy link
Contributor

github-actions bot commented Feb 17, 2026

⏭️ Changeset check skipped via label

version: self.server_info.version().to_string(),
website_url: self.server_info.website_url().map(|s| s.to_string()),
// TODO: Add support for this via configuration similar to above fields
description: None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just noting here that I will be creating a Jira task for this for a follow up PR.

@claude
Copy link

claude bot commented Feb 17, 2026

Review Summary

This PR adds MCP app capability auto-detection when the app target is not explicitly specified in the URL. The implementation updates from rmcp 0.14 to 0.16 and introduces a new capability detection mechanism. The changes are well-structured with good test coverage for the new behavior.

Findings

Code Quality (Minor)

  • Test code allocations: The new tests in app.rs:250 and running.rs:880-881 contain .clone() calls on test data that could be avoided with restructuring, though the performance impact in tests is negligible (Chapter 3.2).
  • Test naming: The test test_app_target_missing_with_mcp_app_capability_defaults_to_mcp_apps (81 chars) would benefit from module organization per Chapter 5.1 best practices, e.g., mod app_target_missing { fn with_mcp_app_capability_defaults_to_mcp_apps() {} }.

Test Coverage Assessment

Excellent test coverage - The PR includes comprehensive tests for:

  • New capability detection logic (has_mcp_app_support)
  • Integration with existing app target detection
  • Both positive and negative test cases
  • All existing tests updated to accommodate API changes

Final Recommendation

Approve with suggestions - The implementation is solid and well-tested. The minor suggestions around test organization and allocations are optional improvements that don't impact correctness.


Reviewed by Claude Code Sonnet 4.5

@andrewmcgivery andrewmcgivery added the skip-changeset Used when the changeset verification can be skipped label Feb 17, 2026
Copy link
Member

@DaleSeo DaleSeo left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the upgrade!

@andrewmcgivery andrewmcgivery merged commit 3ec6779 into main Feb 17, 2026
17 of 18 checks passed
@andrewmcgivery andrewmcgivery deleted the am/apps_detect_mcp branch February 17, 2026 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset Used when the changeset verification can be skipped

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants