Skip to content

fix: stop the provider-drift gate asserting third-party tenant presence - #72

Merged
blisspixel merged 1 commit into
mainfrom
fix/provider-drift-tenant-presence-assertion
Aug 5, 2026
Merged

fix: stop the provider-drift gate asserting third-party tenant presence#72
blisspixel merged 1 commit into
mainfrom
fix/provider-drift-tenant-presence-assertion

Conversation

@blisspixel

Copy link
Copy Markdown
Owner

What went red

The scheduled Provider drift job failed on 2026-08-05, its first failure in four runs. test_resolve_second_reserved_domain failed on:

assert 'Could not resolve display name or default domain' is None
 +  where ... = SourceResult(source_name='user_realm', ...).error

Nothing regressed

example.org no longer has a third-party Microsoft 365 tenant registered against it. Verified live against both endpoints:

Domain GetUserRealm OIDC discovery
example.com State: 3, NameSpaceType: Federated 200, tenant c7c08208-…
example.org State: 4, NameSpaceType: Unknown 400 AADSTS90002: Tenant 'example.org' not found

UserRealmSource handles that correctly. Unknown is not in _TENANT_NAMESPACE_TYPES, Autodiscover returns no domains, so has_data is false and the source returns a stable negative: error set, source_unavailable false, degraded_sources empty.

That split is deliberate and documented on the model (models.py:679):

True only when the source had no observation opportunity because of a transport, protocol, or internal failure. A stable negative response uses error with this flag false. The distinction prevents failed sources from being interpreted as observed absence during merge and delta.

source_unavailable is what merger.py:459 consumes for the degraded set, and SourceResult.error only reaches a user through _raise_if_all_sources_failed, which requires every source to error. DNS succeeded here, so the negative was never surfaced as a failure. No shipped behavior changed and no user-visible output was ever wrong.

What was actually wrong

The gate asserted user_realm.error is None, which is asserting that a tenant exists. That is third-party registration state. example.com passes only because someone currently holds a Federated tenant on it, and that can lapse exactly the way example.org's did.

The helper's own docstring already warned about this — "third parties have been observed registering M365 tenants against example.com, so the state of that specific field is outside our control" — while the shared assertion did the opposite.

The fix

Assert two things the project does control:

  1. Transport and parse healthsource_unavailable is False and no identity:user_realm degradation marker.
  2. Agreement with the provider — read NameSpaceType straight from GetUserRealm in the test, then require recon's auth_type to match it under recon's own _TENANT_NAMESPACE_TYPES.

The raw read also asserts the response is a JSON object carrying a string NameSpaceType. That is the contract UserRealmSource parses, so a field rename or shape change now fails loudly instead of silently producing no auth type while the source still looks healthy — a drift class the old assertion could not distinguish from tenant churn.

Why this is stricter, not weaker

Both branches are exercised today:

example.com: NameSpaceType='Federated' -> tenant-positive branch=True
example.org: NameSpaceType='Unknown'   -> tenant-positive branch=False

example.com takes the exact-agreement assertion, which the old gate never made. Neither branch depends on who owns a tenant.

Verification

  • uv run pytest tests/test_integration.py -m integration -q — 3 passed
  • uv run python scripts/check.py — all 26 stages pass, coverage 91.41% against the 90.2% floor

The scheduled drift job went red on 2026-08-05. `test_resolve_second_reserved_domain`
failed on `user_realm.error is None` for example.org.

Nothing regressed. example.org no longer has a third-party Microsoft 365 tenant
registered against it, so GetUserRealm answers HTTP 200 with
`NameSpaceType: Unknown` and Autodiscover returns no domains. `UserRealmSource`
correctly reports that as a stable negative: `error` set, `source_unavailable`
false, no degraded sources. `SourceResult.source_unavailable` documents exactly
this split, and it is the field merge and delta consume, so the negative was
never presented to a user as a failed source.

The gate was asserting `error is None`, which is asserting that a tenant exists.
example.com only passed because a third party currently holds a Federated tenant
on it; that registration can lapse the same way example.org's did. The helper's
own docstring already warned that tenant state is outside this project's
control, but the assertion contradicted it.

Replace tenant presence with two things the project does control:

- transport and parse health, via `source_unavailable` and the absence of an
  `identity:user_realm` degradation marker
- agreement between recon's parse and the raw provider response, read directly
  from GetUserRealm in the test

The raw read also asserts the response is a JSON object carrying a string
`NameSpaceType`. That is the contract `UserRealmSource` parses, so a rename or
shape change now fails loudly instead of silently yielding no auth type while
the source still looks healthy. Both branches stay exercised today: example.com
returns Federated and takes the agreement assertion, example.org returns Unknown
and takes the stable-negative assertion.

Net effect is a stricter gate with no coupling to tenant churn.
Copilot AI lite review requested due to automatic review settings August 5, 2026 18:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the scheduled provider-drift integration gate so it no longer asserts third-party Microsoft 365 tenant presence on reserved domains, and instead validates provider contract shape plus recon’s interpretation alignment.

Changes:

  • Adds a raw GetUserRealm read in integration tests to treat NameSpaceType as drift-gate ground truth.
  • Updates the reserved-domain health assertion to check transport/parse health and to compare auth_type against the provider’s NameSpaceType when tenant-positive.
  • Documents the drift-gate behavior change in CHANGELOG.md.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/test_integration.py Reworks provider-drift assertions to avoid tenant-presence coupling; adds direct GetUserRealm contract check and auth-type agreement check.
CHANGELOG.md Records the provider-drift gate fix and clarifies that shipped behavior was unchanged.
Suppressed comments (1)

tests/test_integration.py:80

  • In the tenant-positive branch (NameSpaceType in _TENANT_NAMESPACE_TYPES), also assert user_realm.error is None, and in the stable-negative branch assert user_realm.error is not None. Without these, a regression could set error while still leaving auth_type unset/set and the gate wouldn’t distinguish a stable negative from an inconsistent SourceResult state.
    if realm_namespace in _TENANT_NAMESPACE_TYPES:
        assert user_realm.auth_type == realm_namespace
        assert "user_realm" in info.sources
    else:
        # A stable negative. Per ``SourceResult.source_unavailable``, that case

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_integration.py
Comment on lines +49 to +53
payload = response.json()
assert isinstance(payload, dict), "GetUserRealm no longer returns a JSON object"
assert "NameSpaceType" in payload, "GetUserRealm no longer reports NameSpaceType"
namespace = payload["NameSpaceType"]
assert isinstance(namespace, str), "GetUserRealm NameSpaceType is no longer a string"
@blisspixel
blisspixel merged commit 07a1ff8 into main Aug 5, 2026
28 checks passed
@blisspixel
blisspixel deleted the fix/provider-drift-tenant-presence-assertion branch August 5, 2026 18:27
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