[CI] Implement public interface validation#3740
Conversation
WalkthroughThe changes introduce an automated workflow for validating the public interface using a new tool, "interface-analyser". This involves updating the CI pipeline, adding installation logic for the tool, updating ignore rules, defining a Fastlane lane for interface validation, and setting relevant environment variables. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant Bootstrap as bootstrap.sh
participant Fastlane as Fastfile
participant InterfaceAnalyzer as interface-analyser
CI->>Bootstrap: Run with INSTALL_INTERFACE_ANALYZER=true
Bootstrap->>InterfaceAnalyzer: Download & install (version from env)
CI->>Fastlane: Run validate_public_interface lane
Fastlane->>InterfaceAnalyzer: Analyze current branch (output: current.json)
Fastlane->>Git: Determine target branch (main/develop), checkout
Fastlane->>InterfaceAnalyzer: Analyze target branch (output: previous.json)
Fastlane->>InterfaceAnalyzer: Diff current.json vs previous.json (output: report.md)
Fastlane->>CI: Post markdown report as PR comment
Fastlane->>Git: Checkout original branch
Estimated code review effort3 (~40 minutes) Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Public Interface🚀 No changes affecting the public interface. |
testableapple
force-pushed
the
ci/public-interface-analyser
branch
from
1a33770 to
da27dae
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Scripts/bootstrap.sh (1)
97-103: Consider adding security measures for binary installation.The installation follows the established pattern but could benefit from additional security measures:
- Checksum verification: Consider verifying the binary's integrity
- Error handling: Add error handling for failed downloads
- Binary validation: Verify the downloaded file is executable
Example improvement:
if [[ ${INSTALL_INTERFACE_ANALYZER-default} == true ]]; then puts "Install interface-analyser v${INTERFACE_ANALYZER_VERSION}" FILE="interface-analyser" - wget "https://github.com/GetStream/stream-module-interface-analyser/releases/download/v${INTERFACE_ANALYZER_VERSION}/${FILE}" + wget "https://github.com/GetStream/stream-module-interface-analyser/releases/download/v${INTERFACE_ANALYZER_VERSION}/${FILE}" || { echo "Failed to download interface-analyser"; exit 1; } + # Verify it's a valid executable + file ${FILE} | grep -q "executable" || { echo "Downloaded file is not executable"; exit 1; } chmod +x ${FILE} sudo mv ${FILE} /usr/local/bin/ fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/smoke-checks.yml(1 hunks).gitignore(1 hunks)Githubfile(1 hunks)Scripts/bootstrap.sh(1 hunks)fastlane/Fastfile(2 hunks)
🔇 Additional comments (5)
Githubfile (1)
10-10: Version 1.0.7 exists, but security-advisory endpoint returned 404 – please confirm manually
- Confirmed
v1.0.7is published in GetStream/stream-module-interface-analyser GitHub releases.- The download URL (
…/releases/download/v1.0.7/interface-analyser) correctly redirects to the asset.- The GitHub REST API for security advisories returned a 404 (“Not Found”), indicating the endpoint isn’t available.
Please manually verify in your organization’s GitHub Security Advisories (GHAS) dashboard or via a third-party scanner (e.g. Snyk, Dependabot) that no known vulnerabilities affect
interface-analyser@1.0.7..gitignore (1)
17-22: LGTM! Comprehensive ignore patterns for interface analyser artifacts.The ignore patterns properly exclude all generated files and directories from the interface analysis process, preventing them from being tracked in version control.
.github/workflows/smoke-checks.yml (2)
56-58: Well-integrated interface validation setup.The environment variable configuration and lane execution are properly integrated into the CI workflow.
54-54: No action required: interface validation lane explicitly fetches the target branch.The
validate_public_interfacelane in your Fastfile setstarget_branch = original_branch.include?('release/') ? 'main' : 'develop' sh("git fetch origin #{target_branch}") sh("git checkout #{target_branch}")so even with the default shallow clone (
fetch-depth: 1), the script will fetch and checkout the full target branch history at runtime. The removal offetch-depth: 100does not block interface validation.fastlane/Fastfile (1)
833-833: LGTM! Proper sources matrix configuration.The
public_interfacesource group correctly points to theSourcesdirectory for interface analysis.
| lane :validate_public_interface do | ||
| next unless is_check_required(sources: sources_matrix[:public_interface], force_check: @force_check) | ||
|
|
||
| # Run the analysis on the current branch | ||
| original_branch = current_branch | ||
| sh('interface-analyser analysis ../Sources/ public_interface_current.json') | ||
|
|
||
| # Checkout the target branch | ||
| target_branch = original_branch.include?('release/') ? 'main' : 'develop' | ||
| sh("git fetch origin #{target_branch}") | ||
| sh("git checkout #{target_branch}") | ||
|
|
||
| # Run the analysis on the target branch | ||
| sh('interface-analyser analysis ../Sources/ public_interface_previous.json') | ||
|
|
||
| # Run diff | ||
| report_path = 'interface-analyser-report.md' | ||
| sh("interface-analyser diff public_interface_current.json public_interface_previous.json #{report_path}") | ||
|
|
||
| # Check if report exists and is non-zero in size | ||
| diff = | ||
| if File.exist?(report_path) && File.size(report_path) > 0 | ||
| File.read(report_path).strip | ||
| else | ||
| '🚀 No changes affecting the public interface.' | ||
| end | ||
|
|
||
| # Generate markdown table for the PR comment | ||
| header = '## Public Interface' | ||
| content = "#{header}\n#{diff}" | ||
|
|
||
| # Post PR comment if running in CI | ||
| pr_comment(text: content, edit_last_comment_with_text: header) if is_ci | ||
|
|
||
| # Checkout the original branch | ||
| sh("git fetch origin #{original_branch}") | ||
| sh("git checkout #{original_branch}") | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and tool verification to the interface validation lane.
The lane implementation is well-structured but needs improvements for robustness:
- Missing error handling: Git operations can fail if branches don't exist or network issues occur
- Tool availability: No verification that
interface-analyseris installed - Branch restoration: Should use
ensureblock to guarantee branch restoration even if errors occur
Consider these improvements:
lane :validate_public_interface do
next unless is_check_required(sources: sources_matrix[:public_interface], force_check: @force_check)
+ # Verify tool is available
+ begin
+ sh('which interface-analyser')
+ rescue
+ UI.user_error!('interface-analyser tool is not installed. Please run bootstrap with INSTALL_INTERFACE_ANALYZER=true')
+ end
# Run the analysis on the current branch
original_branch = current_branch
- sh('interface-analyser analysis ../Sources/ public_interface_current.json')
+
+ begin
+ sh('interface-analyser analysis ../Sources/ public_interface_current.json')
# Checkout the target branch
target_branch = original_branch.include?('release/') ? 'main' : 'develop'
- sh("git fetch origin #{target_branch}")
- sh("git checkout #{target_branch}")
+ sh("git fetch origin #{target_branch}") rescue UI.user_error!("Failed to fetch #{target_branch}")
+ sh("git checkout #{target_branch}") rescue UI.user_error!("Failed to checkout #{target_branch}")
# Run the analysis on the target branch
sh('interface-analyser analysis ../Sources/ public_interface_previous.json')
# Run diff
report_path = 'interface-analyser-report.md'
sh("interface-analyser diff public_interface_current.json public_interface_previous.json #{report_path}")
# Check if report exists and is non-zero in size
diff =
if File.exist?(report_path) && File.size(report_path) > 0
File.read(report_path).strip
else
'🚀 No changes affecting the public interface.'
end
# Generate markdown table for the PR comment
header = '## Public Interface'
content = "#{header}\n#{diff}"
# Post PR comment if running in CI
pr_comment(text: content, edit_last_comment_with_text: header) if is_ci
+ ensure
# Checkout the original branch
- sh("git fetch origin #{original_branch}")
- sh("git checkout #{original_branch}")
+ begin
+ sh("git fetch origin #{original_branch}")
+ sh("git checkout #{original_branch}")
+ rescue => e
+ UI.error("Failed to restore original branch #{original_branch}: #{e}")
+ end
+ end
end📝 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.
| lane :validate_public_interface do | |
| next unless is_check_required(sources: sources_matrix[:public_interface], force_check: @force_check) | |
| # Run the analysis on the current branch | |
| original_branch = current_branch | |
| sh('interface-analyser analysis ../Sources/ public_interface_current.json') | |
| # Checkout the target branch | |
| target_branch = original_branch.include?('release/') ? 'main' : 'develop' | |
| sh("git fetch origin #{target_branch}") | |
| sh("git checkout #{target_branch}") | |
| # Run the analysis on the target branch | |
| sh('interface-analyser analysis ../Sources/ public_interface_previous.json') | |
| # Run diff | |
| report_path = 'interface-analyser-report.md' | |
| sh("interface-analyser diff public_interface_current.json public_interface_previous.json #{report_path}") | |
| # Check if report exists and is non-zero in size | |
| diff = | |
| if File.exist?(report_path) && File.size(report_path) > 0 | |
| File.read(report_path).strip | |
| else | |
| '🚀 No changes affecting the public interface.' | |
| end | |
| # Generate markdown table for the PR comment | |
| header = '## Public Interface' | |
| content = "#{header}\n#{diff}" | |
| # Post PR comment if running in CI | |
| pr_comment(text: content, edit_last_comment_with_text: header) if is_ci | |
| # Checkout the original branch | |
| sh("git fetch origin #{original_branch}") | |
| sh("git checkout #{original_branch}") | |
| end | |
| lane :validate_public_interface do | |
| next unless is_check_required(sources: sources_matrix[:public_interface], force_check: @force_check) | |
| # Verify tool is available | |
| begin | |
| sh('which interface-analyser') | |
| rescue | |
| UI.user_error!('interface-analyser tool is not installed. Please run bootstrap with INSTALL_INTERFACE_ANALYZER=true') | |
| end | |
| # Run the analysis on the current branch | |
| original_branch = current_branch | |
| begin | |
| sh('interface-analyser analysis ../Sources/ public_interface_current.json') | |
| # Checkout the target branch | |
| target_branch = original_branch.include?('release/') ? 'main' : 'develop' | |
| sh("git fetch origin #{target_branch}") rescue UI.user_error!("Failed to fetch #{target_branch}") | |
| sh("git checkout #{target_branch}") rescue UI.user_error!("Failed to checkout #{target_branch}") | |
| # Run the analysis on the target branch | |
| sh('interface-analyser analysis ../Sources/ public_interface_previous.json') | |
| # Run diff | |
| report_path = 'interface-analyser-report.md' | |
| sh("interface-analyser diff public_interface_current.json public_interface_previous.json #{report_path}") | |
| # Check if report exists and is non-zero in size | |
| diff = | |
| if File.exist?(report_path) && File.size(report_path) > 0 | |
| File.read(report_path).strip | |
| else | |
| '🚀 No changes affecting the public interface.' | |
| end | |
| # Generate markdown table for the PR comment | |
| header = '## Public Interface' | |
| content = "#{header}\n#{diff}" | |
| # Post PR comment if running in CI | |
| pr_comment(text: content, edit_last_comment_with_text: header) if is_ci | |
| ensure | |
| # Checkout the original branch | |
| begin | |
| sh("git fetch origin #{original_branch}") | |
| sh("git checkout #{original_branch}") | |
| rescue => e | |
| UI.error("Failed to restore original branch #{original_branch}: #{e}") | |
| end | |
| end | |
| end |
🤖 Prompt for AI Agents
In fastlane/Fastfile lines 849 to 886, add error handling around git commands to
catch failures like missing branches or network issues by wrapping them in
begin-rescue blocks. Before running interface-analyser commands, verify the tool
is installed by checking its availability and aborting with a clear message if
not found. Use an ensure block to guarantee that the original branch is checked
out again even if any errors occur during the lane execution, ensuring branch
restoration happens reliably.
There was a problem hiding this comment.
These error handlings are not required because the script will fail anyways on those lines and print an error message.
There was a problem hiding this comment.
@testableapple You're absolutely right! Fastlane's default behavior will handle the failures and provide clear error messages when shell commands fail. Keeping the implementation simple and relying on fastlane's built-in error reporting makes sense for your use case. Thanks for the clarification!
✏️ Learnings added
Learnt from: testableapple
PR: GetStream/stream-chat-swift#3740
File: fastlane/Fastfile:849-886
Timestamp: 2025-07-23T16:14:40.239Z
Learning: The team prefers simpler error handling in fastlane scripts and relies on fastlane's default error reporting mechanisms rather than adding explicit begin-rescue blocks around shell commands.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
2 participants
🎯 Goal
Implement public interface validation
Summary by CodeRabbit