chore: address review-round-5 — stale _sync_derived_flags reference + missing migration-hint test
Two findings from the reviewer's latest pass: - config_validators.py:329: comment in the LOGIN_FLOW validation block still referenced `_sync_derived_flags` (removed in commit 5; derivation now lives in `Settings.__post_init__`). Updated the comment to point at the correct location so a future reader grepping for the function name doesn't come up empty. - tests/unit/test_config_validators.py: added `test_oauth_single_audience_migration_hint` next to the existing `test_invalid_deployment_mode_raises_error`. The new test pins the ADR-022 rename-hint branch in `detect_auth_mode` by setting `MCP_DEPLOYMENT_MODE=oauth_single_audience` and asserting the ValueError mentions both the old and new mode names plus "ADR-022". Without this, a future refactor could drop the hint without any test catching it (the prior `invalid_mode` test only asserts the generic "Valid values:" prefix). No functional changes; 1010 unit tests now pass (+1 from the new hint test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
1fa4c82fd2
commit
0fb21b5c6d
@@ -325,8 +325,8 @@ def validate_configuration(settings: Settings) -> tuple[AuthMode, list[str]]:
|
|||||||
# ADR-022 follow-up: the un-augmented OAuth bearer pass-through (the
|
# ADR-022 follow-up: the un-augmented OAuth bearer pass-through (the
|
||||||
# old OAUTH_SINGLE_AUDIENCE without ENABLE_LOGIN_FLOW) needed unmerged
|
# old OAUTH_SINGLE_AUDIENCE without ENABLE_LOGIN_FLOW) needed unmerged
|
||||||
# Nextcloud user_oidc patches and is no longer supported. The
|
# Nextcloud user_oidc patches and is no longer supported. The
|
||||||
# `enable_login_flow` flag is now derived from the resolved mode by
|
# `enable_login_flow` flag is now derived from the resolved mode in
|
||||||
# `_sync_derived_flags`, so users only configure the mode — no
|
# `Settings.__post_init__`, so users only configure the mode — no
|
||||||
# separate ENABLE_LOGIN_FLOW env var is needed.
|
# separate ENABLE_LOGIN_FLOW env var is needed.
|
||||||
|
|
||||||
# If OAuth credentials not provided, DCR must be available
|
# If OAuth credentials not provided, DCR must be available
|
||||||
|
|||||||
@@ -788,6 +788,34 @@ class TestExplicitModeSelection:
|
|||||||
assert "invalid_mode" in str(e)
|
assert "invalid_mode" in str(e)
|
||||||
assert "Valid values:" in str(e)
|
assert "Valid values:" in str(e)
|
||||||
|
|
||||||
|
def test_oauth_single_audience_migration_hint(self):
|
||||||
|
"""ADR-022: rejecting `oauth_single_audience` surfaces a rename hint.
|
||||||
|
|
||||||
|
Pins the special-case branch in detect_auth_mode that helps users
|
||||||
|
upgrading from ADR-021 configurations spot the rename without
|
||||||
|
having to grep the changelog.
|
||||||
|
"""
|
||||||
|
with patch.dict(
|
||||||
|
os.environ,
|
||||||
|
{
|
||||||
|
"NEXTCLOUD_HOST": "http://localhost:8080",
|
||||||
|
"MCP_DEPLOYMENT_MODE": "oauth_single_audience",
|
||||||
|
},
|
||||||
|
clear=True,
|
||||||
|
):
|
||||||
|
from nextcloud_mcp_server.config import get_settings
|
||||||
|
|
||||||
|
_reload_config()
|
||||||
|
settings = get_settings()
|
||||||
|
|
||||||
|
with pytest.raises(ValueError) as exc:
|
||||||
|
detect_auth_mode(settings)
|
||||||
|
|
||||||
|
msg = str(exc.value)
|
||||||
|
assert "oauth_single_audience" in msg
|
||||||
|
assert "login_flow" in msg
|
||||||
|
assert "ADR-022" in msg
|
||||||
|
|
||||||
def test_explicit_mode_overrides_auto_detection(self):
|
def test_explicit_mode_overrides_auto_detection(self):
|
||||||
"""Test explicit mode takes precedence over auto-detection."""
|
"""Test explicit mode takes precedence over auto-detection."""
|
||||||
with patch.dict(
|
with patch.dict(
|
||||||
|
|||||||
Reference in New Issue
Block a user