From 0fb21b5c6dfc49468a6d45e830644b8a23a09e49 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 12 May 2026 22:57:35 +0200 Subject: [PATCH] =?UTF-8?q?chore:=20address=20review-round-5=20=E2=80=94?= =?UTF-8?q?=20stale=20=5Fsync=5Fderived=5Fflags=20reference=20+=20missing?= =?UTF-8?q?=20migration-hint=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- nextcloud_mcp_server/config_validators.py | 4 ++-- tests/unit/test_config_validators.py | 28 +++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/nextcloud_mcp_server/config_validators.py b/nextcloud_mcp_server/config_validators.py index 0b190659..7434afa9 100644 --- a/nextcloud_mcp_server/config_validators.py +++ b/nextcloud_mcp_server/config_validators.py @@ -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 # old OAUTH_SINGLE_AUDIENCE without ENABLE_LOGIN_FLOW) needed unmerged # Nextcloud user_oidc patches and is no longer supported. The - # `enable_login_flow` flag is now derived from the resolved mode by - # `_sync_derived_flags`, so users only configure the mode — no + # `enable_login_flow` flag is now derived from the resolved mode in + # `Settings.__post_init__`, so users only configure the mode — no # separate ENABLE_LOGIN_FLOW env var is needed. # If OAuth credentials not provided, DCR must be available diff --git a/tests/unit/test_config_validators.py b/tests/unit/test_config_validators.py index a814c66f..f7e97dec 100644 --- a/tests/unit/test_config_validators.py +++ b/tests/unit/test_config_validators.py @@ -788,6 +788,34 @@ class TestExplicitModeSelection: assert "invalid_mode" 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): """Test explicit mode takes precedence over auto-detection.""" with patch.dict(