420c8cd2d39025e4c214fe668d6cdb81e552a66a
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Chris CoutinhoandClaude Opus 4.7
|
1fa4c82fd2 |
chore: address review-round-4 nits — stale delenv, upgrade hint, in-sync notes
Four small follow-ups from the reviewer's latest pass:
- tests/unit/test_stdio.py:18: the single_user_env fixture used
monkeypatch.delenv("ENABLE_MULTI_USER_BASIC_AUTH", ...). That env var
is no longer read after the ADR-022 follow-up; switched to delenv of
MCP_DEPLOYMENT_MODE which is the canonical mode-selection input today.
Comment updated to match.
- config_validators.py: when detect_auth_mode rejects an invalid
MCP_DEPLOYMENT_MODE, surface a one-line ADR-022 migration hint if the
rejected value is exactly "oauth_single_audience" (the most common
upgrade pain — users carrying that value over from ADR-021 .env files).
Other invalid values get the regular "Valid values: …" message
unchanged.
- config.py + config_validators.py: added cross-reference comments on
both mode-resolution sites (Settings.__post_init__ and
detect_auth_mode) noting that they each compute the canonical mode
independently and must be kept in sync when a new mode is added.
Surfaces the parallel-duplication intentionally so the next maintainer
doesn't have to discover it.
- docs/ADR-021-configuration-consolidation.md:92: appended a trailing
comment to the historical "valid values" example, marking
oauth_single_audience and oauth_token_exchange as removed in ADR-022.
ADR-021 stays as the historical record; the trailer points future
readers at the current state.
No functional changes; 1009 unit tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Chris CoutinhoandClaude Opus 4.7
|
6e7c821761 |
fix(config): derive mode flags in Settings.__post_init__; address review round 2
The integration jobs for `mcp-multi-user-basic` and `mcp-login-flow`
were failing with HTTP 500s. Root cause: `get_settings()` builds a
fresh Settings on every call (not cached). Commits 3 and 4 set the
derived `enable_login_flow` / `enable_multi_user_basic_auth` flags as
a side effect of `detect_auth_mode`. detect_auth_mode runs once at
startup, against the Settings instance owned by `validate_configuration`.
Every per-request call site that does `settings = get_settings()` got
a fresh Settings with both flags at their default `False` (since the
env-var aliases were dropped), causing the multi-user dispatcher in
`context.py` to take the wrong branch and crash.
Fix: move the derivation into `Settings.__post_init__`. Every Settings
instance now carries correct flags from the moment it's constructed —
no caching needed, no mutation-after-construction race. detect_auth_mode
becomes a pure reader of the already-derived state.
The legacy env-var deprecation check moves with it. It also picks up
the reviewer's truthy-string fix: previously `os.getenv(legacy)` fired
for the literal string "false" (a non-empty Python string is truthy),
which would have errored on any user with a leftover
`ENABLE_LOGIN_FLOW=false` in their `.env`. The check now only fires
when the value lowercases to one of {"1", "true", "yes", "on"}.
- nextcloud_mcp_server/config.py: extend Settings.__post_init__ with
the legacy-deprecation block and the derived-flag derivation
(resolve mode from deployment_mode + username/password, set flags).
- nextcloud_mcp_server/config_validators.py: drop the
`_sync_derived_flags` helper (superseded by __post_init__). Drop the
legacy-env-var deprecation block (moved). `detect_auth_mode` is now
pure — no mutation. Drop the now-unused `import os`.
- tests/unit/test_config_validators.py: legacy-env-var tests now
expect `ValueError` at `Settings(...)` construction (via `get_settings()`),
not at `detect_auth_mode` call. Added two new tests:
* `test_legacy_env_var_check_ignores_falsy_strings` — pins the
truthy-string fix (reviewer round 2 finding).
* `test_derived_flags_stable_across_get_settings_calls` — regression
test pinning the integration-test fix (two consecutive
`get_settings()` calls return Settings instances with the same
derived flags).
Also reworked `test_login_flow_mode_auto_derives_enable_login_flow_flag`
to assert at-construction derivation (not the old mutation pattern).
- docs/configuration-migration-v2.md: dropped the duplicate
`MCP_DEPLOYMENT_MODE=multi_user_basic` line (review round 2 nit — a
sed artifact from commit 4).
- docs/ADR-021-configuration-consolidation.md: sed-replaced the in-body
`MCP_DEPLOYMENT_MODE=oauth_single_audience` examples with `login_flow`
(review round 2 nit — only the status header was updated in commit 4).
- tests/conftest.py: docstring comment for the multi-user-basic fixture
switched from `ENABLE_MULTI_USER_BASIC_AUTH=true` to
`MCP_DEPLOYMENT_MODE=multi_user_basic` (review round 2 nit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Chris CoutinhoandClaude Opus 4.7
|
c74ef014ee |
docs(adr-022): mark Accepted, update env/compose/migration docs for LOGIN_FLOW rename
Follow-up to the LOGIN_FLOW rename. The user-facing surface area — env.sample, docker-compose.yml mcp-login-flow profile, migration guide, ADR statuses, and the running.md boot-log examples — all need to refer to `login_flow` rather than the deprecated `oauth_single_audience` string. - docker-compose.yml: add explicit MCP_DEPLOYMENT_MODE=login_flow to the mcp-login-flow profile (no longer relying on auto-detection). - env.sample: update the deployment-mode list and example, dropping the removed `oauth_token_exchange` and pointing at ADR-022 for the rename rationale. - docs/ADR-022: flip Status to Accepted with a note that this PR implements step 1 (rename + validation gate). - docs/ADR-021: note that it has been partly superseded by ADR-022 (the oauth_single_audience naming is no longer accurate); cross-link. - docs/ADR-025: drop oauth_single_audience/keycloak from the dynaconf validator example and the [oauth_single_audience] TOML section. - docs/configuration-migration-v2.md: bulk-replace oauth_single_audience → login_flow throughout (sed -i). - docs/running.md: re-collapse the per-mode boot-log subsections (added during the closed PR #786 workaround) back into a uniform "<mode>"-substitution block — now correct after this PR's logging cleanup at app.py:1172. No code changes in this commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Chris CoutinhoandClaude Opus 4.6
|
7956c3c061 |
refactor: remove Smithery deployment mode
Smithery is no longer a supported deployment mode. Remove all Smithery-specific code paths, middleware, configuration, and tests. This simplifies the codebase by eliminating DeploymentMode enum, SmitheryConfigMiddleware, session config context variables, and the smithery_main entrypoint. Files deleted: Dockerfile.smithery, smithery.yaml, smithery_main.py ADR-016 retained with deprecated status for historical reference. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Chris CoutinhoandClaude Sonnet 4.5
|
1a5bb10cd0 |
feat(config): consolidate configuration with smart dependency resolution (ADR-021)
Simplifies configuration by consolidating overlapping settings and adding automatic dependency resolution. This makes semantic search configuration significantly easier for users while maintaining 100% backward compatibility. ## Key Changes ### Variable Renaming (Backward Compatible) - `VECTOR_SYNC_ENABLED` → `ENABLE_SEMANTIC_SEARCH` (old name still works) - `ENABLE_OFFLINE_ACCESS` → `ENABLE_BACKGROUND_OPERATIONS` (old name still works) - Deprecation warnings logged when old names used - Old names will be removed in v1.0.0 ### Smart Dependency Resolution - `ENABLE_SEMANTIC_SEARCH` automatically enables background operations in multi-user modes - No need to set both `ENABLE_OFFLINE_ACCESS` and `VECTOR_SYNC_ENABLED` anymore - Single-user mode doesn't auto-enable background ops (not needed) ### Explicit Mode Selection (Optional) - New `MCP_DEPLOYMENT_MODE` environment variable - Valid values: single_user_basic, multi_user_basic, oauth_single_audience, oauth_token_exchange, smithery - Removes ambiguity about which deployment mode is active - Falls back to auto-detection if not set (existing behavior) ### Configuration Templates - Reorganized `env.sample` by deployment mode with clear sections - Added mode-specific quick-start templates: - `env.sample.single-user` - Simplest configuration - `env.sample.oauth-multi-user` - Recommended multi-user - `env.sample.oauth-advanced` - Token exchange mode ## Implementation Details ### Files Modified - `nextcloud_mcp_server/config.py` - Smart dependency resolution helpers - `nextcloud_mcp_server/config_validators.py` - Simplified validation, explicit mode - `tests/unit/test_config_validators.py` - 19 new tests (60 total, all passing) - `env.sample` - Reorganized by deployment mode - `docs/configuration.md` - Complete rewrite with consolidated approach - `docs/troubleshooting.md` - New consolidation troubleshooting section - `README.md` - Updated variable references ### New Files - `docs/ADR-021-configuration-consolidation.md` - Architecture decision record - `docs/configuration-migration-v2.md` - Comprehensive migration guide - `env.sample.single-user` - Single-user quick-start template - `env.sample.oauth-multi-user` - OAuth multi-user quick-start template - `env.sample.oauth-advanced` - Token exchange quick-start template ## User Impact ### Before (Confusing) ```bash ENABLE_OFFLINE_ACCESS=true # Why both? VECTOR_SYNC_ENABLED=true # What's the relationship? ``` ### After (Simplified) ```bash MCP_DEPLOYMENT_MODE=oauth_single_audience # Explicit (optional) ENABLE_SEMANTIC_SEARCH=true # Auto-enables background ops! ``` ### Benefits - 📉 2 fewer variables to understand for semantic search - 📋 Clear intent ("I want semantic search") - 🎯 Explicit mode declaration available - 🔄 100% backward compatible - ✅ All 265 unit tests passing ## Testing - All 60 config validation tests passing - 10 new tests for configuration consolidation - 9 new tests for explicit mode selection - Full unit test suite: 265 tests passing - Backward compatibility verified ## Migration Users can migrate at their own pace. Old variable names continue working with deprecation warnings. See docs/configuration-migration-v2.md for detailed migration instructions. Related: ADR-021 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> |