From 8f92a7ea29e4ceb27dbed7673bea80cf23014d7e Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Thu, 30 Apr 2026 00:29:04 +0200 Subject: [PATCH] docs(talk): address PR #741 reviewer nits 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) --- app-hooks/post-installation/10-install-spreed-app.sh | 5 +++++ nextcloud_mcp_server/models/talk.py | 3 +++ nextcloud_mcp_server/server/talk.py | 1 + 3 files changed, 9 insertions(+) diff --git a/app-hooks/post-installation/10-install-spreed-app.sh b/app-hooks/post-installation/10-install-spreed-app.sh index a4b84974..f1df23b6 100755 --- a/app-hooks/post-installation/10-install-spreed-app.sh +++ b/app-hooks/post-installation/10-install-spreed-app.sh @@ -6,4 +6,9 @@ set -euox pipefail # app is not yet installed, so app:install pulls it. On rebuilds where the # volume already has it, app:install would fail, so fall through to # app:enable. +# +# Caveat: this `||` also masks unrelated install failures (network outage, +# bad version pin, etc.) — they will fall through to app:enable, which +# will then fail with a clearer "app not found" error. Acceptable for a +# dev-fixture script; do not copy this pattern into production tooling. php /var/www/html/occ app:install spreed || php /var/www/html/occ app:enable spreed diff --git a/nextcloud_mcp_server/models/talk.py b/nextcloud_mcp_server/models/talk.py index 536ab2cd..0312bb03 100644 --- a/nextcloud_mcp_server/models/talk.py +++ b/nextcloud_mcp_server/models/talk.py @@ -58,6 +58,9 @@ class TalkConversation(BaseModel): unreadMessages: int = 0 unreadMention: bool = False lastActivity: int | None = None + # spreed returns `0` (not `null`) when the user has not read any + # message in the room yet — compare to ``None`` explicitly rather + # than relying on truthiness, since `0` is falsy but valid. lastReadMessage: int | None = None lastMessage: TalkMessage | None = None readOnly: int | None = None diff --git a/nextcloud_mcp_server/server/talk.py b/nextcloud_mcp_server/server/talk.py index cdf8a9a3..a8bdfb36 100644 --- a/nextcloud_mcp_server/server/talk.py +++ b/nextcloud_mcp_server/server/talk.py @@ -188,6 +188,7 @@ def configure_talk_tools(mcp: FastMCP) -> None: token, message, reply_to=reply_to, + # 32 hex chars, no dashes — spreed accepts either UUID format. reference_id=uuid.uuid4().hex, silent=silent, )