Fix issues raised by reviewer on PR #743:
- troubleshooting.md: renumber "Getting Help" steps (4→3, 5→4) after
earlier consolidation left a gap
- installation.md: drop stale "OIDC app" prerequisite; admin access is
now optional under Login Flow v2 (works on stock Nextcloud 16+)
- semantic-search-architecture.md: rename VECTOR_SYNC_ENABLED to
ENABLE_SEMANTIC_SEARCH in the Status callout (renamed in v0.58.0)
- configuration.md: remove Quick Start references to deprecated
oauth-multi-user / oauth-advanced templates and point to
login-flow-v2.md; update "OAuth, Multi-User BasicAuth" label to
"Login Flow v2, Multi-User BasicAuth"
- auth-flows.md: fix background-sync diagram so Encrypt+persist step
no longer crosses into the Nextcloud column
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both auth surfaces now fail-closed by default:
- ALLOWED_MCP_CLIENTS: removed the silent `claude-desktop` and
`test-mcp-client` fallbacks. Empty/unset env var leaves the registry
empty so /oauth/authorize rejects every client_id.
- ALLOWED_MGMT_CLIENT (new): comma-separated list of OIDC client_ids
whose tokens are accepted by /api/management/*. Enforced in
verify_token_for_management_api on both the cache-hit and cache-miss
paths against the token's client_id claim. Unset/empty rejects all.
Compose: set ALLOWED_MGMT_CLIENT=nextcloudMcpServerUIPublicClient on
mcp-multi-user-basic so the existing Astrolabe integration test
(test_astrolabe_chunk_context.py) still passes.
env.sample documents both vars and notes they may be consolidated later.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Nextcloud installs without pretty URLs return a 301 from
`/.well-known/openid-configuration` to
`/index.php/.well-known/openid-configuration` (e.g. Hetzner StorageShare).
`_get_cached_discovery` did not enable follow_redirects, so httpx raised
HTTPStatusError on the 301 and the AS-proxy authorize handler returned
500, breaking client connections (e.g. claude.ai).
Pass `follow_redirects=True` to the httpx client used for the discovery
fetch only — downstream OIDC endpoints (token, userinfo, etc.) are
absolute URLs read from the discovery doc and are unaffected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the seven outstanding items from the @claude review on PR #741:
1. Add empty `tests/client/talk/__init__.py` for pytest discovery parity
with `tests/client/{collectives,news}/`.
2. Standardise boolean query params to integers — `includeStatus` was the
string `"true"` in `list_conversations`/`list_participants` while every
other flag (`noStatusUpdate`, `lookIntoFuture`, `setReadMarker`,
`includeLastKnown`) used `1`/`0`.
3. Replace the `app:install || app:enable` chain in the spreed install hook
with `app:install --keep-disabled --force || true; app:enable spreed`,
so unrelated install failures surface as a clear "app not found" from
`app:enable` rather than being silently masked.
4. Add `_validate_token()` (alphanumeric whitelist) and call it from all
six TalkClient methods that interpolate the token into a URL path —
defence-in-depth against pathological tokens reaching httpx.
5. Rename `TalkConversation.type` to `room_type` with `Field(alias="type")`
and `populate_by_name=True`, so the field no longer shadows Python's
builtin while preserving spreed's wire format on input. MCP responses
now serialize `room_type` (field name) instead of `type`.
6. `mark_as_read` now passes `json=body or None` so the bodyless
"mark everything as read" call doesn't send a spurious `{}` body and
`Content-Type: application/json` header.
7. `_validate_message_text` rejects whitespace-only messages, not just
empty strings.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses the missing-test and Content-Type points from the latest
PR #741 review:
- client/talk.py _talk_headers(): drop the manual `Content-Type:
application/json`. httpx sets it automatically on requests that pass
`json=`, and we no longer leak it onto bodyless GETs and DELETEs.
- tests/client/talk/test_talk_api.py:
- new `test_talk_list_participants_with_include_status` asserting
`includeStatus=true` is forwarded.
- new `test_talk_get_messages_invalid_last_given_header` covering
the defensive try/except around the `X-Chat-Last-Given` parse —
asserts the fallback `last_given=None` and that a warning is
logged.
- existing `test_talk_list_participants` extended to assert that
`includeStatus` is *absent* by default.
Unit tests: 13 → 15. Integration tests still 7/7.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the seven OAuth-to-Nextcloud docs (oauth-setup, quickstart-oauth,
oauth-architecture, oauth-upstream-status, oauth-troubleshooting,
jwt-oauth-reference, audience-validation-setup) with a single new
docs/login-flow-v2.md. The deprecated flow required upstream user_oidc
patches that were never merged; Login Flow v2 is the forward-looking
multi-user mode (see ADR-022), and works with stock Nextcloud 16+.
Rewrite docs/authentication.md and docs/auth-flows.md around three modes:
Single-User BasicAuth, Multi-User BasicAuth pass-through, and Login Flow v2.
Update README to add an Astrolabe Cloud (https://astrolabecloud.com)
callout for users who prefer not to self-host, drop the OAuth deployment
mode from the auth table, simplify the Docker block, and trim the
Examples and Security sections.
Sweep configuration.md, installation.md, troubleshooting.md, running.md,
and semantic-search-architecture.md to replace links to the deleted docs
and update deprecated mode names.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
- SECURITY.md: add Supported Versions table; reword SLA paragraph as
a bullet list per reviewer suggestion
- bug_report.yml: render reproduction textarea as shell so commands and
JSON get syntax highlighting, matching the logs field
- question.yml: add transport and install_method dropdowns mirroring
bug_report.yml so setup questions capture the same context
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pymupdf-layout is Artifex commercial-only proprietary (its wheel ships
only a one-line COPYING noting "Commercial license. See artifex.com"),
incompatible with the project's AGPL build. It was declared as a runtime
dependency but unused: not imported anywhere in nextcloud_mcp_server/ or
tests/.
Also drop tools/parse-doc.py, an unused dev scratch script that was the
only caller of pymupdf.layout.activate(). Per the explicit warning in
document_processors/pymupdf.py, activating layout breaks
pymupdf4llm.to_markdown(page_chunks=True) per pymupdf4llm#323.
Closes#725
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>
Surface GitHub's native private reporting workflow as the primary
disclosure channel, with security@astrolabecloud.com kept as a fallback
for reporters without a GitHub account. Updates SECURITY.md, the README
Security section, the issue-template config link, and the bug-template
warning banner.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a security policy directing private vulnerability reports to
security@astrolabecloud.com instead of public issues, and update the
README's Security section to point at it.
Add structured issue forms under .github/ISSUE_TEMPLATE/ covering bugs,
feature requests, questions, and documentation, plus a config.yml that
disables blank issues and routes security reports and open-ended
questions to the appropriate channels. The bug template captures
fields most commonly missing from past reports (server/Nextcloud/app
versions, deployment mode, transport, MCP client).
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>
caldav 3.x lists niquests as a mandatory dependency and prefers it over
httpx. Passing httpx.BasicAuth via the auth= argument breaks under the
niquests backend with "Unexpected non-callable authentication" — see #731.
Switch CalendarClient.__init__ from auth=Auth|None to keyword-only
password/token, and forward them to AsyncDAVClient as password= plus an
explicit auth_type ("basic" or "bearer"). caldav then builds whichever
auth object its active backend needs (niquests.auth.HTTPBasicAuth or
httpx.BasicAuth), so we stay backend-agnostic.
Threaded raw credentials through NextcloudClient — added keyword-only
password/token to its __init__, and updated from_env, from_token, and
the four call sites that build NextcloudClient (context.py basic-auth
and Login Flow paths, auth/userinfo_routes.py, vector/oauth_sync.py).
Four new unit tests pin the construction wiring so the niquests
regression can't recur silently — basic, bearer, no-creds, and
password-precedence cases.
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>
