fix: warn on empty destination include; correct README default docs - #100
Merged
Diego Colombo (colombod) merged 1 commit intoAug 20, 2026
Merged
Conversation
… default docs
The `include` fan-out pattern has always been fail-closed in code -- an
omitted or empty `include` matches NOTHING (the `Destination` docstring
and `fanout.py`'s `_matches`, which returns `False` on empty patterns) --
but the README documented the opposite default (`["**"]`, match
everything) in two places, and `validate_destinations()` never surfaced
the fail-closed default with any signal, silent or otherwise. A
destination with a forgotten `include` line silently receives nothing,
with no warning anywhere.
- README.md: correct the `include` sub-key's default-column entry and the
"Defaults & validation" paragraph to state the real fail-closed
behavior. The one already-correct statement ("empty include list ->
nothing") and the legacy-scalar synthesis note (`include: ["**"]`) are
left as-is and cross-checked for consistency with the corrected text.
- config_resolver.py: `validate_destinations()` now emits a per-destination
WARNING (not an error -- an inactive destination is a legal config, just
easy to produce by accident) when a destination that otherwise passes
validation has an empty `include`. The destination is NOT dropped from
the returned dict -- it remains reachable explicitly via the query
tools' `source=` override.
- tests: five new cases in `TestEmptyIncludeWarning` covering an omitted
`include`, an explicit `include: []`, a populated `include` (no
warning), the legacy-scalar synthesized destination (no warning --
always gets `("**",)`), and a destination already dropped for a bad
url/api_key (must not also emit the include warning).
Verified: modules/hook-context-intelligence unit suite (610 passed),
repo-root tests/ suite (753 passed), ruff check + format clean, pyright
clean on the touched module file.
Not included: a real DTU run. This repo's AGENTS.md requires DTU evidence
for seam-crossing changes; that run is a separate follow-up step, not
part of this commit.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Brian Krabach (bkrabach)
requested review from
Diego Colombo (colombod),
Marc Goodner (robotdad) and
Salil Das (sadlilas)
August 20, 2026 03:44
Diego Colombo (colombod)
approved these changes
Aug 20, 2026
Diego Colombo (colombod)
deleted the
fix/include-default-docs-and-validation
branch
August 20, 2026 09:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Small fail-closed-documentation + validation-warning fix for the destination fan-out
includeconfig.Bugs Fixed
Bug 1: README Documentation Contradiction
File reference:
README.mdThe README incorrectly claimed that an omitted
includedefaults to["**"](match everything) in two places:However, the actual code behavior is:
config_resolver.pydefinesDestination.include = ()(empty tuple as default)fanout._matches()returnsFalseon empty patterns → destination receives nothingThe README even contradicted itself within 30 lines — a third line correctly stated "empty include list → nothing (destination inactive)".
Resolution: All three statements now agree: an omitted
includefails closed to match nothing. The legacy-scalar note (correctly documenting that legacy config synthesizesinclude: ["**"]) was verified accurate and remains untouched.Why fail-closed is correct: An operator accidentally omitting
includeshould get a safe, inactive destination — not silent data loss. Documentation was updated to match code, not vice versa.Bug 2: Silent Validation of Inactive Destinations
File reference:
config_resolver.pyvalidate_destinations()The validator passed a silently-dead destination without any signal. An operator following the (incorrect) README and omitting
includewould end up with a destination that never receives anything, with no warning whatsoever.Resolution: Added a per-destination WARNING when
includeis empty: "this destination will never match any session... set include: ["**"] to receive all sessions".This is a warning, not an error — an inactive destination is legal (still reachable via the query tool with
source=), just surprising. The warning gives operators visibility into what they've accidentally created.Evidence
Unit tests (all passing):
ruffformatting: cleanpyrighttype checking: cleanNew test coverage (5 cases):
include→ warns[]→ warnsinclude→ no warning("**",)→ no warningImplementation Scope
No DTU run performed. This change consists of:
All changes are covered by unit tests. Reviewers who want DTU evidence should say so.
Related Work
This fix was discovered during the telemetry secret-leak investigation that produced microsoft/amplifier-bundle-redaction PR #9. The two are independent but related in origin.
Generated with Amplifier