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>
Surface the text-merge update path's limitation rather than silently no-op:
when contact_data['email'] or ['tel'] arrives as a dict/list on update, log a
warning at the top of _merge_vcard_properties pointing callers at plain str
or create_contact. Existing EMAIL/TEL lines are still preserved unchanged.
Bring nc_contacts_update_contact docstring into parity with create — the
update tool now documents the same keys plus the explicit single-string
limitation for email/tel and the BDAY validation / URL first-only behaviours.
Three new TestMergeVcardProperties cases pin the warning: dict email warns,
list tel warns, plain str email is silent (no false positives).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- _merge_vcard_properties: list-form ORG was passed through
_safe_vcard_value unchanged, emitting a Python repr on the wire. Both
branches now ;-join list components per RFC 6350 §6.6.4 (ORG is
Company;Department;…) before interpolation.
- _wrap_contact_field: a dict whose ``type`` was a bare string used to
hit ``list("WORK")`` and explode into ``["W","O","R","K"]``. Wrap
bare-string types into a single-element list before the list() call.
Regression tests pin both shapes:
- list-org overwrites and add-new produce ``ORG:Acme;Engineering``
- dict email with ``type="WORK"`` (bare str) emits ``EMAIL;TYPE=WORK:``,
not ``EMAIL;TYPE=W,O,R,K:``.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- _merge_vcard_properties no longer silently drops the existing EMAIL /
TEL line when contact_data supplies a dict/list shape: the input is
unhandled by the text merge, so the original line is preserved
instead of being consumed and replaced with nothing.
- Extracted _parse_bday so the update path validates ISO format the
same way create does. Invalid → keep existing BDAY line (or skip on
add-new) rather than writing a malformed one.
- Added _safe_vcard_value to escape newlines per RFC 6350 §3.4 at every
interpolation site in _merge_vcard_properties, blocking value-driven
property injection (e.g. NOTE: containing a literal \n + EMAIL:).
- Removed dead "organization" alias references from _merge_vcard_properties:
unreachable since update_contact normalises before calling.
- New regression tests pin all four behaviours (dict-email preserves
existing line, list-tel ditto, invalid-bday-update preserves original,
invalid-bday-add-new is dropped, newline-in-note no injection).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulls the remaining review feedback into one commit:
- Remove the double _normalize_contact_data call: the helper now assumes
canonical keys, and create_contact normalises before calling it
(update_contact already did). Docstring states the invariant.
- Drop phone/organization from _SUPPORTED_CONTACT_KEYS; they never reach
the unknown-key check post-normalisation.
- Tighten generics to dict[str, Any] / list[str] across helpers and
ContactsClient signatures.
- Comment both URL-merge sites noting only the first URL is written.
- Log a warning when fn is missing from contact_data.
- Test coverage for _wrap_contact_field dropping value-less dicts and
for the fn-missing warning; _vcard helper now mirrors the real call
chain (normalise → build).
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>
The previous manifest hardcoded /bin/sh which fails on Windows. Fix by:
- Adding mcpb/run.cmd: Windows batch wrapper that searches common uvx
install locations (%USERPROFILE%\.local\bin, %LOCALAPPDATA%\uv\bin,
%USERPROFILE%\.cargo\bin) before falling back to PATH, with a clear
error message pointing to winget/PowerShell install instructions
- Adding platform_overrides.win32 in mcp_config to use cmd /c run.cmd
on Windows while macOS/Linux continue using /bin/sh run.sh
- Expanding compatibility.platforms to include win32
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Use SPDX identifier AGPL-3.0-only (consistent with pyproject.toml)
- Add compatibility.platforms: [darwin, linux] — run.sh uses /bin/sh
and is not compatible with Windows; this surfaces that at install time
rather than silently failing to launch
- Improve run.sh fallback: use 'command -v uvx' before exec so a missing
uvx prints a clear error with install instructions instead of a generic
shell 'not found' message
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a `mcpb/` directory containing a Claude Desktop Extension bundle
that lets users install nextcloud-mcp-server from Claude Desktop without
needing HTTPS — the extension uses stdio transport (already supported via
`nextcloud_mcp_server/stdio.py`) so no network connection or TLS is
required.
Bundle contents:
- `manifest.json` — extension metadata, user_config fields for
NEXTCLOUD_HOST, NEXTCLOUD_USERNAME, and NEXTCLOUD_PASSWORD (sensitive,
stored in OS keychain)
- `run.sh` — shell wrapper that locates `uvx` across common install
paths (official uv installer at ~/.local/bin, Homebrew at
/opt/homebrew/bin, etc.) since Claude Desktop uses a restricted PATH
- `.gitignore` — excludes the built *.mcpb artifact
Build the distributable with:
npm install -g @anthropic-ai/mcpb
cd mcpb && mcpb pack
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PR #719 review raised a claim that these three fields fall through to
the "keep unchanged" catch-all in _merge_vcard_properties. The existing
elif branches for NICKNAME/BDAY/CATEGORIES already prevent that, but
the behaviour wasn't pinned by a test. Add a focused TestMergeVcardProperties
class that calls the merge helper directly and asserts:
- Existing NICKNAME/BDAY/CATEGORIES lines are overwritten by new values.
- When the existing vCard has none of these lines, update adds them.
- A URL update doesn't clobber unrelated ORG/NOTE/TEL properties.
If the primary update path ever regresses for these fields, these tests
will catch it immediately.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Type-annotate _wrap_contact_field signature; drop stale "url" mention
from its docstring (url is handled by the list-coercion helper, not
this one).
- Split the shape-coercion helper so comma-splitting only applies to
categories: _as_str_list (no split) for org/nickname/url,
_split_categories (comma split) for CATEGORIES. Fixes the case where
organization="Smith, Jones & Associates" was mangled into a two-
component ORG.
- Share _normalize_contact_data between create and update so
_merge_vcard_properties only sees canonical keys; add URL handlers in
both update branches so the primary update path no longer drops URL
silently.
- Annotate the Contact(**kwargs) type:ignore with the reason
(pythonvCard4 typeshed doesn't accept **dict[str, Any]).
- Add tests/unit/client/test_contacts.py (pure unit, no HTTP) covering
the #716 round-trip, comma-in-org regression, invalid-bday warning,
tel/phone precedence, categories string-vs-list behaviour, and direct
_normalize_contact_data cases.
- Extend the MCP workflow test with an update-with-url step asserting
the URL handler in _merge_vcard_properties.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
create_contact previously read only fn/email/tel from contact_data and
silently dropped org, organization, note, title, nickname, bday,
categories, url — and didn't accept phone as an alias for tel, so the
reporter's exact call lost every field except fn and email. Introduce
_build_contact_from_data, share it with update_contact's fallback, and
normalise str→list inputs so pythonvCard4 doesn't iterate bare strings
character-by-character for list-typed properties.
Closes#716
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>
