Add thread-safety to HostMatcher and Tree insertion operations

[anfragment]

What does this PR do?

• Protects internal HostMatcher and Tree slices with mutexes
• Fixes HostMatcher correctness by appending generic exceptions to the correct slice
• Makes the hostmatcher package internal, as it should not be used publicly

How did you verify your code works?

New and existing unit tests.

What are the relevant issues?

Closes #41

[coderabbitai]

Walkthrough

The PR addresses thread-safety issues in HostMatcher and Tree by relocating hostmatch to an internal package, adding mutex synchronization for concurrent slice access, and updating all import paths to reflect this structural change.

Changes

Cohort / File(s)	Summary
Import path updates to internal hostmatch internal/asset/cosmetic/injector.go, internal/asset/cssrule/injector.go, internal/asset/extendedcss/injector.go, internal/asset/jsrule/injector.go, internal/asset/scriptlet/injector.go	Import path changed from external to internal hostmatch package; behavior and usage remain unchanged.
HostMatcher concurrency protection internal/hostmatch/hostmatcher.go	Adds mu sync.RWMutex field and applies read/write locks around mutations of generic and genericExceptions slices in AddPrimaryRule, AddExceptionRule, and Get methods.
HostMatcher test updates internal/hostmatch/hostmatcher_test.go	Updates import path to internal hostmatch package; adds two new subtests for generic exception rules and tilde-based exception matching behavior.
Tree concurrency fix internal/ruletree/ruletree.go	Adds mutex lock acquisition around empty-pattern insertions to generic slice, preventing data race with concurrent Insert and Get operations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• PR #28: Updates ruletree package import/visibility patterns, sharing structural refactoring concerns with this PR's internal package reorganization.

Suggested reviewers

• spellcascade

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description covers all required sections: what the PR does, verification method, and relevant issues. It clearly explains the thread-safety improvements and issue closure.
Linked Issues check	✅ Passed	The code changes fully address issue #41 requirements: mutex protection added to HostMatcher slices [hostmatcher.go], read locks in Get(), write locks in AddPrimaryRule/AddExceptionRule, ruletree insert protection [ruletree.go], and package moved to internal.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #41: four files update imports for the internal hostmatch package move, hostmatcher.go adds mutex protection, ruletree.go adds locking for concurrent safety, and test files add verification. No unrelated modifications present.
Title check	✅ Passed	The title clearly and accurately summarizes the main changes: adding thread-safety mechanisms to HostMatcher and Tree insertion operations through mutex protection.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch anfragment/hostmatcher-tree-safety

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[anfragment]

@spellcascade Publishing v1.3.0 after we merge this.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/ruletree/ruletree.go (1)

14-18: ⚠️ Potential issue | 🟡 Minor

Doc comment is inconsistent with PR objectives.

The comment states "Insert and Compact must not run concurrently with Get", but the PR objectives from issue #41 aim to make these operations safe for concurrent use. If the intention is to keep this limitation, the PR description should be updated. Otherwise, the implementation needs to guard Get() as well.

🤖 Fix all issues with AI agents

In `@internal/ruletree/ruletree.go`:
- Around line 40-44: The code has a data race because Insert appends to
t.generic under insertMu while Get reads t.generic without locking; change
insertMu from sync.Mutex to sync.RWMutex (or add a new RWMutex protecting
t.generic) and update Insert to use the write lock and Get to use a read lock
when accessing t.generic (i.e., call RLock()/RUnlock() around the slice read in
Get and Lock()/Unlock() in Insert) so concurrent Get() calls can safely read
while Insert() writes are exclusive.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eeaf873 and 2e11aa7.

📒 Files selected for processing (9)

• internal/asset/cosmetic/injector.go
• internal/asset/cssrule/injector.go
• internal/asset/extendedcss/injector.go
• internal/asset/jsrule/injector.go
• internal/asset/scriptlet/injector.go
• internal/hostmatch/hostmatcher.go
• internal/hostmatch/hostmatcher_test.go
• internal/hostmatch/triestore.go
• internal/ruletree/ruletree.go

🧰 Additional context used

🧬 Code graph analysis (1)

internal/hostmatch/hostmatcher_test.go (1)

internal/hostmatch/hostmatcher.go (1)

• NewHostMatcher (29-34)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Lint Go (windows-latest)

🔇 Additional comments (11)

internal/hostmatch/hostmatcher.go (3)

54-56: LGTM! Wildcard exception handling aligns with primary rules.

The automatic wildcard prefixing for exception patterns mirrors the behavior for primary rules (lines 61-63), ensuring consistent subdomain matching. This addresses the correctness fix mentioned in the PR objectives.

---

69-90: LGTM!

The locking for genericExceptions is correctly implemented, and the wildcard prefixing ensures consistent subdomain matching behavior.

---

92-97: No issue: trieStore has its own internal sync.RWMutex (defined at line 54 of triestore.go). Both Add() and Get() methods properly acquire locks, so all primaryStore and exceptionStore operations are thread-safe. The mu in HostMatcher is correctly scoped to protect only the generic and genericExceptions slices. There is no data race.

internal/asset/cosmetic/injector.go (1)

10-10: LGTM!

Import path correctly updated to the internal package location.

internal/asset/extendedcss/injector.go (1)

13-13: LGTM!

Import path correctly updated to the internal package location.

internal/asset/jsrule/injector.go (1)

9-9: LGTM!

Import path correctly updated to the internal package location.

internal/asset/scriptlet/injector.go (1)

10-10: LGTM!

Import path correctly updated to the internal package location.

internal/asset/cssrule/injector.go (1)

10-10: LGTM!

Import path correctly updated to the internal package location.

internal/hostmatch/hostmatcher_test.go (3)

6-6: LGTM!

Import path correctly updated to reflect the package relocation to internal/hostmatch.

---

126-140: LGTM!

This test correctly validates the correctness fix for generic exceptions. It ensures that a generic exception rule (empty pattern) properly neutralizes a generic primary rule, which directly addresses the bug mentioned in the PR objectives.

---

142-153: LGTM!

Good addition testing that tilde exceptions propagate correctly to deeper subdomain hierarchies. This complements the existing test at lines 97-111 which only tests direct subdomains and siblings.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}