Skip to content

advisory-match: add --rejected-candidates - #24224

Merged
p-linnane merged 1 commit into
advisory-split-intervalsfrom
advisory-rejected-candidates
Oct 11, 2026
Merged

p-linnane merged 1 commit into
advisory-split-intervalsfrom
advisory-rejected-candidates

Conversation

@p-linnane

Copy link
Copy Markdown
Contributor

Ingest deletes candidates listed in Homebrew/advisory-database's data/rejected-candidates.txt after matching, but brew advisory-match still walks their history on every platform. --rejected-candidates=<file> skips those new candidates first, using the same rules as Ingest's filter.

brew benchmark --exec over a seeded replay of ansible, httpie and poetry with frozen OSV responses (3 runs): 546.1 ± 24.0 s without the flag, 217.3 ± 1.6 s with it (main: 184.8 ± 4.0 s).


  • 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.

Codex (GPT-6 Astra) drafted the change and tests and ran brew lgtm --online; Claude Code (Opus 5.5) reviewed it, ran brew lgtm --online on this commit and ran the benchmark. I reviewed the diff.


Skip new candidates whose record IDs are listed in the given rejection
file before querying other platforms or walking history. Ingest deletes
these candidates after matching anyway. Existing records, aliases,
--no-history and reconciliation are unchanged.

Signed-off-by: Patrick Linnane <patrick@linnane.io>
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1aff0062-3206-4dbb-8daa-6f82bbdcd0e8

📥 Commits

Reviewing files that changed from the base of the PR and between 49036b9 and 76fdf66.


📒 Files selected for processing (7)
  • Library/Homebrew/dev-cmd/advisory-match.rb
  • Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/advisory_match.rbi
  • Library/Homebrew/test/dev-cmd/advisory-match_spec.rb
  • completions/bash/brew
  • completions/fish/brew.fish
  • completions/zsh/_brew
  • docs/Advisory-Matching.md

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


📜 Recent review details
⏰ Context from checks skipped due to timeout. (19)
  • GitHub Check: upload_sarif
  • GitHub Check: docker (x86_64 Ubuntu 24.04)
  • GitHub Check: docker (arm64 Ubuntu 26.04)
  • GitHub Check: docker (x86_64 Ubuntu 26.04)
  • GitHub Check: docker (arm64 Ubuntu 24.04)
  • GitHub Check: tests (load-only Linux)
  • GitHub Check: tests (macOS, 1/2)
  • GitHub Check: tests (generic OS, 2/2)
  • GitHub Check: tests (Linux, 1/2)
  • GitHub Check: tests (Linux, 2/2)
  • GitHub Check: tests (no Sorbet)
  • GitHub Check: tests (macOS, 2/2)
  • GitHub Check: tests (load-only macOS)
  • GitHub Check: tests (generic OS, 1/2)
  • GitHub Check: tests (online, 2/2)
  • GitHub Check: tests (online, 1/2)
  • GitHub Check: syntax
  • GitHub Check: docs
  • GitHub Check: Analyze

🧰 Additional context used
📚 Code guidelines (2)
AGENTS.md — auto-discovered
docs/AGENTS.md — auto-discovered

📓 Path-based instructions (2)
Source excerpt: `completions/` and `manpages/`: generated shell completions and `man` pages.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • completions/bash/brew
  • completions/zsh/_brew
  • completions/fish/brew.fish

Source excerpt: Use the system Homebrew `brew bundle exec ...`

📄 CodeRabbit inference engine (docs/AGENTS.md)

Files:

  • docs/Advisory-Matching.md


📝 Summary

Summary by CodeRabbit

  • New Features
    • Added the --rejected-candidates option to brew dev-cmd advisory-match. With --output, it skips listed new candidates during history processing and reports the rejected IDs.
    • Rejection lists support exact, case-sensitive IDs; blank lines, surrounding whitespace, comments, and duplicate entries are ignored.
    • Added command-line completion support for Bash, Fish, and Zsh.
  • Documentation
    • Documented how to use the option, its requirements, and how it affects candidate processing.

Walkthrough

Adds --rejected-candidates=<file> to brew dev-cmd advisory-match. With history enabled, the command filters listed new comparable candidates before further platform queries and history walks, then reports skipped IDs. Existing records continue through processing. The option requires --output.

Changes

Rejected candidate filtering

Layer / File(s) Summary
Read and apply rejected candidate IDs
Library/Homebrew/dev-cmd/advisory-match.rb, Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/advisory_match.rbi, Library/Homebrew/test/dev-cmd/advisory-match_spec.rb
The command reads and parses rejected IDs, then filters eligible candidates after alias preparation and reports skipped IDs. Tests cover ID matching, parsing, candidate order, and existing records.
Validate option behavior and document usage
Library/Homebrew/test/dev-cmd/advisory-match_spec.rb, docs/Advisory-Matching.md, completions/bash/brew, completions/fish/brew.fish, completions/zsh/_brew
Tests cover option errors and history modes. Documentation and shell completions describe the option and its behavior.

Priority: ⬇️ Low

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

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant AdvisoryMatch
  participant RejectedCandidatesFile
  participant AliasPreparation
  participant HistoryProcessing
  participant DirEmitter
  AdvisoryMatch->>RejectedCandidatesFile: Read and parse rejected IDs
  AdvisoryMatch->>AliasPreparation: Prepare actionable hits
  AliasPreparation-->>AdvisoryMatch: Return prepared hits
  AdvisoryMatch->>AdvisoryMatch: Filter eligible listed candidates
  AdvisoryMatch->>HistoryProcessing: Process retained candidates
  HistoryProcessing->>DirEmitter: Emit candidate output
  AdvisoryMatch->>DirEmitter: Finish and report skipped IDs
Loading

Suggested reviewers: andrew


Merge Risk | ⚪ Minimal · up to 76fdf

Merge Risk: ⚪ Minimal · up to 76fdf

The new option appears mergeable after normal checks. No actionable mismatch in its documented candidate-filtering behavior was established.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 76fdf

The rejection list is explicitly supplied by the command operator and only suppresses eligible new candidates. Existing records, reviewed state, and protected aliases retain their normal processing. The reviewed change does not introduce a material security risk.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The newly selectable outcome is omission of eligible new advisory records and their initial history work within the chosen command run and output directory. The gate does not authorize deletion of existing records or suppress their normal checks.

Trust Boundaries and Controls

  • inferred — The command operator explicitly selects the local suppression policy. OSV-derived identity is compared against that policy through the existing canonical record-ID contract; advisory data does not select or fetch the rejection list. The inspected change therefore does not grant a new lower-trust source authority over the output workflow.
  • observed — Protected aliases, reviewed range state, and existing alias-target files prevent prefiltering. Reconciliation branches separately before this gate, preserving the existing authority and conditions for revisiting or deleting persisted records.

Resilience and Maintainability Implications

  • observed — The PR leaves emitter persistence logic unchanged, including direct file writes. External serialization of concurrent invocations and crash-recovery guarantees remain unverified baseline conditions, rather than demonstrated protections or newly introduced concerns.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly identifies the main change: adding the --rejected-candidates option to advisory-match.
Description check Passed The description explains the option's purpose, behavior, performance impact, testing, and validation steps. It is directly related to the changeset.
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.

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

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

@p-linnane
p-linnane added this pull request to stack #24226 October 11, 2026 06:15
@p-linnane
p-linnane added this pull request to the merge queue Oct 11, 2026
Merged via the queue into main with commit 2cbb569 Oct 11, 2026
54 checks passed
@p-linnane
p-linnane deleted the advisory-rejected-candidates branch October 11, 2026 16:16
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