[p-linnane]

brew test-bot's dependency impact annotation compared an added dependency only against the formula's other current dependencies, so replacing openssl@3 with openssl@4 was reported as two new recursive dependencies: the removed openssl@3 and the ca-certificates both versions share weren't taken into account.

It now compares the formula's recursive runtime dependencies at origin/HEAD and HEAD, loading formulae changed or deleted in the pull request from origin/HEAD, and writes one annotation listing added and removed dependencies with the net installed size change. A missing size for a removed dependency makes the change unknown and one for an added dependency gives a lower bound. An unchanged dependency set gets no annotation.

Replaying Homebrew/homebrew-core#316286 (202 formulae moved to openssl@4) on macOS, 199 formulae are now annotated with openssl@4 added and openssl@3 removed; previously 57 reported two new dependencies, two reported one and 143 reported nothing.

---

• 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; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

GPT-6 Astra drafted the implementation and tests and ran brew lgtm --online. Claude Code (Opus 5.5) reviewed each revision, reproducing its findings in throwaway taps with real Git history, and replayed Homebrew/homebrew-core#316286 against the final version. I reviewed the diff.

---

[coderabbitai]

📝 Summary

Summary by CodeRabbit

• New Features
  • Dependency impact annotations now compare the full recursive runtime dependency sets between revisions. They report added and removed dependencies, the net count change, and the change in installed size.
  • Annotations account for shared dependencies and dependency changes involving renamed formulas. When installed-size data is unavailable, the size impact is reported as unknown.

Walkthrough

Formulae#annotate_added_dependencies now compares recursive runtime dependencies between origin/HEAD and HEAD. Its annotation reports added and removed dependencies, net count, and estimated installed-size change.

Changes

Recursive dependency impact reporting

Layer / File(s)	Summary
Load and traverse historical dependency sets Library/Homebrew/test_bot/formulae.rb, Library/Homebrew/test/test_bot/formulae_spec.rb	Formulae loads and caches historical formula definitions and traverses recursive runtime dependencies. Tests cover history-backed dependency resolution, optional dependencies, and tap-qualified names.
Compare and report dependency impact Library/Homebrew/test_bot/formulae.rb, Library/Homebrew/test/test_bot/formulae_spec.rb	The annotation compares added and removed recursive dependencies and reports their names, counts, and estimated installed-size change. Tests cover replacements, shared dependencies, unchanged sets, and unknown sizes.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Suggested reviewers: mikemcquaid

---

Merge Risk | 🔵 Low · up to 31459

Merge Risk: 🔵 Low · up to 31459

The dependency annotation can report an inaccurate size change in this narrow case. The PR is mergeable with owner awareness, though historical sizes should be used for removed dependencies.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 31459

The change evaluates historical formula definitions through existing loading controls. No new privilege or control bypass was established, but the permissions and isolation of the CI runner were not available for assessment.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The additional evaluation inputs are historical formula definitions selected from the owning tap's changed/deleted paths. Evaluation occurs in the existing test-bot process, so its effective filesystem, network, and credential exposure depends on runner permissions not established by the available context.

Trust Boundaries and Controls

• observed — Historical loading reuses the existing configurable tap/formula trust gate and environment masking. These are not an execution sandbox: trust enforcement has existing bypass conditions, and HOMEBREW_GITHUB_API_TOKEN is expressly exempt from masking. Neither control nor exception changes in this PR.
• observed — Current transitive formula execution predates the PR. The base reporting helper invokes runtime_dependencies, whose declared-runtime path recursively resolves formulae through Dependency.expand. Recursive depth alone therefore does not establish a newly introduced attack path.

Resilience and Maintainability Implications

• observed — Historical contents use a content-derived namespace rather than the current path-loader namespace and do not populate the current path-class cache. The evaluator restores stdout, while its environment wrapper restores the prior environment through ensure. These mechanisms limit accidental loader-state contamination, not arbitrary Ruby side effects.

---

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: reporting net dependency impact in test-bot.
Description check		The description directly explains the dependency comparison changes, annotation behavior, size handling, tests, and validation results.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[p-linnane]

@cho-m This was inspired by your OpenSSL PRs.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: cffd2144-4f35-4b1a-96a6-00f44d7c5126

📥 Commits

Reviewing files that changed from the base of the PR and between d779f8e and 314597c.

📒 Files selected for processing (2)

• Library/Homebrew/test/test_bot/formulae_spec.rb
• Library/Homebrew/test_bot/formulae.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.

📜 Review details

⏰ Context from checks skipped due to timeout. (19)

• GitHub Check: docker (arm64 Ubuntu 24.04)
• GitHub Check: docker (x86_64 Ubuntu 26.04)
• GitHub Check: docker (arm64 Ubuntu 26.04)
• GitHub Check: docker (x86_64 Ubuntu 24.04)
• GitHub Check: tests (load-only macOS)
• GitHub Check: tests (macOS, 1/2)
• GitHub Check: tests (generic OS, 2/2)
• GitHub Check: tests (generic OS, 1/2)
• GitHub Check: tests (Linux, 2/2)
• GitHub Check: tests (macOS, 2/2)
• GitHub Check: tests (online, 2/2)
• GitHub Check: tests (no Sorbet)
• GitHub Check: tests (load-only Linux)
• GitHub Check: tests (Linux, 1/2)
• GitHub Check: tests (online, 1/2)
• GitHub Check: workflow_syntax
• GitHub Check: syntax
• GitHub Check: docs
• GitHub Check: Analyze

🧰 Additional context used

🪛 ast-grep (0.45.3)

Library/Homebrew/test/test_bot/formulae_spec.rb

[warning] 161-163: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: allow(Utils).to receive(:safe_popen_read)
.with("git", "-C", repository, "diff", "--no-ext-diff", "--unified=0",
"origin/HEAD", "HEAD", "--", formula.path.relative_path_from(repository).to_s).and_return <<~DIFF
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures

(hardcoded-secret-rsa-passphrase-ruby)

---

[warning] 161-163: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: receive(:safe_popen_read)
.with("git", "-C", repository, "diff", "--no-ext-diff", "--unified=0",
"origin/HEAD", "HEAD", "--", formula.path.relative_path_from(repository).to_s)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures

(hardcoded-secret-rsa-passphrase-ruby)

---

[warning] 363-366: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: allow(Utils).to receive(:safe_popen_read)
.with("git", "-C", repository, "diff", "--no-ext-diff", "--unified=0",
"origin/HEAD", "HEAD", "--", current.path.relative_path_from(repository).to_s)
.and_return <<~DIFF
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures

(hardcoded-secret-rsa-passphrase-ruby)

---

[warning] 363-365: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: receive(:safe_popen_read)
.with("git", "-C", repository, "diff", "--no-ext-diff", "--unified=0",
"origin/HEAD", "HEAD", "--", current.path.relative_path_from(repository).to_s)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures

(hardcoded-secret-rsa-passphrase-ruby)

---

[warning] 436-439: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: allow(Utils).to receive(:safe_popen_read)
.with("git", "-C", repository, "diff", "--no-ext-diff", "--unified=0",
"origin/HEAD", "HEAD", "--", current.path.relative_path_from(repository).to_s)
.and_return <<~DIFF
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures

(hardcoded-secret-rsa-passphrase-ruby)

---

[warning] 436-438: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: receive(:safe_popen_read)
.with("git", "-C", repository, "diff", "--no-ext-diff", "--unified=0",
"origin/HEAD", "HEAD", "--", current.path.relative_path_from(repository).to_s)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures

(hardcoded-secret-rsa-passphrase-ruby)

🔇 Additional comments (5)

Library/Homebrew/test_bot/formulae.rb (3)

582-582: Pass the dependency's own spec context when loading historical formulae.

recursive_runtime_dependency_names calls dependency_impact_formula without spec:, so every transitive dependency loads with the default :stable spec. The top-level formula, by contrast, uses formula.active_spec_sym. For a formula tested with HEAD or devel specs, the previous set comes from the formula's own active spec, but its dependencies come from :stable. The current side also resolves through :stable via Formulary.factory(name, :stable). Both sides are consistent with each other, so this does not cause a false diff. The report can still describe a different graph than the one actually tested. This is low impact, and I am not raising it as a defect.

---

243-259: The size lookup uses current formulae for removed dependencies.

Formulary.factory(formula_name) resolves removed dependencies through the current tap. A removed dependency that was deleted in the PR raises FormulaUnavailableError. The code then rescues the error and treats the size as unknown. The total becomes "unknown", and the tests expect that result. This matches the stated design.

---

43-43: LGTM!

Also applies to: 225-241, 260-284, 545-571, 573-595

Library/Homebrew/test/test_bot/formulae_spec.rb (2)

161-167: This static-analysis hint is a false positive.

The hardcoded-secret-rsa-passphrase-ruby hints flag a git diff heredoc stub. The stub contains no credential. No change is needed.

---

94-122: LGTM!

Also applies to: 168-184, 186-333, 335-453

[MikeMcQuaid]

Good idea!