Skip to content

Type cask audit options instead of **options - #24542

Merged
dduugg merged 1 commit into
Homebrew:mainfrom
dduugg:cask-audit-typed-options
Nov 8, 2026
Merged

dduugg merged 1 commit into
Homebrew:mainfrom
dduugg:cask-audit-typed-options

Conversation

@dduugg

@dduugg dduugg commented Nov 8, 2026

Copy link
Copy Markdown
Member
  • Pass explicit keywords to validate_url_for_https_availability so brew typecheck catches a mistyped keyword at its three callers, which options: T.untyped hid.
  • Narrow Cask::URL#user_agent to non-nil, since URL#initialize always stores user_agent || :default.
  • Pass the binary URL's referer to curl_check_http_content as a String, as SystemCommand already did when building the curl arguments.
  • Cover the binary URL HTTPS audit with and without a referer in unit tests.

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include brew benchmark results.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; every commit has a human author or co-author and I will answer maintainer questions and review comments myself without AI/LLM.

Claude Code with Opus 5.5, with local review and testing.


- `Cask::Audit#validate_url_for_https_availability` took `**options` as
  `T.untyped` and forwarded it to `curl_check_http_content`, so a
  mistyped keyword at any of its three callers only failed at runtime.
- Accept only the four keywords the callers pass, with `user_agents:`
  required since every caller passes it.
- Narrow `URL#user_agent` to non-nil: `URL#initialize` always stores
  `user_agent || :default`.
- Pass `url.referer&.to_s`, since `referer` can be a `URI`.
  `SystemCommand` already stringified it, so `curl` gets the same
  arguments.
- Cover the binary URL HTTPS audit with and without a referer.
@coderabbitai

coderabbitai Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability of HTTPS availability checks when a referring URL is provided, including when no referrer is available.
    • Cask URLs now consistently use a valid user-agent value, with a default applied when one is not supplied.
  • Tests
    • Added coverage for binary URL HTTPS checks with and without a referring URL, including verification of the relevant check options.

The cask URL user-agent type is now non-nullable. HTTPS availability audits convert referers to strings and pass explicit keyword options to the HTTP content check. Tests cover binary URL audits with and without a referer.

Changes: Cask HTTPS availability audit

Layer / File(s) Summary
Cask URL user-agent type
Library/Homebrew/cask/url.rb
The user_agent reader and stored value use a non-nullable type. Initialization retains the :default fallback.
HTTPS audit argument forwarding
Library/Homebrew/cask/audit.rb, Library/Homebrew/test/cask/audit_spec.rb
The audit converts the referer to a string. The validation method forwards explicit keyword options to curl_check_http_content. Tests cover binary URL checks with absent and present referers.
Merge Risk: Minimal · up to ae4d3

No actionable issue is established for the cask HTTPS audit changes; the PR is mergeable after normal checks.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title 'Type cask audit options instead of **options' matches the main change. The PR replaces the variadic **options in validate_url_for_https_availability with explicit typed keyword parame…
Description check Passed The description states the purpose of each change: explicit keywords for validate_url_for_https_availability, narrowing Cask::URL#user_agent to non-nil, passing the referer as a String, and adding…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title 'Type cask audit options instead of **options' matches the main change. The PR replaces the variadic **options in validate_url_for_https_availability with explicit typed keyword parameters, which the raw summary and objectives both confirm.

Full details: Description check

Explanation

The description states the purpose of each change: explicit keywords for validate_url_for_https_availability, narrowing Cask::URL#user_agent to non-nil, passing the referer as a String, and adding unit tests. These match the raw summary and objectives.

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 239aabc1-c24a-4f7d-8a74-80dbb0fa1298
Commits

Reviewing files that changed from the base of the PR and between 25ccd49 and ae4d31c.

Files selected for processing: 3

  • Library/Homebrew/cask/audit.rb
  • Library/Homebrew/cask/url.rb
  • Library/Homebrew/test/cask/audit_spec.rb

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@dduugg
dduugg enabled auto-merge November 8, 2026 22:28
@dduugg
dduugg added this pull request to the merge queue Nov 8, 2026
Merged via the queue into Homebrew:main with commit ee7fa78 Nov 8, 2026
54 of 55 checks passed
@dduugg
dduugg deleted the cask-audit-typed-options branch November 8, 2026 23:01
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

⌂ 换网址