refactor(thold): extract the notification delivery from the threshold check

[somethingwithproof]

Phase 3, first slice.

Stacked on #800, which is stacked on #799. Merge order is #799, #800, then this. Until they land this PR's diff carries theirs.

What moved

Every send site in thold_check_threshold() did the same two things: resolve the notification list's format file, then mail the list unless it was empty or the threshold had been acknowledged. There are 18 of those pairs.

That is now thold_mail_notification(). Five sites matched the shape exactly and call it; the other thirteen differ in ways that are themselves the subject of open issues, so they stay put until Phase 4 decides which form is correct.

Two details worth flagging, because both were judgement calls:

• The body is still built inside the guard, not by the caller. Composing it runs get_thold_alert_text(), which resolves the device's site through a query, so hoisting it would make an empty recipient list pay for a message nobody receives. The function takes the text class and builds lazily instead.
• The function returns the message. Sites that mail a second list reuse it exactly as before.

The one observable difference

Where the primary list is empty, $message is now '' rather than unset. The second mail already read that unset variable, so this removes an undefined-variable read. It cannot change what a mail contains — null and '' render identically in the body — but it is a difference and it belongs in the description rather than buried.

thold_check_threshold() goes from 1378 lines to 1343.

A correction to my own earlier analysis

Coverage flagged one converted line as unreached: the ALERT > WARNING branch in the hi/low arm. I had previously described that branch as unreachable, and #752 describes its $suspend_notify guard as structurally dead.

Rather than assume, I tried to reach it. It is reachable. With a threshold already in alert, a reading falling back into the warning band, and both fail counts at or above their triggers, it fires and logs ST_NOTIFYAW with an ALERT > WARNING subject. My earlier claim was wrong; the line was uncovered because none of the scenarios set both counters up that way.

There is now a characterization test for it. #752's narrower claim about the $suspend_notify guard is untouched by this and still stands on its own.

Verification

61 tests, 100% of changed lines covered. The characterization tests pass unchanged apart from the one added above. composer lint and composer test clean.

Next

The remaining thirteen sites differ on: whether a second list is mailed, whether a trigger command runs, which SNMP payload is built, and whether graph_timespan is passed. Each difference maps to a filed issue, so the rest of Phase 3 lands alongside Phase 4 rather than before it.

[Copilot]

Pull request overview

This PR is the next refactor slice for the Thold threshold evaluator, extracting repeated notification-delivery logic out of thold_check_threshold() while adding a PHPUnit-based characterization test harness and CI coverage gate to keep behavior pinned during future restructuring.

Changes:

• Extracted shared mail-delivery logic into thold_mail_notification() and reused it at several send sites.
• Centralized evaluation-time settings into thold_evaluation_context() to reduce duplicated config/lookup code.
• Added a PHPUnit unit-test harness (scenarios/outcomes + characterization tests), patch-level coverage enforcement, and a GitHub Actions workflow to run it in Docker.

Reviewed changes

Copilot reviewed 18 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
thold_functions.php	Adds thold_mail_notification() and thold_evaluation_context(), and updates some send sites in thold_check_threshold().
tests/Unit/ThresholdTimeBasedCharacterizationTest.php	Characterization tests for time-based thresholds to pin existing behavior.
tests/Unit/ThresholdHiLowCharacterizationTest.php	Characterization tests for hi/low thresholds including the ALERT→WARNING downgrade path.
tests/Unit/ThresholdBaselineCharacterizationTest.php	Characterization tests for baseline thresholds using reference statistics fixtures.
tests/Unit/TholdMailNotificationTest.php	Direct unit tests for the extracted thold_mail_notification() helper.
tests/Unit/TholdEvaluationContextTest.php	Direct unit tests for thold_evaluation_context() behavior (triggers, recipients, attachments).
tests/TestCase.php	Base PHPUnit TestCase for resetting stub state and loading plugin sources/constants.
tests/Helpers/ThresholdScenario.php	Scenario builder to drive one threshold through one poll of thold_check_threshold().
tests/Helpers/ThresholdOutcome.php	Outcome reader for asserting on side effects (mail/log/traps/persisted columns).
tests/Helpers/CactiStubs.php	Recording + programmable stub layer for global Cacti functions used by the plugin.
tests/fixtures/optional-core-functions.php	Optional core-function fixtures for exercising function_exists() branches.
tests/fixtures/cacti-lib/variables.php	Placeholder fixture file to satisfy runtime include_once() expectations in tests.
tests/docker/Dockerfile	Docker image for running unit tests + coverage consistently (PHP 8.1 + pcov).
tests/docker/docker-compose.yml	Local docker-compose runner matching CI behavior.
tests/bootstrap-unit.php	PHPUnit bootstrap providing global Cacti-function shims and constants for unit tests.
tests/bin/patch-coverage.php	Script to enforce coverage on changed PHP lines only (diff-based gate).
phpunit.xml	PHPUnit configuration (bootstrap, strictness, and source include).
composer.json	Adds dev dependencies (PHPUnit, phplint) and scripts for lint/test/coverage/docker.
.gitignore	Ignores vendor, PHPUnit cache, and coverage outputs.
.github/workflows/php-unit-tests.yml	GitHub Actions workflow to build the test container, run lint/tests/coverage, and enforce patch coverage.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[somethingwithproof]

@TheWitness @bmfmancini The branch has been rebuilt directly on current develop to resolve the merge conflicts while preserving the PR-specific commits. All required Pest and PHP 8.1–8.4 integration checks are green. The force-push dismissed the prior approval; please re-approve so this can be squash-merged.