fix(ui): Remove leaked think and final tags#3
Open
iclem wants to merge 12 commits intofeat/nextcloud-talkfrom
Open
fix(ui): Remove leaked think and final tags#3iclem wants to merge 12 commits intofeat/nextcloud-talkfrom
iclem wants to merge 12 commits intofeat/nextcloud-talkfrom
Conversation
- Export parseStructuredNextcloudTalkBody and resolveExplicitNextcloudTalkMention - Rename mentionDebug -> mentionEntries and drop unused explicitMention field from the parsed body struct - Remove verbose per-message debug log line from the hot path - Add inbound.mentions.test.ts with 16 unit tests covering plain text pass-through, structured JSON extraction, malformed JSON fallback, and mention matching (id, mentionId, name, email local-part, case-insensitive, type guard)
…ge body
Nextcloud Talk's structured payload puts mention tokens like {mention0} or
{mention-user1} directly in the message string. Leaving them in effectiveBody
caused two problems:
- Command parsing (hasControlCommand, directives) would not see commands that
follow an @-mention, e.g. "{mention0} /reset" -> "/reset" was never reached.
- A mention-only message ("{mention0}") produced a non-empty prompt dispatched
to the agent, causing spurious replies.
Fix: strip all {key} placeholder tokens whose keys appear in parameters[], then
treat a structured body that is empty after stripping as a mention-only ping and
return early without dispatching.
Also add ParsedNextcloudTalkBody.structured flag to distinguish plain-text
fall-through from a successfully parsed JSON body.
Tests: 2 new cases (mention-only drop, command after mention), updated existing
assertions to reflect stripped text. 18 unit tests, 68 total, all passing.
- Outbound: wire sendReactionNextcloudTalk into a ChannelMessageActionAdapter (nextcloudTalkMessageActions) exposing the 'react' action on the message tool - Inbound: extend webhook decoder to parse Like/Dislike events (Talk 21+) into NextcloudTalkInboundReaction; route through onReaction callback in monitorNextcloudTalkProvider; add handleNextcloudTalkInboundReaction stub (TODO: full dispatch on a future pass) - Types: extend NextcloudTalkWebhookPayload with Like/Dislike types and optional content field; add NextcloudTalkInboundReaction type - SDK: export jsonResult, readReactionParams, readStringParam from plugin-sdk/nextcloud-talk surface - Tests: channel-actions.test.ts (2 tests), monitor.reaction.test.ts (3 tests) — all 73 nextcloud-talk tests pass
Adds an optional `ackReaction` config field (account- and room-level). When set, the bot sends a reaction emoji to every received message that passes all gate checks (best-effort; errors are logged, not thrown). Room-level `ackReaction` takes precedence over account-level. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…c ackReaction field
Implements typing indicator signaling so the bot shows a 'composing'
status while processing a message.
- Add NextcloudTalkTypingIndicator type and typingIndicator config option
to NextcloudTalkRoomConfig and NextcloudTalkAccountConfig
- Add send-typing.ts: sendTypingNextcloudTalk() using the proposed bot
typing endpoint POST /ocs/v2.php/apps/spreed/api/v1/bot/{token}/typing
with HMAC-SHA256 auth (same pattern as sendMessageNextcloudTalk)
- Gracefully handles 404 (endpoint not yet available on server) with a
one-time console.warn and continues without throwing
- Add resolveTypingIndicatorEnabled() for room-level override of
account-level setting (room config takes precedence)
- Integrate into inbound.ts: typing=true before dispatch, typing=false
after delivery or on dispatch error (fire-and-forget, non-blocking)
- Add config-schema.ts entries for typingIndicator on both room and
account schemas
- Add 16 unit tests covering valid/error paths and config resolution
Feature is opt-in: typingIndicator defaults to false. Enable per-account
or override per-room with typingIndicator: true/false.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb22ceb729
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if (item.type === "text" && typeof item.text === "string" && isAssistantMessage) { | ||
| const expanded = expandTextContent(item.text); | ||
| const expanded = expandAssistantTextContent(item.text); |
There was a problem hiding this comment.
normalizeMessage now runs sanitizeAssistantVisibleText on each assistant text block independently, so malformed reasoning tags that span blocks are not handled correctly. If block A opens <think> and block B contains continuation text (or a visible answer) without a closing tag, block A is dropped but block B is still rendered, which leaks content that the new strict behavior is supposed to suppress until thinking closes. This affects assistant messages delivered as multi-block arrays, not just single-string payloads covered by the new tests.
Useful? React with 👍 / 👎.
iclem
force-pushed
the
feat/nextcloud-talk
branch
from
b8a08c3 to
aa2a646
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Describe the problem and fix in 2–5 bullets:
If this PR fixes a plugin beta-release blocker, title it
fix(<plugin-id>): beta blocker - <summary>and link the matchingBeta blocker: <plugin-name> - <summary>issue labeledbeta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write
N/A. If the cause is unclear, writeUnknown.Regression Test Plan (if applicable)
For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write
N/A.User-visible / Behavior Changes
List user-visible changes (including defaults/config).
If none, write
None.Diagram (if applicable)
For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write
N/A.Security Impact (required)
Yes/No)Yes/No)Yes/No)Yes/No)Yes/No)Yes, explain risk + mitigation:Repro + Verification
Environment
Steps
Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes/No)Yes/No)Yes/No)Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.