Skip to content

Conversation

@jakejarvis
Copy link
Owner

No description provided.

…roach with enhanced evaluation methods and update schemas for improved clarity and maintainability
…proach, update schemas for clarity, and remove deprecated alias handling
@vercel
Copy link

vercel bot commented Sep 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
hoot Ready Ready Preview Comment Sep 30, 2025 8:28pm

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors the provider detection logic from a simple rule-based system to a more sophisticated AST (Abstract Syntax Tree) approach with composable logic operators. The change enables more complex detection rules while maintaining type safety and JSON serializability.

  • Replaced individual DetectionRule types with a unified Logic AST supporting AND/OR/NOT composition
  • Consolidated provider definitions to use a single Provider type with categories instead of separate types per provider kind
  • Moved DoH (DNS over HTTPS) provider definitions from a separate file into the main DNS service file

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
server/services/registration.ts Updated to use new detectRegistrar function and handle both name and domain updates
server/services/hosting.ts Removed IP address field, reorganized email provider detection timing, and cleaned up formatting
server/services/hosting.test.ts Removed test assertion for deprecated ipAddress field
server/services/doh-providers.ts Deleted file - DoH provider definitions moved to dns.ts
server/services/dns.ts Consolidated DoH provider definitions from separate file
lib/schemas/tls.ts Removed CertificateAuthorityProvider schema in favor of unified provider system
lib/schemas/registration.ts Removed RegistrarProvider schema in favor of unified provider system
lib/schemas/provider.ts Complete rewrite to define Logic AST types and unified Provider interface
lib/schemas/hosting.ts Removed HostingProvider schema and ipAddress field
lib/providers/detection.ts Refactored detection logic to use new AST evaluation with evalRule function
lib/providers/detection.test.ts Updated test to use new detectRegistrar function
lib/providers/catalog.ts Converted all provider definitions to use new AST rule format with categories
lib/providers/README.md Updated documentation to reflect new AST-based detection system
components/domain/sections/hosting-email-section.test.tsx Removed test assertion for deprecated ipAddress field
components/domain/domain-report-view.test.tsx Removed test assertion for deprecated ipAddress field

const ttl = record.isRegistered ? 24 * 60 * 60 : 60 * 60;
const registrarName = (record.registrar?.name || "").toString();
const registrarUrl = (record.registrar?.url || "").toString();
let registrarName = (record.registrar?.name || "").toString();
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

[nitpick] The variable declaration has changed from const to let but the initial assignment logic remains the same. Consider keeping it as const registrarName and use a separate variable for the potentially updated name to maintain immutability where possible.

Copilot uses AI. Check for mistakes.
const ctx: DetectionContext = {
headers: headersObj,
mx: (mxHosts ?? []).map((h) => h.toLowerCase().replace(/\.$/, "")),
ns: (nsHosts ?? []).map((h) => h.toLowerCase().replace(/\.$/, "")),
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

[nitpick] The regex /\.$$/ could be extracted as a constant for better readability and reusability, especially since FQDN trailing dot removal is a common DNS operation.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Sep 30, 2025

Codecov Report

❌ Patch coverage is 97.72727% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.52%. Comparing base (266ba50) to head (59a30b1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/providers/detection.ts 89.39% 6 Missing and 1 partial ⚠️
lib/cloudflare.ts 63.63% 4 Missing ⚠️
server/services/registration.ts 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #52      +/-   ##
==========================================
+ Coverage   64.73%   66.52%   +1.79%     
==========================================
  Files          94       93       -1     
  Lines        4942     5201     +259     
  Branches      410      405       -5     
==========================================
+ Hits         3199     3460     +261     
+ Misses       1727     1723       -4     
- Partials       16       18       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jakejarvis jakejarvis changed the title Refactor provider detection logic AST chore: Refactor provider detection logic AST Sep 30, 2025
@jakejarvis jakejarvis changed the title chore: Refactor provider detection logic AST Refactor provider detection logic AST Sep 30, 2025
@jakejarvis jakejarvis merged commit 5302bf0 into main Sep 30, 2025
5 checks passed
@jakejarvis jakejarvis deleted the feat/new-provider-rule-schema branch September 30, 2025 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants