Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ The `config` dict passed to `mount()` uses the same keys as the `overrides.hook-
|---------|----------|---------|-------------|
| `url` | yes | — | Base URL of the CI server for this destination. |
| `api_key` | yes | — | Bearer token for this destination. Sent as `Authorization: Bearer <value>` on that destination's POSTs only — because each destination references its own `${VAR}`, distinct keys never cross between servers. |
| `include` | no | `["**"]` | `.gitignore`-style patterns matched against the session's working directory. The destination is a candidate when any pattern matches. |
| `include` | no | *(none — matches nothing)* | `.gitignore`-style patterns matched against the session's working directory. The destination is a candidate when any pattern matches; an omitted or empty `include` matches **nothing** (fail-closed) and the destination is inactive — set `include: ["**"]` explicitly to match every session. |
| `exclude` | no | `[]` | `.gitignore`-style patterns; if any matches, the destination is dropped for that session (**exclude wins**). |

```yaml
Expand Down Expand Up @@ -309,7 +309,7 @@ overrides:

Prefer the trailing-slash directory form (e.g. `**/work/`) to mean "this project and all its sessions" — it matches whether the session is launched from the project **root** or any subdirectory. (A pattern that targets only contents, like `**/work/**`, still also matches the directory itself here, because the match key is a directory.)

**Defaults & validation.** Omitted `include` defaults to `["**"]` (match everything); omitted `exclude` defaults to none. After `${VAR}` expansion, a `destinations` entry whose `url` **or** `api_key` is empty is a **mount error** (fail-fast, naming the offending destination). With no `destinations` configured, the hook is local-JSONL-only. (The legacy scalar path is intentionally more lenient — see [Deprecated — legacy single-server scalars](#deprecated--legacy-single-server-scalars) below.)
**Defaults & validation.** An omitted or empty `include` matches **nothing** — the destination never receives events (fail-closed); to receive everything, set `include: ["**"]` explicitly. Validation logs a per-destination WARNING when this happens (an inactive destination is legal, just easy to configure by accident). Omitted `exclude` defaults to none. After `${VAR}` expansion, a `destinations` entry whose `url` **or** `api_key` is empty is a **mount error** (fail-fast, naming the offending destination). With no `destinations` configured, the hook is local-JSONL-only. (The legacy scalar path synthesizes its single `default` destination with `include: ["**"]` — see [Deprecated — legacy single-server scalars](#deprecated--legacy-single-server-scalars) below — and is intentionally more lenient on url/api_key.)

**Per-project override.** Because `destinations` is keyed by name, a project `.amplifier/settings.yaml` can override a single destination's `include`/`exclude` (e.g. `destinations.team.include`) without restating the others — the app-cli deep-merges user → project settings.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,20 @@ def validate_destinations(self) -> dict[str, Destination]:
configured any destination at all (the latter is this method's
ordinary, silent local-only case per the Returns note below).

Empty-include WARNING (fail-closed surprise, not a failure): a
destination that survives url/api_key/auth_mode validation but has
an empty ``include`` (omitted or ``[]`` \u2014 see the ``Destination``
docstring above) is legal \u2014 it is simply permanently inactive, since
an empty pattern set matches no session (``fanout.py``'s
``_matches`` returns ``False`` on empty patterns). That is easy to
configure by accident (a hand-typed destination with no ``include``
line looks identical to one meant to match everything), so it gets
its own per-destination WARNING naming the destination \u2014 distinct
from the misconfigured-url/api_key path above, and it does NOT drop
the destination from the returned dict (it remains a legitimate,
just currently inactive, target \u2014 e.g. still selectable by name via
the query tools' ``source=`` override).

Returns:
The validated (surviving) destinations dict \u2014 possibly empty,
which is always OK (local-only, S4). Never raises.
Expand Down Expand Up @@ -793,6 +807,25 @@ def validate_destinations(self) -> dict[str, Destination]:
)
continue

if not dest.include:
# Fail-closed by design (see the Destination docstring and
# fanout.py's _matches): an empty include pattern set means
# this destination will never match any session. That is a
# legal configuration (e.g. a destination meant only to be
# reached explicitly via the query tools' source= override),
# but it is also easy to produce by accident, so it gets its
# own WARNING -- distinct from the misconfigured-url/api_key
# path above -- and does NOT drop the destination.
log.warning(
"context-intelligence destination %r: no include patterns "
"-- this destination will never match any session and "
'will receive nothing; set include: ["**"] to receive '
"all sessions. Fix under "
"overrides.hook-context-intelligence.config.destinations.%s.include.",
name,
name,
)

valid[name] = dest

if dests and not valid:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,3 +250,82 @@ def test_unknown_auth_mode_dropped_and_logged_as_error(
assert result == {}
errors = [rec for rec in caplog.records if rec.levelno == logging.ERROR]
assert any("weird" in rec.message and "kerberos" in rec.message for rec in errors)


class TestEmptyIncludeWarning:
"""A destination with an empty ``include`` (omitted or ``[]``) is fail-closed --
it matches no session (see ``Destination`` docstring, ``fanout.py``'s ``_matches``)
-- but that is a LEGAL configuration, not a validation failure. It must still be
a WARNING (not an error), must NOT drop the destination from the returned dict,
and must not fire for a destination whose ``include`` is actually populated
(explicitly, or via the legacy-scalar synthesis, which always sets ``("**",)``)."""

def test_omitted_include_warns_and_destination_survives(
self, caplog: pytest.LogCaptureFixture
) -> None:
r = _resolver({"destinations": {"quiet": {"url": "http://q:8000", "api_key": "qk"}}})
with caplog.at_level(logging.WARNING):
result = r.validate_destinations()
assert set(result.keys()) == {"quiet"}
warnings = [rec for rec in caplog.records if rec.levelno == logging.WARNING]
assert any(
"quiet" in rec.message and "no include patterns" in rec.message for rec in warnings
)

def test_explicit_empty_include_list_warns_and_destination_survives(
self, caplog: pytest.LogCaptureFixture
) -> None:
r = _resolver(
{"destinations": {"quiet": {"url": "http://q:8000", "api_key": "qk", "include": []}}}
)
with caplog.at_level(logging.WARNING):
result = r.validate_destinations()
assert set(result.keys()) == {"quiet"}
assert any(
"quiet" in rec.message and "no include patterns" in rec.message
for rec in caplog.records
)

def test_nonempty_include_does_not_warn(self, caplog: pytest.LogCaptureFixture) -> None:
r = _resolver(
{
"destinations": {
"active": {
"url": "http://a:8000",
"api_key": "ak",
"include": ["**"],
}
}
}
)
with caplog.at_level(logging.WARNING):
result = r.validate_destinations()
assert set(result.keys()) == {"active"}
assert not any("no include patterns" in rec.message for rec in caplog.records)

def test_legacy_scalar_synthesized_destination_does_not_warn(
self, caplog: pytest.LogCaptureFixture
) -> None:
"""The legacy scalar path always synthesizes include=("**",) -- it must
never trigger the empty-include warning."""
r = _resolver(
{
"context_intelligence_server_url": "http://legacy:8000",
"context_intelligence_api_key": "legacy-key",
}
)
with caplog.at_level(logging.WARNING):
result = r.validate_destinations()
assert set(result.keys()) == {"default"}
assert not any("no include patterns" in rec.message for rec in caplog.records)

def test_dropped_destination_does_not_also_get_include_warning(
self, caplog: pytest.LogCaptureFixture
) -> None:
"""A destination dropped for a bad url/api_key must not ALSO emit the
empty-include warning -- it never reaches that check."""
r = _resolver({"destinations": {"broken": {"url": "", "api_key": "k"}}})
with caplog.at_level(logging.WARNING):
result = r.validate_destinations()
assert result == {}
assert not any("no include patterns" in rec.message for rec in caplog.records)
Loading