Cask::Audit: reduce livecheck fetches

[samford]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

---

The existing livecheck-related cask audits can end up fetching the livecheck URL four times and this can cause problems if a server has sensitive rate limiting (e.g., Homebrew/homebrew-cask#241429 (comment)). This modifies the livecheck_version audit to cache the result that's also used in the hosting_with_livecheck and livecheck_https_availability audits. This will avoid a duplicate fetch and can avoid another if the livecheck URL uses HTTPS and the check doesn't fail. Most casks meet that criteria, so this should be a notable improvement.

There's still room for improvement, as the min_os audit calls cask_sparkle_min_os, which can also fetch the livecheck URL. It would be ideal if we could cache the livecheck content to avoid this duplicate fetch but I don't think that's possible with the current setup, so I'll have to revisit that later. [I have some in-progress work to add caching to livecheck and I may be able to leverage that in the future as a way of addressing this.]

[Copilot]

Pull request overview

This PR optimizes cask audit performance by reducing redundant livecheck URL fetches from up to three times down to one in most cases. This is achieved by introducing a caching mechanism for the livecheck version audit result.

• Adds @livecheck_result instance variable to cache livecheck audit results
• Modifies audit_livecheck_version to cache and reuse results across multiple audit methods
• Updates audit_hosting_with_livecheck to call audit_livecheck_version directly instead of accepting it as a parameter
• Adds early return optimization in audit_livecheck_https_availability to skip unnecessary checks when livecheck already uses HTTPS and succeeds

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The condition to skip the HTTPS availability check is potentially incorrect. When audit_livecheck_version returns nil (which happens when there's no cask.version), the condition audit_livecheck_version != false evaluates to true, causing the HTTPS availability check to be skipped even though the livecheck URL might still benefit from being checked for HTTPS availability.

The condition should explicitly check for the success cases (:skip or :auto_detected) rather than just checking if the result is not false. For example: [:skip, :auto_detected].include?(audit_livecheck_version)

Suggested change

	if url.start_with?("https:") && audit_livecheck_version != false
	if url.start_with?("https:") && [:skip, :auto_detected].include?(audit_livecheck_version)

[samford]

My intention is for the audit_livecheck_version != false condition to represent "livecheck ran and did not fail". As above, a nil value can occur from the return unless cask.version guard in audit_livecheck_version but that means livecheck didn't run.

The question is, should we still check for HTTPS availability for a livecheck block URL if there is no cask version? cask.version is necessary to perform the livecheck_version audit (comparing the cask version to the upstream version) but not the HTTPS availability audit, so I think it's fine. [That said, if we did go this route, I would approach it by also accounting for nil (the other non-success condition) rather than explicitly checking for success conditions, as that would need to be kept up to date when we add other values (I have a forthcoming PR that adds another symbol).]

I may be overlooking something here (it's somewhat tricky to reason about) and I can be convinced otherwise but I think this is okay.

[bevanjkay]

Is the purpose behind the https availability audit for livecheck, just to make sure we aren't using http when we should be https? Otherwise the audit probably isn't necessary at all, because we're almost always auditing the livecheck version anyway?

[samford]

From my understanding, it seems to be for checking 1) if an HTTP URL can be upgraded to HTTPS and 2) if an existing HTTPS URL is reachable. The latter situation is covered by the livecheck_version audit (this is the overlap I'm aiming to avoid here) but there's still value in checking if an HTTP URL can be upgraded to HTTPS. We only have a handful of cask livecheck block URLs that use HTTP, so it will be rare that the livecheck_https_availability audit will fetch the livecheck block URL but it's still necessary to keep it around.

[MikeMcQuaid]

Looks good once small nit addressed! Can self-merge without rereview.

[MikeMcQuaid]

Splitting this into multiple lines would be nice here; the assignment inline is a bit hard to follow.