Comment-only follow-up to surface non-obvious behavior at the call
sites flagged in review:
- server/talk.py: note the `uuid.uuid4().hex` 32-char no-dashes format
(spreed accepts either form).
- models/talk.py: warn that spreed returns `lastReadMessage: 0` rather
than `null` for unread rooms, so consumers should compare to ``None``
rather than rely on truthiness.
- 10-install-spreed-app.sh: document that the `app:install || app:enable`
fallback also masks unrelated install failures, and limit its use to
dev fixtures.
No runtime behavior changes; tests unchanged (still 13 unit + 7 integ).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four targeted fixes from the AI code review:
1. TalkConversation.description: drop the misleading `str | None`
union (spreed always sends `""`, never null) — type is now `str`
with default `""`.
2. get_messages: guard the X-Chat-Last-Given int parse with
try/except so a misbehaving proxy can't crash the read flow;
logs a warning and falls back to None.
3. get_messages: clamp `limit` to [1, 200] in the client (spreed
caps server-side at 200 and silently truncates) so the returned
`count` always matches what was actually requested. Both client
and server-tool docstrings updated to state the valid range.
4. Add an integration test covering the 32000-char message ceiling
in talk_send_message — the empty-message case was already tested,
the over-length case was not.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds 6 MCP tools so an LLM can read a user's Talk conversations and
post messages on their behalf, addressing the "read my chats and reply"
use case from issue #720:
- talk_list_conversations
- talk_get_conversation
- talk_get_messages
- talk_list_participants
- talk_send_message (auto-attaches a referenceId for retry dedup)
- talk_mark_as_read
Edit/delete messages, reactions, threads, and call/session ops are
intentionally out of scope for this first PR.
The TalkClient also exposes create_conversation/delete_conversation
for the integration test fixture; these are not registered as MCP
tools. A post-installation hook enables spreed in the docker dev env
so the integration suite has a real Talk backend to talk to.
Closes#720
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bumps the claude-code-action pin to v1.0.97 and turns on
`track_progress` + `use_sticky_comment` so reviews update a single
tracking comment per PR instead of appending a fresh comment on every
push. Mirrors the pattern in astrolabe-cloud-website.
The prompt now directs Claude to deliver the review by editing the
tracking comment via `mcp__github_comment__update_claude_comment`, and
`Bash(gh pr comment:*)` is dropped from the allowed-tools list since
that path is no longer used. Permissions widen from read to write on
pull-requests + issues so the action can edit its own comment.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Wrap raw DeckComment returns in CardCommentResponse(BaseResponse) for
create/update so the success/timestamp envelope matches other deck tools
(#737 review issue 2).
- Rename ListCardCommentsResponse.total → count and clarify in the
description that it's the page size, not a server-side total — the Deck
list endpoint does not expose one (#737 review issue 3).
- Validate the documented 1000-character limit on create/update with an
inline length check + ValueError, matching the pattern in
api/management.py (#737 review issue 4).
- Use modern int | None union syntax for the new parent_id parameter
(#737 review issue 1); rest of the file is left in the existing
Optional[...] style.
Also add an MCP-level test that the >1000 char message is rejected, and
update the existing comment tests to unwrap the new comment field.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cover full CRUD lifecycle (create → list → update → delete → verify gone)
and the reply path where parent_id populates replyTo on the new comment.
Tests run against the live mcp container via the existing nc_mcp_client
fixture and reuse the temporary_board_with_card fixture for setup/cleanup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expose four new MCP tools backed by existing DeckClient comment methods:
- deck_get_card_comments — list with limit/offset pagination
- deck_create_card_comment — top-level or threaded (via parent_id)
- deck_update_card_comment — author-only on the server
- deck_delete_card_comment — author-only, destructive, idempotent
Adds ListCardCommentsResponse and CardCommentOperationResponse models, and
extends the client unit tests to cover replies, deletion, pagination, and
the request shape for updates.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Notes app v5.0.0 has scenarios where the API returns a JSON list where the
MCP server expects a single note object — notably the notes_api#fail
catch-all returning [] for unmatched routes. Without a guard, callers hit
a cryptic Pydantic "argument after ** must be a mapping, not list" from
Note(**payload).
Add a small _expect_note_object helper at the client layer:
- dict → pass through (the healthy case)
- single-element list → unwrap and warn (Notes v5.0.0 quirk)
- empty list, multi-element list, non-dict → raise a diagnostic ValueError
that names the operation and points at the likely root cause (URL prefix,
unmatched route, wrong API version)
Wire it into get_note / create_note / update so any list-shaped response
fails clearly instead of cryptically.
Six unit tests pin every branch of the helper.
Note: The 405s the issue reports for update_note / append_content match
Notes v5.0.0's documented routes (PUT /api/v1/notes/{id}) per upstream
appinfo/routes.php. They are most likely a downstream effect of #732
(missing /index.php URL prefix on installs without Pretty URLs) — the fix
in PR #733 should resolve those once it lands.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two upstream Pydantic ValidationErrors that took down whole list responses.
#704: Contact.birthday is declared str, but vobject parses BDAY as a
datetime.date — any contact with a populated BDAY broke nc_contacts_list_contacts
entirely. Add a field_validator(mode="before") that coerces date / datetime
to ISO strings. Strings and None pass through unchanged. Defense in depth:
existing call sites already coerce, but the model is now correct on its own
so any future code path that constructs Contact from raw vobject output
stays safe.
#728: Tables app v2.0.1 stopped emitting owner_display_name on the top-level
table payload (still present inside views via get_schema), so list_tables
failed for every user with a Pydantic ValidationError. Make the field
Optional[str] = None — captures the value when present, won't blow up when
missing.
Six new direct-construction unit tests in tests/unit/test_response_models.py
pin both fixes (date / datetime / str / None for birthday; with / without
owner_display_name for Table) so the regressions can't recur silently.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bare /apps/<app>/... URLs return 404 on Nextcloud installs without Pretty
URLs (URL rewriting), which is opt-in and not the default — see #732. The
/index.php/apps/... form is the universal entry point and works regardless
of web-server config, matching how /remote.php/dav and /ocs/v2.php already
have dedicated entry points.
Add a small _resolve_url helper on BaseNextcloudClient that rewrites
/apps/... → /index.php/apps/... at the top of _make_request, so every
current call site (notes, deck, cookbook, news) and any future ones are
covered transparently with no per-client churn.
Other path prefixes (/remote.php, /ocs, absolute URLs, already-prefixed
/index.php/apps) pass through unchanged. New unit tests in
tests/unit/client/test_base.py pin all six cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds "Astrolabe Cloud" as the named maintainer in the CLA Background
so the contracting party is identifiable, addressing reviewer feedback
on PR #723.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a CLA so future contributions can be relicensed if the project
later offers commercial terms alongside AGPL-3.0. Adapted from the
Apache 2.0 ICLA with Dutch-law modifications: moral rights waiver
under Auteurswet art. 25, GDPR data-processing notice referencing
cla-assistant.io, and Amsterdam jurisdiction.
Signing is administered via the hosted cla-assistant.io service
(configured outside this repo); the Gist referenced there is kept in
sync with CLA.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the per-user delay pattern used in tests/conftest.py:all_oauth_tokens
(commit 963a504). Without it, all four Playwright browser contexts hit
Nextcloud's OIDC authorize endpoint simultaneously and the last users in
iteration order (charlie/diana) frequently time out on the consent screen
in CI, producing `TimeoutError: Timeout waiting for OAuth callback`.
Uses a 0.5s stagger locally and 10s in GITHUB_ACTIONS, matching the
existing fixture so behaviour stays consistent across the two parallel
fixtures.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous run on nc32 failed at the search-result assertion because
`wait_for_vector_sync` returned on the first indexed-count bump (deck
seed cards) before this specific note hit Qdrant. Replace the single
search call with a poll that retries every 2s until the unique term
returns our note, or times out after 60s with a loud diagnostic. The
previously-observed flake would now wait past the deck-card indexing
window rather than racing it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per review:
- Hoist `import httpx` out of the two test function bodies and into
the module imports at the top of
test_astrolabe_chunk_context.py.
- Simplify the regression guard in
test_management_chunk_context_endpoint.py to use
`mock.assert_awaited_once_with(...)` instead of manually unpacking
call_args. This is stricter — it fails loudly on signature change —
and matches the canonical pattern for asserting mock calls.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Rename test_chunk_context_endpoint_handles_missing_app_password to
test_chunk_context_endpoint_rejects_invalid_bearer so it reflects
what is actually exercised: an invalid bearer is rejected upfront at
validate_token_and_get_user, not at the NotProvisionedError branch.
The NotProvisionedError path is covered by the corresponding unit
test in test_management_chunk_context_endpoint.py.
- Hoist `import base64` to module level per PEP 8.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Astrolabe's ApiController endpoints (search, chunk-context) require a
CSRF `requesttoken` header — axios picks it up from OC.requestToken
automatically in the SPA, but page.request.get() does not.
The first CI run failed on the search step with 412 CSRF check failed
before reaching the chunk-context assertion that was supposed to
surface the handler bug. Load the Astrolabe page, read OC.requestToken,
and pass it on both calls.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The /api/v1/chunk-context and /api/v1/pdf-preview handlers in
api/visualization.py forwarded the incoming OAuth bearer directly to
Nextcloud via NextcloudClient.from_token. In multi-user BasicAuth mode
Nextcloud has no validator for those bearers on Notes/WebDAV, so it
treats the request as anonymous and returns 401 — surfaced to the user
as a 500 from /apps/astrolabe/api/chunk-context. Search worked because
it only hits Qdrant.
Architecturally, OAuth is only for Astrolabe→MCP server; MCP server→
Nextcloud always uses the per-user app password stored during provision
(background sync already does this via vector.oauth_sync).
- Resolve the Nextcloud client through get_user_client_basic_auth in
both get_chunk_context and get_pdf_preview, surfacing
NotProvisionedError as a clean 401 instead of opaque 500.
- Apply the same fix to the session-cookie variant in
auth/viz_routes.chunk_context_endpoint for the internal viz UI.
Tests:
- New unit file test_management_chunk_context_endpoint.py, including a
regression guard that asserts get_user_client_basic_auth is awaited
(so reverting to from_token fails without needing a live Nextcloud).
- Updated test_management_pdf_preview_endpoint.py to mock the new auth
path (drops extract_bearer_token / NextcloudClient.from_token patches).
- New integration test test_astrolabe_chunk_context.py drives the full
chain (browser → Astrolabe → MCP → Nextcloud) in multi-user BasicAuth
mode, plus bare-bones 401 checks on the MCP endpoint.
Full unit suite: 546 passed.
Companion PR on astrolabe (cbcoutinho/astrolabe#66) sends the Nextcloud
UID as loginName in the app-password POST body so the stored record is
complete. Submodule bump to that branch will follow once CI reproduces
the failure on the old submodule.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Picks up astrolabe/astrolabe#61 which fixes app password provisioning
failure caused by loginName mismatch in ITokenProvider::generateToken().
This was the root cause of vector sync never indexing in multi-user
BasicAuth mode, which caused the plotly visualization test to fail.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The multi-user-basic integration job was consistently failing with
`CancelledError: Cancelled via cancel scope ... by <async_generator_athrow>`
followed by a cascade of `anyio.ClosedResourceError` in every subsequent
test. Root cause: `create_mcp_client_session` was declared as an async
generator driven by `async for session in ...:`, so Python's generator
finalizer (`aclose`) ran under pytest-asyncio's cleanup task instead of
the task that owned the nested `streamablehttp_client` cancel scope.
anyio then raised when the inner task group saw its scope being exited
from a foreign task, leaving the memory object streams half-closed and
poisoning the rest of the session.
Switching to `@asynccontextmanager` + `async with ... as session:` makes
`__aenter__`/`__aexit__` run in the frame that owns the context manager,
satisfying anyio's structured concurrency requirements.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- _resolve_settings_files() now raises FileNotFoundError when
NEXTCLOUD_MCP_SETTINGS_FILE points to a missing file, instead of
silently falling back to defaults (footgun on typos).
- .secrets.toml is now looked for alongside the explicit settings file
when NEXTCLOUD_MCP_SETTINGS_FILE is set, matching user expectation for
/etc-style deployments. Unset behaviour (cwd lookup) is unchanged.
- get_token_db_path() drops the redundant os.environ.get() short-circuit;
TOKEN_STORAGE_DB is already bound through dynaconf because the key is
declared in _DEFAULTS.
- is_ephemeral_token_db() docstring documents the "must call
get_token_db_path() first" precondition.
- alembic.ini comment clarifies the ./tokens.db placeholder is cwd-relative
by design and points readers at the -x database_url escape hatch.
- New tests/unit/test_config_paths.py (12 tests) covering the ephemeral
tempfile lifecycle, the TOKEN_STORAGE_DB override path, and all six
_resolve_settings_files() cases including the two new behaviours.
Full unit suite now at 476 passed (464 + 12 new). Ruff + ty clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two bugs made `uvx --from . nextcloud-mcp-server run` (and any pip install)
unusable outside Docker:
1. Dynaconf was configured with ignore_unknown_envvars=True and relied on
settings.toml to declare the key schema. With no settings.toml in a wheel
install, every env var (NEXTCLOUD_HOST, MCP_DEPLOYMENT_MODE, ...) was
silently dropped. Moved the schema into a Python _DEFAULTS dict passed
directly to Dynaconf, kept settings.toml as an optional external override
(renamed to settings.toml.example, gitignored), and pointed docker-compose
at the example file.
2. Token SQLite DB defaulted to /app/data/tokens.db in multiple places
(auth/storage.py, migrations.py, alembic/env.py, cli.py db subcommands),
which blew up at uvicorn startup with FileNotFoundError on non-Docker
hosts. Replaced with a new config.get_token_db_path() helper that
resolves TOKEN_STORAGE_DB if explicitly set, otherwise allocates a
per-process tempfile cleaned up at interpreter exit via atexit — mirroring
the "ephemeral by default" pattern used for QDRANT_LOCATION=:memory:.
Containers are unaffected: docker-compose services now explicitly set
TOKEN_STORAGE_DB=/app/data/tokens.db (the fourth service that was missing
this pin has been brought in line with the other three).
Verified end-to-end in an isolated /tmp venv: env-var-only startup, Alembic
migrations run against the tempfile, Application startup complete, /health/live
returns 200, tempfile deleted on SIGTERM. Unit tests (464) + ruff + ty pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
External IdPs like AWS Cognito return scopes prefixed with the resource
server identifier (e.g. https://mcp.example.com/notes.read). MCP tools
use bare scope names (notes.read) in @require_scopes decorators. Without
stripping the prefix, scope matching fails and only identity-only tools
(openid/profile/email) are visible — resulting in 4/125 tools shown.
Strip the OIDC_RESOURCE_SERVER_ID prefix in both get_access_token_scopes()
(used by list_tools filtering) and the require_scopes decorator (used at
tool execution time).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add BasicAuthLifespanContext Protocol to make the contract between
StdioContext and get_client() explicit and type-safe. Document why
mcp.get_context() is required for non-template resources. Add News
and Collectives to README Supported Apps table, fix transport default.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AWS Cognito provides refresh tokens automatically with the authorization
code flow but does not list offline_access as a supported scope. Check
the IdP's scopes_supported discovery field before including it in
requests, and always accept refresh tokens from responses regardless.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove dead monkeypatch in test_stdio_calls_get_stdio_mcp
- Add _reload_config() teardown to single_user_env fixture
- Tighten AVAILABLE_APPS type to Callable[[FastMCP], None]
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Revert default transport to streamable-http (not a breaking change)
- Extract AVAILABLE_APPS constant to server/__init__.py (DRY)
- Wrap get_stdio_mcp ValueError in click.ClickException for clean errors
- Fix test_stdio.py: call _reload_config() so dynaconf sees env changes
- Use lazy %-style logging in stdio.py
- Add private API comments in test assertions
- Derive --enable-app CLI choices from AVAILABLE_APPS
- README: show explicit --transport stdio in uvx examples
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a lightweight stdio transport path so users can run the server
locally with MCP clients like Claude Code using `uvx nextcloud-mcp-server run`.
- New `nextcloud_mcp_server/stdio.py` with minimal FastMCP setup for
single-user BasicAuth (no OAuth, semantic search, or background sync)
- Default transport changed from streamable-http to stdio
- Dockerfile updated to explicitly use streamable-http for containers
- CLI `--enable-app` now includes news, collectives, and sharing
- README Quick Start section with uvx and MCP client config examples
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tags pushed with GITHUB_TOKEN don't trigger other workflows (GitHub's
anti-recursion protection), which is why a PAT was needed. Instead,
chain release and docker workflows directly via workflow_call from
bump-version, eliminating the need for a personal access token.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Flow 2 hardcoded offline_access in the scope string, but providers
like AWS Cognito don't support this scope (they handle refresh tokens
via client config). This caused invalid_scope errors on the Astrolabe
semantic search enablement flow.
Only include offline_access when enable_offline_access is explicitly
set, matching the behavior of DCR scope registration.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AWS Cognito access tokens do not include an `aud` claim per RFC 7519 —
they use `client_id` instead. This causes `_has_mcp_audience` to reject
all Cognito-issued tokens with "Missing MCP audience. Got []".
When `aud` is empty, fall back to the `client_id` JWT claim for audience
validation. The MCP server's own client_id will be present there since
the AS proxy exchanges the authorization code using its credentials.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use dynaconf (get_settings()) instead of os.getenv for OIDC_RESOURCE_SERVER_ID
- Re-add Settings field, _field_map entry, and settings.toml default
- Add trailing-slash guard (.rstrip("/")) to prevent double-slash in scopes
- Add double-prefixing guard: skip scopes already carrying the prefix
- Add @pytest.mark.unit to test module
- Add test for already-prefixed scopes
- Document OIDC_RESOURCE_SERVER_ID in docs/configuration.md
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add offline_access to OIDC standard scopes exclusion list to prevent it
from being incorrectly prefixed, which would break Cognito refresh token
flows. Extract scope transformation into testable _transform_scopes_for_idp()
helper, add debug logging for prefixed scopes, remove unused Settings field
(oauth_routes.py consistently uses os.getenv), and add unit tests.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When OIDC_RESOURCE_SERVER_ID is set, prefix resource scopes with the
identifier when forwarding to the IdP (e.g., calendar.read becomes
https://example.com/calendar.read). Required for IdPs like AWS Cognito
that mandate {resource_server_id}/{scope} format for custom scopes.
OIDC standard scopes (openid, profile, email) are forwarded as-is.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The helm chart has been migrated to a dedicated repository at
https://github.com/cbcoutinho/helm-charts. This removes the chart
source, release workflow, bump script, and updates all documentation
to point to the new repository.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Incorporate reviewer feedback across three review rounds:
- Remove post_hooks from Phase 1 constructor; defer to Phase 4
- Fix Validator syntax: use condition=lambda instead of ne= kwarg
- Add MCP_DEPLOYMENT_MODE validator to catch typos at startup
- Add CRITICAL to LOG_LEVEL validator enum
- Make OTEL_TRACES_SAMPLER_ARG validation conditional on ratio samplers
- Add all missing provider env vars to settings.toml (Bedrock, Anthropic, Ollama, Simple)
- Add provider secrets to .secrets.toml.example
- Fix DynaconfDict import to stable public API path
- Strengthen ignore_unknown_envvars risk: CI lint check mandatory before Phase 2
- Document ValidationError vs ValueError breaking change in Phase 3
- Acknowledge environments=True legacy risk with mitigation
- Address root_path pip-install concern (intentional: pip uses env vars)
- Add enable_token_exchange to adapter example; note exhaustive field mapping
- Clarify Provider Registry is Phase 6 with explanation of os.getenv coexistence
- Improve test isolation fixture with teardown reload + _dynaconf visibility note
- Add Docker Compose volume mount host-file existence note
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The root_path for dynaconf resolved to site-packages instead of the
application root when installed non-editable (Docker). This caused all
settings without env var overrides to be None, crashing on startup with
a TypeError in chunk size validation.
Fix root_path to fall back to CWD when settings.toml isn't at the
source-tree path, and refactor get_settings() to only pass values
dynaconf actually has — letting Settings dataclass defaults apply for
unconfigured keys. Mount settings.toml into all docker-compose MCP
services as a read-only volume.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Many identity providers (AWS Cognito, Okta, Azure AD) reject or mishandle
colons in OAuth scope names. This migrates all custom scopes from
`resource:action` to `resource.action` format (e.g., `notes:read` →
`notes.read`), which is universally accepted and aligns with industry
conventions (Microsoft, Google).
Includes Alembic migration 004 for stored scope strings and ADR-024
documenting the rationale and RFC references.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Document wildcard scope policy in ClientRegistry class docstring
- Add hostname None guard and IPv6 loopback (::1) to redirect URI validation
- Simplify redirect URI scheme validation into single guard clause
- Add try/finally cleanup to DCR client deletion test
- Validate 302 Location header in unknown client rejection test
- Add unit tests for IPv6 loopback, malformed URIs, and DCR proxy paths
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address all 9 review points from PR #680:
- Fix post_hooks code examples to use correct return-dict signature
- Expand test isolation section with fixture factory, DynaconfDict, and
reload patterns
- Document ignore_unknown_envvars silent failure mode in Negative
Consequences and add env var audit to Phase 1 checklist
- Fix NEXTCLOUD_HOST validator to be unconditional (required in all modes)
- Document environments=True edge cases (unset mode, ENV_FOR_DYNACONF
shadowing)
- Add upper bound to dynaconf version pin (>=3.2.13,<4.0)
- Tighten Pydantic Settings comparison to acknowledge 2.x TOML support
- Make .gitignore additions explicit in Phase 1 checklist
- Clarify that shell-level .env loading still works with load_dotenv=False
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the unused ALLOWED_MCP_CLOUD_CLIENTS env var — all clients are
defined via ALLOWED_MCP_CLIENTS or the static well-known defaults.
Add keycloak as an integration test profile in CI now that login-flow
replaces the old bearer token approach for external IdPs.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix incorrect hook syntax (@hookable.post → Dynaconf(post_hooks=[...])),
broken Qdrant mutual exclusivity validator, missing root_path for settings
file resolution, and empty string defaults that bypass validators. Add test
isolation section, mark Phase 4 as optional/future with risk note, and
correct Pydantic comparison (already a project dependency).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merge ALLOWED_MCP_CLOUD_CLIENTS into a single ALLOWED_MCP_CLIENTS env var
that supports both simple client IDs and pipe-separated client_id|redirect_uri
entries. Enforce HTTPS for non-localhost redirect URIs, warn on malformed
entries, and use wildcard scopes for all static clients (upstream IdP enforces
actual scopes). Add deprecation warning for the old env var.
Also fixes DCR proxy error messages to reference only ALLOWED_MCP_CLIENTS and
use "Upstream" instead of "Nextcloud" for IdP-agnostic language. Enables
Login Flow v2 + DCR on the mcp-keycloak docker-compose service.
Adds 17 unit tests for ClientRegistry parsing/validation and 7 keycloak
integration tests for DCR lifecycle, AS metadata, and client authorization.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Propose migrating from manual os.getenv() calls to dynaconf for
file-based configuration. Key decisions: envvar_prefix=False for
backward compatibility, MCP_DEPLOYMENT_MODE as environment switcher,
TOML settings files with secret separation, and incremental migration
via adapter pattern.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude AI (web) sends a Cognito-issued client_id with an HTTPS redirect
URI, but the client registry only supported localhost redirect URIs via
ALLOWED_MCP_CLIENTS. Add ALLOWED_MCP_CLOUD_CLIENTS env var for web-based
clients with format "client_id|redirect_uri".
Also fix the DCR proxy to return a clear error when the upstream IdP
(e.g. Cognito) doesn't support dynamic client registration, instead of
silently falling back to a Nextcloud-specific endpoint that fails.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove all RFC 8693 token exchange tests (integration, manual, keycloak)
since Nextcloud doesn't support bearer tokens without upstream patches
- Remove manual impersonation/ADR-004 scripts and their docs
- Clean up token_exchange singleton from integration conftest
- Improve logging in _complete_login_flow_v2_as_user with step-by-step
[username] prefixed messages matching _complete_login_flow_v2 style
- Remove unnecessary time staggering from all_login_flow_user_tokens;
concurrent token acquisition works without artificial delays
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The OAuth profile removal dropped cross-user permission tests (deck, files,
notes) that validated Nextcloud sharing/ACL enforcement through MCP tools.
These tested general functionality, not OAuth-specific behavior.
Restores coverage with login-flow fixtures and 9 tests covering file share
read/write enforcement, folder sharing, Deck board ACL view/edit, and
per-user resource isolation for files, boards, and notes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace 4 stale mcp-oauth references in CLAUDE.md with mcp-login-flow
- Import and catch playwright.async_api.TimeoutError in consent retry loop
(Playwright's TimeoutError doesn't inherit from Python's built-in)
- Replace unreachable `return True` with explicit RuntimeError raise
- Add clarifying comment for hardcoded login-flow port default
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The login-flow MCP server exposes 3 additional auth tools
(nc_auth_provision_access, nc_auth_check_status, nc_auth_update_scopes)
from ADR-022 that require only 'openid' scope. Update the
no-custom-scopes test to expect 7 auth tools instead of 4.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Discourage raw docker exec for database queries; use scripts/dbquery.py
and scripts/sqlitequery.py exclusively
- Ensure all python commands use uv run prefix (mcp run, pytest, etc.)
- Replace Progressive Consent section with concise Deployment Modes
overview (single-user, multi-user BasicAuth, Login Flow v2)
- Normalize docker-compose to docker compose throughout
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pythonvCard4 parses vCard BDAY fields into datetime.date objects, but
the Contact model expects Optional[str]. This caused a validation error
that crashed the entire contact list. Convert at the client layer
(consistent with the calendar client pattern) with a defensive check
at the server mapping layer.
Closes#672
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both /app/provision and /app/provision/status now require a valid
Nextcloud OIDC bearer token via the Authorization header, reusing the
existing validate_token_and_get_user pattern from the management API.
This eliminates the open redirect vulnerability (only authenticated
Astrolabe users can trigger the flow) and prevents unauthenticated
resource exhaustion via Login Flow v2 session creation.
The authenticated user_id from the token replaces the untrusted
user_id query parameter.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Log warning if /app mount not found when sharing poll task group
- Add docstring explaining unconditional task group creation
- Check session expires_at in provision_status to catch stale sessions
- Guard _poll_and_store status writes against cleanup-while-polling race
- Use "error" status (not "expired") when app_password is missing
- Remove hardcoded "Astrolabe Background Sync" user_agent string
- Fix async mock pattern (new_callable=AsyncMock) in test
- Add autouse fixture to clear _provision_sessions between tests
- Add _poll_and_store unit tests: completed, expired, error, cleanup
- Document all status values in provision_status docstring
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Escape HTML in _render_error to prevent XSS from exception messages
- Replace asyncio.create_task/sleep with anyio task group and sleep,
tying poll task lifetime to the app lifespan for proper cleanup
- Extract rewrite_url_origin() utility to fix duplicated URL rewriting
logic and replace urlparse._replace with stable urlunparse API
- Add warning log for insecure HTTP redirect URIs
- Add unit tests for validation, XSS escaping, route handlers, and
URL rewriting (16 new tests in test_provision_routes.py)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Login Flow v2 is a deployment-wide mode where all users authenticate
with app passwords (not OAuth refresh tokens). Set use_basic_auth=True
when enable_login_flow is true so the background sync user manager
queries the app_passwords table and scanners use app password
authentication for Nextcloud API calls.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When enable_login_flow is true, also check the app_passwords table
for provisioned users. Previously, OAuth mode only queried the
refresh_tokens table, missing users who were provisioned via
Login Flow v2 (which stores app passwords, not refresh tokens).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Nextcloud returns poll/login URLs using its internal hostname (e.g.
http://localhost/login/v2/poll) which is unreachable from the MCP
server container in Docker networks. Rewrite the poll endpoint's
origin to use the configured NEXTCLOUD_HOST so server-side polling
works correctly.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Nextcloud may omit default ports in the login_url (e.g. http://app
instead of http://app:80). Extract just scheme+hostname from
NEXTCLOUD_HOST for the URL replacement check.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the popup-based approach with a direct redirect to Nextcloud's
login page. This is more compatible with Playwright E2E tests and
simpler for users. The background polling task still runs server-side
to store the app password when the user grants access.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add /app/provision and /app/provision/status endpoints for browser-based
Login Flow v2 app password provisioning. Used by Astrolabe's "Enable
Semantic Search" to chain OAuth (bearer token) + Login Flow v2 (app
password) in a single user interaction.
The provision page initiates Login Flow v2, opens Nextcloud's login URL
in a popup, polls for completion via background task, and redirects back
to the caller's redirect_uri on success.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add docker compose services (tailscale-mcp + nginx-claude-filter) behind
a claude-funnel profile that expose the login-flow MCP server via
Tailscale Funnel with IP-based access control:
- /mcp endpoint restricted to Claude AI outbound IPs (160.79.104.0/21)
- /oauth/*, /.well-known/*, /app paths open to all IPs (user login flow)
- All other paths return 404
Also add favicon.png served at /favicon.ico for connector directory
discovery (Google favicon service).
Usage:
docker compose --profile login-flow --profile claude-funnel up -d
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Relax redirect_uri validation to accept HTTPS for remote hosts (e.g.,
cloud-hosted MCP clients like Claude AI) while keeping HTTP allowed
for localhost per RFC 8252 loopback exception.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move 26-configure-astrolabe-oauth.sh from post-installation (runs once
on first boot) to before-starting (runs on every start). This ensures
the Astrolabe OIDC client is created as soon as MCP_SERVER_URL is
available, even if it wasn't set during initial installation.
Also copy 25-configure-mcp-server-url.sh to before-starting so the
mcp_server_url config stays current across container recreations.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Drop OIDC fork: comment out third_party/oidc mount, use upstream
v1.16.3 from app store (fixes consent redirect race, PR #631)
- Support client_secret_basic auth: add _extract_basic_auth() helper
so TS MCP SDK can authenticate at token endpoint (RFC 6749 §2.3.1)
- Multi-issuer JWT validation: accept tokens with internal Docker
issuer (http://app:80) or public URL (NEXTCLOUD_PUBLIC_ISSUER_URL)
since AS proxy obtains tokens server-to-server
- Introspection fallback: try token introspection when JWT verification
fails, supporting both JWT and opaque token types
- Register all tool scopes in DCR: add semantic:read, collectives:read,
collectives:write to OIDC client allowed_scopes so tokens include
them and semantic search tools are visible to authenticated clients
- Auto-create Astrolabe OAuth client: new app-hook creates OIDC client
and stores credentials in config.php so the "Authorize via OAuth"
button works without manual setup
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The custom regex manager matched all nextcloud_image entries with the
same depName, causing Renovate to bump all matrix entries (NC 31, 32, 33)
to the latest version instead of only the targeted major.
Fix by capturing nextcloud_version to create version-specific dep names
(nextcloud-31, nextcloud-32, nextcloud-33) with allowedVersions rules
constraining each to its own major. Also pins docker-compose.yml to 32.x
and removes redundant inline # renovate: comments that could cause
duplicate matching.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix emoji clearing bug: use _UNSET sentinel in update_collective so
emoji=None sends {"emoji": null} instead of raising ValueError
- Move collectives_get_trashed_collectives to Read Tools section
- Remove redundant is_trash field from ListTrashedPagesResponse
- Add page lifecycle note to collectives_trash_page docstring
- Add unit test for clearing collective emoji via update_collective
- Add integration test for clearing collective emoji via MCP tool
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix inconsistent error code in set_collective_emoji (400 → -32603)
- Allow clearing emoji via set_collective_emoji(emoji=None)
- Remove destructiveHint from trash operations (soft deletes are recoverable)
- Change delete_collective to idempotentHint=False (requires trash precondition)
- Add restore_collective and get_trashed_collectives tools
- Add unit tests for ValueError guard, clear-emoji path, and new tools
- Add integration test for full trash/restore/delete lifecycle
- Verify move_page returns new title in response message
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Rename collectives_update_collective to collectives_set_collective_emoji
(more precise since only emoji is settable)
- Use standard JSON-RPC error code -32603 (INTERNAL_ERROR) instead of -1
- Handle UnicodeDecodeError when reading page content via WebDAV
- Replace brittle 'Welcome' content assertion with length check
Fixes CI: test_update_operations_not_idempotent no longer matches the
renamed tool, which is correctly idempotent (no ETag involved).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Starlette 1.0.0 removed the @app.middleware() decorator, which breaks
nextcloud-mcp-server on fresh installs. Pin starlette<1.0 until we
address the full set of breaking changes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix assign_tag sending Content-Type header with no body
- Mark collectives_update_collective as idempotent (no ETag involved)
- Raise OCSError when 'data' key missing instead of silent fallback
- Tighten color validator to 3 or 6 hex chars only
- Add comment explaining null emoji semantics in set_page_emoji
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Validate OCS envelope in trash_collective, delete_collective, trash_page
- Guard _unwrap_ocs against non-OCS responses with informative OCSError
- Remove _get_ocs_headers() indirection, use class constants directly
- Split headers: _OCS_HEADERS (GET) vs _OCS_HEADERS_JSON (with body)
- Fix docstring claiming emoji param is required when it is optional
- Rename misleading test, add test for non-OCS envelope handling
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add collectives_trash_collective and collectives_delete_collective MCP
tools with proper destructiveHint annotations. Refactor integration test
fixture to use MCP tools for cleanup instead of direct httpx/OCS calls.
Optimize _get_ocs_headers() to class-level constant.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bugs:
- assign_tag/remove_tag now call _unwrap_ocs to surface OCS-level errors
- trash_page changed to idempotentHint=False (trashing twice errors)
- WebDAV path parts stripped of slashes to prevent double-slash paths
Robustness:
- _unwrap_ocs uses ocs.get("data", {}) instead of ocs["data"]
- Unit test added for missing data key in OCS envelope
Minor:
- MCP error codes use -1 (project convention) instead of HTTP status codes
- update_collective docstring notes that emoji is required
- CollectiveTag.color validated as hex format via field_validator
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bug fixes:
- Catch OCSError/HTTPStatusError in all server tools, convert to McpError
- Guard update_collective against empty body (raise ValueError)
- Use restore_page response data in status message
ADR-017 annotation fix:
- Distinguish "remove" (reversible association) from "delete" (permanent):
remove_tag and deck_remove_label_from_card no longer set destructiveHint
- Update annotation test to exclude "remove" from destructive keywords
Data model improvements:
- Add trashTimestamp field to PageInfo
- Create ListTrashedPagesResponse with is_trash context flag
- Add collective_id to ListTagsResponse
Test robustness:
- Read NC credentials from environment variables (not hardcoded)
- Filter landing page by parentId == 0 instead of assuming pages[0]
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add destructiveHint=True to collectives_remove_tag (matches "remove"
keyword pattern in annotation tests)
- Change collectives_update_collective to idempotentHint=False (update
operations are non-idempotent per project convention)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Validate OCS envelope status before unwrapping data (raise OCSError on
statuscode >= 400)
- Fix test data: filePath should be "" for root-level pages, not filename
- Catch specific exceptions (HTTPStatusError, OSError) instead of bare
Exception in WebDAV content fetch, include error in log message
- Return updated resource data from update_collective, move_page, and
set_page_emoji instead of discarding API responses
- Fix create_page docstring to mention collectivePath/filePath/fileName
- Remove unused additional_headers parameter from _get_ocs_headers
- Add unit test for OCS error status validation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implement MCP tools for the Collectives wiki/documentation app, enabling
agentic workflows for team knowledge base management.
16 tools covering collectives, pages, tags, search, and trash:
- Read: list collectives, list/get pages (with WebDAV content), search,
list tags, list trashed pages
- Write: create/update collective, create/move/trash/restore pages,
set emoji, create/assign/remove tags
Includes Docker hook for app installation, OCS API client with envelope
unwrapping, Pydantic models, unit tests (16), and integration tests (10).
Closes#621
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The fork (cbcoutinho/qdrant-client fix/fusion-score-threshold) has been
merged upstream as PR #1138 and released in v1.17.0. Remove the git source
override in [tool.uv.sources] to unblock clean PyPI publishing, since git
dependencies are excluded from wheels.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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>
Extract reusable wait_for_vector_sync() helper with 90s max_wait (up from
30s) to handle slow single-worker processing in CI. Increase processor
workers to 2 for the mcp service to parallelize note indexing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test_semantic_search_answer_successful_sampling test creates a note
and waits 30s for indexing, but the scanner only ran every 60s. Aligning
with the CI overlay's 5s interval ensures new notes are indexed in time.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Make /api/v1/status and /api/v1/vector-sync/status available in all
non-Smithery deployment modes so Astrolabe can show server status even
in single-user BasicAuth mode. Previously these were only mounted when
OAuth or multi-user BasicAuth with offline access was enabled.
- Split management API routes into public (Tier 1) and authenticated (Tier 2+)
- Enable semantic search with in-memory Qdrant for single-user docker service
- Update astrolabe submodule with admin settings fix
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Root cause: ConsentController::grant() only passed client_id and scope
in the post-consent redirect, relying on PHP session fallback for state,
response_type, redirect_uri etc. On NC 32 (PHP 8.4), session values
were intermittently lost between session->close() and the subsequent GET
request, causing 500 errors from trim(null) / matchRedirectUri(null).
OIDC app fixes:
- Pass all OAuth params in consent redirect URL (eliminates session race)
- Add null safety guard in authorize endpoint (400 instead of 500)
Test infra fixes:
- Wait for OIDC redirect chain to settle before handling consent screen
(fixes "Execution context was destroyed" Playwright errors)
- Capture nextcloud.log in CI failure artifacts for PHP error debugging
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update stale httpx reference to niquests in calendar.py type comment
- Replace inline inspect.isawaitable with _maybe_await helper in tests
- Fix incorrect port number in docker-compose unstructured comment
- Remove commented-out smithery service block (dead code)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The OIDC submodule volume mount is on the base app service, so all
modes mount it. Without composer install, the post-install hook enables
a broken app (missing vendor/autoload.php), causing Nextcloud to fail.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The OIDC app 1.16.2 broke the consent flow by only falling back to
session params when client_id is missing. After consent, the redirect
includes client_id and scope but loses state, response_type, and
redirect_uri — causing a 500. The submodule fix restores per-param
session fallback when ANY critical param is missing.
Also adds a CI build step for the OIDC app (composer + npm) so the
JS assets (oidc-consent.js, oidc-redirect.js) are available in OAuth
test profiles.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
