Agents plugin 2.x catch-up: interactive questions, thread/unreads analysis, custom prompts, and rendering fixes - #10026
Open
crspeller wants to merge 13 commits into
Open
Agents plugin 2.x catch-up: interactive questions, thread/unreads analysis, custom prompts, and rendering fixes#10026crspeller wants to merge 13 commits into
crspeller wants to merge 13 commits into
Conversation
…_default and turn_count columns - Raise the plugin version floor to 2.0.0; Agents hides entirely on older plugins - Rename the agent-list-based observeIsAgentsEnabled to observeHasAgents to resolve the collision with the version-based gate of the same name - Drop the bridge /api/v4/agents client method; fetchAgents now projects the plugin /ai_bots response (same bot-user ID space, superset of fields) into the rewrite store while persisting bots to the database - Read the camelCase isDefault field the plugin actually sends; sort cached bots default-first so server ordering survives caching - Drop the bridge-only service_type from the agent selector subtitle - Add resolveAgentSelection: shared >1-agent picker rule for action entry points - Single schema migration to v21: ai_bot.is_default, ai_thread.turn_count (transformer, comparator, models, docs updated) Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
WS1 — tool-call block metadata:
- Mirror would_auto_execute, decided_at, mcp_bare_name, and server_origin
from the plugin's tool_use/tool_result content blocks onto ToolCall
- Hide approval controls for policy-approved pending calls; a mixed batch
no longer counts them as needing decisions, and an interrupted all-auto
round gets a single Run tools button (submits no accepted ids; the
server re-checks policy and runs them)
- Skip share/keep-private re-prompting for results already decided
- Tool cards title with the server-provided MCP bare name, falling back
to a wire-prefix strip for pending blocks where the server omits it
- Redacted (null) arguments render nothing; empty arguments render an
explicit note instead of a {} code block
- Locally decided cards show an Accepted/Rejected status line instead of
going blank
WS5 — persisted citations:
- extractAnnotationsFromTurn now reads the Annotations content block
(web_search_context.results url_citations), the branch the plugin
actually writes, so sources survive the post-stream conversation
refetch and app restarts
- Strip inline OpenAI (source: URL) citation clutter from round text
Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
- ToolAnswer type ({selected, custom?}) mirroring mmtools.UserInteractionAnswer
- submitToolApproval (client + action) accepts answers keyed by tool_use
block ID and sends them as tool_answers alongside accepted_tool_ids
- user_interaction mirrored from tool_use content blocks onto ToolCall
Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
Agent questions were a hard dead-end on mobile: the generic tool card sent
{accepted_tool_ids} without an answer, the server replied 400
(ErrInvalidToolAnswer), and the only exit was Reject.
- QuestionCard renders AskUserQuestion tool calls inline: numbered radio
rows (single-select) or checkboxes (multi-select) with descriptions, a
free-form answer that expands in place (offered unless allow_free_form
is false), Skip, and Answered/Skipped/waiting states
- Single-select answers submit immediately on tap; multi-select and typed
free-form answers require an explicit Submit, since answers are terminal
server-side with no edit path
- ToolApprovalSet routes select-interaction calls to the card, records
answers keyed by tool id, sends them as tool_answers, and excludes
questions from batch accept/reject and the decisions counter; answered
results skip the share/keep-private stage
- Guard conversation fetches against a stale in-flight response
overwriting a newer refetch, and refetch once more when a streaming
session clears while the cached conversation still has no rounds for
the post (stream-end refetch can race the server's turn finalization,
which left the post blank until remount)
Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
AgentPost previously replaced the entire Body, silently dropping every piece of standard post chrome: file attachments (a files-only CreateFile reply rendered completely empty), reactions (writable server-side but never rendered), priority acknowledgements, and permalink preview embeds. - AgentPost now renders as the message slot inside Body, keeping agent posts on the standard render path so future Body additions apply to them automatically; conversation file blocks render through the same post-attachment path the webapp uses - Permalink embeds are recognized anywhere in the embeds list, not just at index 0 (matches the webapp, which checks every embed) Channel-switch perf on the agent-heavy test DM, measured via the mobile_channel_switch metric (same device, same channel, n=4 each): before mean 721.0ms (674.6-838.4), after mean 720.8ms (674.3-823.8) - no regression. Verified on device: files-only CreateFile reply renders the file card, reactions render on agent posts, question/tool/thinking rounds unchanged in the nested layout, ordinary channel view unchanged. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
WS4 - link safety: - Persist the plugin's global allowUnsafeLinks setting (returned by /ai_bots, previously fetched and discarded) into the agents config store - Pass isUnsafeLinksPost to the Markdown renderers in both agent post renderers and both tool card sites, so links, channel links, hashtags and LaTeX in agent-authored content stay inert unless the admin has explicitly allowed them (agent content may be prompt-injected; the plugin tags every bot post unsafe_links for this reason) WS7 - render fixes: - Regenerate no longer shows the old answer above the new stream: the prior persisted rounds are suppressed while regenerating (the server has already deleted those turns) and restored once the post-stream refetch lands; a failed regenerate request lifts the suppression - Long Thinking content is scrollable inside the existing 600dp cap via a nested scroll region instead of being clipped and unreachable; the gesture-handler ScrollView is used because the core one never wins the nested-pan arbitration inside the post touchable tree - Cold-opening a persisted agent post shows a loading indicator instead of a blank body while the conversation fetch is in flight Device-verified: agent reply with a raw URL, ~channel link, and #hashtag renders all three inert with allowUnsafeLinks off, and active after flipping it via the plugin admin config; regenerate replaces the old answer instantly with only the new stream visible. The nested reasoning scroll claims drags that start inside the region (the outer list no longer moves); the inner scroll motion itself could not be reproduced through the simulator driver's synthetic swipes, which teleport the touch rather than panning. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
…dates
- The threads list's new-chat button previously just navigated back into
the still-mounted conversation, so after a first message the user was
stuck in it; it now signals the agent chat to reset its conversation
root while keeping the selected agent
- Thread history rows hard-switch to the plugin 2.x turn_count field
('N messages'); every row previously read '0 replies' because the 2.0
DTO renamed the count and dropped the preview, whose dead render branch
is removed along with the unsupported pre-2.0 id fallback
- Handle the plugin's bots_invalidate websocket broadcast and refresh the
agent list on reconnect, so agents added or removed server-side update
the cached bots and the composer AI gate without an app restart
Device-verified: + button opens a fresh conversation with the same agent
selected; history rows show real counts; an agent created server-side
appeared in the selector within seconds and disappeared on deletion,
both without relaunching.
Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
…g, gating
- 'Summarize unreads' now sends since=<last channel visit> (viewedAt, the
same boundary as the New Messages line); the previously sent unreads_only
flag does not exist server-side, so the option silently produced a
whole-channel summary labelled as an unreads summary
- Send team_id so the server can set the LLM team context for DM/GM
channels, which carry no team of their own
- The agent picker filters channel-scoped agents with the same predicate
the webapp uses; a blocked agent previously reached the server and came
back as a generic 403 alert
- The sheet renders an explicit empty state when no agent is usable in the
channel instead of a dead option list with a blank selection
- The Ask Agents entry point hides on servers whose license does not cover
channel analysis (plugin IsBasicsLicensed: enterprise tier or dev mode)
and when no agents exist, mirroring the webapp's gating
- Add a privacy footer: analysis results stream to a DM only the requester
can see
Device-verified: with known read/unread state the unreads summary covered
only the two unread messages ('No other posts were found in the specified
timeframe'); a channel-scoped agent disappeared from the picker; Ask
Agents vanished from quick actions with server dev-mode off (unlicensed)
and returned when restored.
Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
- doThreadAnalysis client method: POST /post/{id}/analyze?botUsername=
with analysis_type (summarize_thread | action_items | open_questions)
- requestThreadAnalysis remote action mirroring channel summary: on
success, switch to the agent DM where the result streams
- One 'Ask Agents' row in the post options sheet (every regular post,
gated on analysis license + available agents + non-archived channel)
opening a thread-analysis bottom sheet with the three analysis types,
a Selected Agent row only when more than one channel-scoped agent is
usable, and the results-only-visible-to-you footer
- Harden the agent post terminal-state self-heal: retry the conversation
refetch on a short backoff ladder (0.5s/2s/5s) instead of a single
immediate refetch, since server-side turn persistence can lag the
stream-end signal by seconds and a lone refetch can cache a stale
conversation until remount
Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
- Client methods for the plugin's custom prompts endpoints: list, pins, set pin, and server-side render (channel_id + bot_username context) - Ephemeral per-server custom prompts store (D14 - no schema change), cleared on per-server logout, with a useCustomPrompts hook - Agent projection now carries dmChannelID so surfaces can detect agent DM channels (isAgentDMChannel util) - Pills surface: pinned prompts render as tappable pills on the agent chat intro; tapping renders the prompt server-side, posts it into the agent DM, and routes into the new conversation thread - Composer surface: 'Custom prompts' entry in the AI actions sheet (both edit and generation modes) opens an in-sheet prompt list; selecting renders with the channel + selected agent context into the draft, prepending the agent @mention outside agent DMs (web parity with custom_prompts_dropdown) - channelId threaded through quick actions to the AI actions sheet - No authoring/management on mobile; prompt visibility (own + shared) enforced server-side Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
- getChannelInterval client method: POST /channel/{id}/interval with
{start_time: unread boundary, end_time: 0 (until now), preset_prompt}
- requestUnreadsSummary remote action: on success switch to the agent DM
where the result streams (mirrors channel/thread analysis)
- 'Ask Agents' pill on the New Messages separator (channel view only),
mirroring the webapp's UnreadsSumarize button: hidden when the
analysis license gate fails or no agents exist; the separator line
itself stays non-tappable
- Compact three-preset sheet (summarize new messages / find action
items / find open questions) with channel-scoped agent filtering,
agent picker only when more than one agent is usable, and the
results-only-visible-to-you footer
- start_time uses the same viewedAt-derived lastViewedAt prop that
positions the New Messages line, so the summary window exactly
matches what the separator shows
Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
- Manual mock for @context/server so suites that automock the module keep a composable withServerUrl (automock returned undefined, crashing any import-time HOC composition like withDatabase(withServerUrl(...))) - Channel quick actions tests now arrange the analysis-license gate (EnableTesting + EnableDeveloper config) and seed an agent before expecting the Ask Agents option, plus a new test covering the hidden state when unlicensed Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
mattermost-build
added
the
E2E/Run
Coverage Comparison Report |
mattermost-build
removed
the
E2E/Run
crspeller
marked this pull request as ready for review
August 8, 2026 04:22
Documentation Impact Analysis — updates neededDocumentation Impact AnalysisOverall Assessment: Documentation Updates Recommended Changes SummaryThis PR updates the Mattermost mobile Agents integration to the agents plugin 2.x contract, adding several new user-visible features: interactive agent questions (AskUserQuestion card), thread analysis from the post long-press menu, unreads summarization from the New Messages separator ("Ask Agents" pill), and custom prompt consumption (pinned pills in agent chat intro and a "Custom prompts" panel in the composer). It also raises the minimum plugin version to 2.0 and gates channel/thread/unreads analysis on an Enterprise license. Documentation Impact Details
Recommended Actions
ConfidenceMedium — The docs repo pulls agent user/admin content from the plugin repo via
|
Contributor
📝 WalkthroughWalkthroughThis PR updates Agents plugin 2.x contracts, database models, agent synchronisation, analysis sheets, custom prompts, interactive tool approvals, agent post rendering, and channel entry points. It also adds licence gating, unsafe-link configuration, citation handling, and related tests. ChangesAgents plugin 2.x contracts and storage
Analysis and prompt flows
Interactive tools and rendering
Sequence Diagram(s)sequenceDiagram
participant User
participant PostOptions
participant AnalysisSheet
participant AgentsClient
participant Channel
User->>PostOptions: select Ask Agents
PostOptions->>AnalysisSheet: open with post and channel IDs
AnalysisSheet->>AgentsClient: request thread or unread analysis
AgentsClient->>Channel: switch to returned analysis channel
Channel-->>User: show analysis result
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
app/screens/channel/header/quick_actions/index.test.tsx (1)
93-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the required test names.
Rename each test to start with
should, such asit('should show Ask Agents option in all channel types', ...).As per coding guidelines, use
it('should...')test names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/screens/channel/header/quick_actions/index.test.tsx` around lines 93 - 116, Rename the three tests in the Ask Agents test block to use names beginning with “should”, including the all-channel-types, DM/GM-channels, and unlicensed-analysis cases, while preserving their existing test behavior.Source: Coding guidelines
app/products/agents/client/rest.ts (1)
103-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse nullish coalescing for the options fallback.
Replace
options || {}withoptions ?? {}. This preserves optional-argument semantics without treating other falsy runtime values as absent. As per coding guidelines, use??instead of||for applicable fallbacks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/products/agents/client/rest.ts` at line 103, Update the options destructuring in the affected request method to use nullish coalescing, replacing the `options || {}` fallback with `options ?? {}` while preserving the existing destructured fields.Source: Coding guidelines
app/products/agents/actions/remote/custom_prompts.ts (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a consistent operation prefix in custom prompt logs.
Use
[fetchCustomPrompts],[fetchCustomPromptPins], and[renderCustomPrompt]as the respective log-message prefixes.
app/products/agents/actions/remote/custom_prompts.ts#L23-L23: prefix the fetch-prompts log with[fetchCustomPrompts].app/products/agents/actions/remote/custom_prompts.ts#L40-L40: prefix the fetch-pins log with[fetchCustomPromptPins].app/products/agents/actions/remote/custom_prompts.ts#L61-L61: prefix the render log with[renderCustomPrompt].As per coding guidelines, include a function or class prefix in log messages.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/products/agents/actions/remote/custom_prompts.ts` at line 23, Update the logError calls in app/products/agents/actions/remote/custom_prompts.ts at lines 23, 40, and 61 to use the operation prefixes [fetchCustomPrompts], [fetchCustomPromptPins], and [renderCustomPrompt], respectively; no other logging behavior needs to change.Source: Coding guidelines
app/products/agents/components/question_card/index.tsx (1)
366-380: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider double-tap protection for the immediate single-select submit.
handleSubmitandhandleSkipuseusePreventDoubleTap.handleOptionPresssubmits immediately in single-select mode without that protection. Two fast taps on two different options can callonAnswertwice before the parent setsisSubmitting. The parent then records the second answer over the first.The parent guard reduces the impact, so this is a hardening suggestion only.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/products/agents/components/question_card/index.tsx` around lines 366 - 380, The immediate single-select submission in handleOptionPress lacks double-tap protection. Apply the existing usePreventDoubleTap mechanism to the onAnswer call in the non-multiSelect path, while preserving selection updates and multi-select behavior.app/products/agents/components/citations_list/index.tsx (1)
160-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid the inline arrow callback for the mapped citation item.
Line 163 creates a new arrow function for every annotation on every render. Extract a memoized
CitationItemchild that receives the annotation and the stablehandleCitationPress, and let the child invoke the handler with its own URL.As per coding guidelines: "Avoid inline arrow callbacks for mapped list items; pass a single memoized callback and let the child invoke it with the item ID."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/products/agents/components/citations_list/index.tsx` around lines 160 - 166, Replace the inline mapped Pressable callback in the annotations rendering with a memoized CitationItem child component that receives the annotation and stable handleCitationPress; have CitationItem invoke the handler using its own annotation URL while preserving the existing key, styles, and testID behavior.Source: Coding guidelines
app/products/agents/components/agent_post/agent_post_new.tsx (1)
184-185: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the ref write out of the render body.
isRegeneratingRef.currentis assigned during render. React can replay or discard a render, so a render-phase ref write can leak from work that never commits. Assign the ref in an effect instead.♻️ Proposed change
const [isRegenerating, setIsRegenerating] = useState(false); const isRegeneratingRef = useRef(isRegenerating); - isRegeneratingRef.current = isRegenerating; + useEffect(() => { + isRegeneratingRef.current = isRegenerating; + }, [isRegenerating]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/products/agents/components/agent_post/agent_post_new.tsx` around lines 184 - 185, Move the isRegeneratingRef.current assignment out of the render body and into an effect that runs when isRegenerating changes, while preserving the ref’s current-value behavior for committed renders.Source: Linters/SAST tools
app/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsx (1)
176-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a component prefix to the log message.
The coding guidelines require a function or class prefix in log messages. Change the message to identify
ThreadAnalysisSheet.♻️ Proposed change
- logError('Failed to persist agent selection', getFullErrorMessage(error)); + logError('ThreadAnalysisSheet: failed to persist agent selection', getFullErrorMessage(error));As per coding guidelines: "Include a function or class prefix in log messages."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsx` around lines 176 - 179, Update the error message in the saveSelectedAgent handling within ThreadAnalysisSheet to include the required ThreadAnalysisSheet component prefix while preserving the existing error details.Source: Coding guidelines
app/products/agents/components/unreads_summary_sheet/unreads_summary_sheet.tsx (2)
176-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a component prefix to the log message.
Change the message to identify
UnreadsSummarySheet.As per coding guidelines: "Include a function or class prefix in log messages."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/products/agents/components/unreads_summary_sheet/unreads_summary_sheet.tsx` around lines 176 - 179, Update the error message in the saveSelectedAgent error path to include the component prefix “UnreadsSummarySheet,” while preserving the existing failure context and getFullErrorMessage(error) details.Source: Coding guidelines
35-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDo not reuse
agents.thread_analysis.*message ids in the unreads sheet.
actionItemsandopenQuestionspoint at ids owned by the thread-analysis feature. A future reword of the thread-analysis strings silently changes this sheet. Define ids underagents.unreads_summary.*instead, then runnpm run i18n-extract.As per coding guidelines: "Define new messages with
defineMessages()and runnpm run i18n-extractto updateen.json."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/products/agents/components/unreads_summary_sheet/unreads_summary_sheet.tsx` around lines 35 - 39, Update the actionItems and openQuestions entries in defineMessages to use unique agents.unreads_summary.* message IDs instead of agents.thread_analysis.* IDs, then run npm run i18n-extract to update the translation catalog.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/products/agents/actions/remote/bots.ts`:
- Around line 25-28: Update fetchAIBots to track a per-server refresh generation
before each request, then verify that generation is still current when the
response completes. Discard stale responses before both setAgentsConfig and bot
database updates, and add a test covering concurrent requests that complete in
reverse order.
In `@app/products/agents/actions/remote/threads.ts`:
- Around line 14-24: Update handleAIThreads to process an empty normalized
thread list through the stale-record deletion path instead of returning early,
ensuring prepareDestroyPermanently() is still called when normaliseThread
filters out every server thread lacking root_post_id.
In `@app/products/agents/actions/remote/unreads_summary.ts`:
- Around line 36-38: In the catch blocks of
app/products/agents/actions/remote/unreads_summary.ts:36-38 and
app/products/agents/actions/remote/thread_analysis.ts:34-36, create one message
using getFullErrorMessage(error), pass that message to logError, and return the
same message instead of logging or recomputing from the raw error object.
In `@app/products/agents/actions/websocket/reconnect.ts`:
- Around line 23-25: Update the error log in the reconnect handler around
fetchAgents to wrap agentsResult.error with getFullErrorMessage and prefix the
message with [handleAgentsReconnect].
In `@app/products/agents/components/agent_post/agent_post_new.tsx`:
- Around line 369-380: Ensure handleRegenerate cannot leave isRegenerating true
when regeneration succeeds without a stream-start or stream-end event. Add a
bounded fallback that clears the state, or clear it when the conversation
refetch receives turns for post.id, while preserving the existing error and
normal stream-completion behavior.
In
`@app/products/agents/components/channel_summary_sheet/channel_summary_sheet.tsx`:
- Around line 146-156: Update the agent-loading useEffect around fetchAgents to
clear agents and selectedAgent before each channel load, ignore results from an
outdated or cleaned-up request, and reset loadingAgents in a guarded finally
block even when fetchAgents rejects. Handle returned fetch errors explicitly so
the UI does not treat them as an empty-agent result.
In
`@app/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsx`:
- Around line 121-131: The agent-loading useEffect in
app/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsx:121-131
must guard asynchronous state updates with a cancellation flag and cleanup, and
reset loadingAgents in a finally block; apply the same changes to the
corresponding useEffect in
app/products/agents/components/unreads_summary_sheet/unreads_summary_sheet.tsx:121-131.
Ensure stale fetchAgents responses cannot update agents or loading state after
cleanup, while rejected requests still stop the spinner.
In `@app/products/agents/screens/agent_threads_list/thread_item.tsx`:
- Around line 112-115: Update the FormattedText label in thread_item.tsx to
describe thread.turnCount as turns rather than messages, preserving the
pluralization behavior. Update the corresponding assertions in
app/products/agents/screens/agent_threads_list/thread_item.test.tsx lines 52-65
to expect the corrected turn label.
In `@app/products/agents/turn_content.ts`:
- Around line 165-182: Update openAICitationRegex to use [^\s)]* for the URL
portion so matching stops at the citation’s closing parenthesis. Add a test for
stripCitationClutter where a citation is immediately followed by non-whitespace
text and a later closing parenthesis, preserving that trailing text.
In `@app/screens/channel/header/quick_actions/index.test.tsx`:
- Around line 48-50: Update the afterEach cleanup in the test suite to destroy
the server database created by the setup at line 43, using the existing server
database teardown API and preserving the rewriteStore agent reset.
---
Nitpick comments:
In `@app/products/agents/actions/remote/custom_prompts.ts`:
- Line 23: Update the logError calls in
app/products/agents/actions/remote/custom_prompts.ts at lines 23, 40, and 61 to
use the operation prefixes [fetchCustomPrompts], [fetchCustomPromptPins], and
[renderCustomPrompt], respectively; no other logging behavior needs to change.
In `@app/products/agents/client/rest.ts`:
- Line 103: Update the options destructuring in the affected request method to
use nullish coalescing, replacing the `options || {}` fallback with `options ??
{}` while preserving the existing destructured fields.
In `@app/products/agents/components/agent_post/agent_post_new.tsx`:
- Around line 184-185: Move the isRegeneratingRef.current assignment out of the
render body and into an effect that runs when isRegenerating changes, while
preserving the ref’s current-value behavior for committed renders.
In `@app/products/agents/components/citations_list/index.tsx`:
- Around line 160-166: Replace the inline mapped Pressable callback in the
annotations rendering with a memoized CitationItem child component that receives
the annotation and stable handleCitationPress; have CitationItem invoke the
handler using its own annotation URL while preserving the existing key, styles,
and testID behavior.
In `@app/products/agents/components/question_card/index.tsx`:
- Around line 366-380: The immediate single-select submission in
handleOptionPress lacks double-tap protection. Apply the existing
usePreventDoubleTap mechanism to the onAnswer call in the non-multiSelect path,
while preserving selection updates and multi-select behavior.
In
`@app/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsx`:
- Around line 176-179: Update the error message in the saveSelectedAgent
handling within ThreadAnalysisSheet to include the required ThreadAnalysisSheet
component prefix while preserving the existing error details.
In
`@app/products/agents/components/unreads_summary_sheet/unreads_summary_sheet.tsx`:
- Around line 176-179: Update the error message in the saveSelectedAgent error
path to include the component prefix “UnreadsSummarySheet,” while preserving the
existing failure context and getFullErrorMessage(error) details.
- Around line 35-39: Update the actionItems and openQuestions entries in
defineMessages to use unique agents.unreads_summary.* message IDs instead of
agents.thread_analysis.* IDs, then run npm run i18n-extract to update the
translation catalog.
In `@app/screens/channel/header/quick_actions/index.test.tsx`:
- Around line 93-116: Rename the three tests in the Ask Agents test block to use
names beginning with “should”, including the all-channel-types, DM/GM-channels,
and unlicensed-analysis cases, while preserving their existing test behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2a3293c2-adf9-4346-a3f2-e1e6e1356818
📒 Files selected for processing (100)
app/actions/local/session/index.tsapp/actions/websocket/event.tsapp/components/channel_actions/ask_agents_option/ask_agents_option.tsxapp/components/channel_actions/ask_agents_option/index.tsapp/components/post_draft/draft_input/draft_input.tsxapp/components/post_draft/quick_actions/index.tsapp/components/post_draft/quick_actions/quick_actions.test.tsxapp/components/post_draft/quick_actions/quick_actions.tsxapp/components/post_list/new_message_line/index.tsxapp/components/post_list/post/body/body.tsxapp/components/post_list/post/body/content/content.tsxapp/components/post_list/post/body/content/index.tsxapp/components/post_list/post/post.tsxapp/components/post_list/post_list.tsxapp/constants/events.tsapp/constants/websocket.tsapp/context/server/__mocks__/index.tsxapp/database/migration/server/index.tsapp/database/schema/server/index.tsapp/database/schema/server/test.tsapp/products/agents/actions/remote/agents.test.tsapp/products/agents/actions/remote/agents.tsapp/products/agents/actions/remote/bots.tsapp/products/agents/actions/remote/conversation.tsapp/products/agents/actions/remote/custom_prompts.test.tsapp/products/agents/actions/remote/custom_prompts.tsapp/products/agents/actions/remote/thread_analysis.tsapp/products/agents/actions/remote/threads.test.tsapp/products/agents/actions/remote/threads.tsapp/products/agents/actions/remote/tool_approval.test.tsapp/products/agents/actions/remote/tool_approval.tsapp/products/agents/actions/remote/unreads_summary.tsapp/products/agents/actions/websocket/index.tsapp/products/agents/actions/websocket/reconnect.tsapp/products/agents/client/rest.tsapp/products/agents/components/agent_post/agent_post_legacy.tsxapp/products/agents/components/agent_post/agent_post_new.test.tsxapp/products/agents/components/agent_post/agent_post_new.tsxapp/products/agents/components/ai_rewrite_action/index.tsxapp/products/agents/components/channel_summary_sheet/channel_summary_sheet.tsxapp/products/agents/components/channel_summary_sheet/index.tsapp/products/agents/components/citations_list/index.tsxapp/products/agents/components/new_messages_ask_agents/index.tsapp/products/agents/components/new_messages_ask_agents/new_messages_ask_agents.tsxapp/products/agents/components/question_card/index.tsxapp/products/agents/components/question_card/question_card.test.tsxapp/products/agents/components/reasoning_display/index.tsxapp/products/agents/components/thread_analysis_sheet/index.tsapp/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsxapp/products/agents/components/tool_approval_set/index.tsxapp/products/agents/components/tool_approval_set/tool_approval_set.test.tsxapp/products/agents/components/tool_card/index.tsxapp/products/agents/components/tool_card/tool_card.test.tsxapp/products/agents/components/unreads_summary_sheet/index.tsapp/products/agents/components/unreads_summary_sheet/unreads_summary_sheet.tsxapp/products/agents/constants.tsapp/products/agents/constants/version.tsapp/products/agents/database/models/ai_bot.tsapp/products/agents/database/models/ai_thread.tsapp/products/agents/database/operators/comparators/index.tsapp/products/agents/database/operators/handlers/index.test.tsapp/products/agents/database/operators/transformers/index.tsapp/products/agents/database/queries/bot.tsapp/products/agents/database/schema/ai_bot.tsapp/products/agents/database/schema/ai_thread.tsapp/products/agents/hooks/index.tsapp/products/agents/hooks/use_custom_prompts.tsapp/products/agents/queries/agents.tsapp/products/agents/queries/license.test.tsapp/products/agents/queries/license.tsapp/products/agents/screens/agent_chat/agent_chat.tsxapp/products/agents/screens/agent_chat/agent_chat_content.tsxapp/products/agents/screens/agent_chat/agent_chat_intro.tsxapp/products/agents/screens/agent_selector/agent_item.tsxapp/products/agents/screens/agent_threads_list/agent_threads_list.tsxapp/products/agents/screens/agent_threads_list/thread_item.test.tsxapp/products/agents/screens/agent_threads_list/thread_item.tsxapp/products/agents/screens/rewrite_options/rewrite_options.tsxapp/products/agents/store/agents_config.test.tsapp/products/agents/store/agents_config.tsapp/products/agents/store/custom_prompts_store.tsapp/products/agents/store/index.tsapp/products/agents/turn_content.test.tsapp/products/agents/turn_content.tsapp/products/agents/types/api.tsapp/products/agents/types/conversation.tsapp/products/agents/types/database/models/ai_bot.tsapp/products/agents/types/database/models/ai_thread.tsapp/products/agents/types/index.tsapp/products/agents/utils.test.tsapp/products/agents/utils.tsapp/routes/(bottom_sheet)/agents_rewrite_options.tsxapp/screens/channel/header/quick_actions/index.test.tsxapp/screens/edit_post/edit_post_input/edit_post_input.tsxapp/screens/post_options/index.tsapp/screens/post_options/options/ask_agents_post_option.tsxapp/screens/post_options/post_options.tsxassets/base/i18n/en.jsondocs/database/server/server.mdtest/test_helper.ts
Comment on lines
+25
to
+28
| // Keep the global unsafe-links setting current for the markdown | ||
| // renderers on agent posts and tool cards. | ||
| setAgentsConfig(serverUrl, {allowUnsafeLinks: response.allowUnsafeLinks ?? false}); | ||
|
|
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent stale bot responses from changing the unsafe-link policy.
Line 27 writes allowUnsafeLinks for every completed request. Concurrent fetchAIBots calls can complete out of order. An older response can restore allowUnsafeLinks: true after a newer response disabled it. It can also overwrite the newer bot database state.
Track a per-server refresh generation before the request starts. Discard a response when its generation is no longer current before updating the configuration or database. Add a reverse-completion test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/products/agents/actions/remote/bots.ts` around lines 25 - 28, Update
fetchAIBots to track a per-server refresh generation before each request, then
verify that generation is still current when the response completes. Discard
stale responses before both setAgentsConfig and bot database updates, and add a
test covering concurrent requests that complete in reverse order.
Comment on lines
14
to
+24
| function normaliseThread(raw: RawAIThread): AIThread | null { | ||
| const hasRootPostField = 'root_post_id' in raw; | ||
| const postId = hasRootPostField ? raw.root_post_id : raw.id; | ||
| const postId = raw.root_post_id; | ||
| if (!postId) { | ||
| return null; | ||
| } | ||
|
|
||
| return { | ||
| id: postId, | ||
| message: raw.message ?? '', | ||
| title: raw.title ?? '', | ||
| channel_id: raw.channel_id ?? '', | ||
| reply_count: raw.reply_count ?? 0, | ||
| turn_count: raw.turn_count ?? 0, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 '\bhandleAIThreads\b|\bprepareDestroyPermanently\b' app/products/agents/database app/products/agents/actions/remote/threads.tsRepository: mattermost/mattermost-mobile
Length of output: 16144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
handler = Path("app/products/agents/database/operators/handlers/index.ts").read_text()
remote = Path("app/products/agents/actions/remote/threads.ts").read_text()
required = [
"if (!threads?.length)",
"return [];",
"const incomingIds = new Set(uniqueRaws.map((raw) => raw.id));",
"record.prepareDestroyPermanently()",
]
for text in required:
print(f"{text!r}: {text in handler}")
start = handler.index("handleAIThreads =")
end = handler.index("\n };\n", start) + len("\n };")
print("\nhandleAIThreads implementation:\n" + handler[start:end])
remote_start = remote.index("function normaliseThread")
remote_end = remote.index("\n\n", remote_start)
print("\nnormaliseThread implementation:\n" + remote[remote_start:remote_end])
PYRepository: mattermost/mattermost-mobile
Length of output: 2345
Delete stale threads when normalization returns an empty list.
When all server threads lack root_post_id, normaliseThread produces an empty array, and handleAIThreads exits before calling prepareDestroyPermanently(). Process empty responses through the stale-record deletion path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/products/agents/actions/remote/threads.ts` around lines 14 - 24, Update
handleAIThreads to process an empty normalized thread list through the
stale-record deletion path instead of returning early, ensuring
prepareDestroyPermanently() is still called when normaliseThread filters out
every server thread lacking root_post_id.
Source: Coding guidelines
Comment on lines
+36
to
+38
| } catch (error) { | ||
| logError('[requestUnreadsSummary]', error); | ||
| return {error: getFullErrorMessage(error)}; |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not pass raw error objects to logError.
Both actions pass the caught error directly to logError. Error objects can contain raw request or response metadata. Create one message value with getFullErrorMessage(error), log that value, and return that same value.
app/products/agents/actions/remote/unreads_summary.ts#L36-L38: loggetFullErrorMessage(error)instead oferror.app/products/agents/actions/remote/thread_analysis.ts#L34-L36: loggetFullErrorMessage(error)instead oferror.
As per coding guidelines, wrap errors with getFullErrorMessage(error) before logging them.
📍 Affects 2 files
app/products/agents/actions/remote/unreads_summary.ts#L36-L38(this comment)app/products/agents/actions/remote/thread_analysis.ts#L34-L36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/products/agents/actions/remote/unreads_summary.ts` around lines 36 - 38,
In the catch blocks of
app/products/agents/actions/remote/unreads_summary.ts:36-38 and
app/products/agents/actions/remote/thread_analysis.ts:34-36, create one message
using getFullErrorMessage(error), pass that message to logError, and return the
same message instead of logging or recomputing from the raw error object.
Source: Coding guidelines
Comment on lines
+23
to
+25
| const agentsResult = await fetchAgents(serverUrl); | ||
| if (agentsResult.error) { | ||
| logDebug('Error refreshing agents on reconnect', agentsResult.error); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Normalise and prefix the reconnect failure log.
Line 25 logs an unknown value and omits the handler prefix. Use getFullErrorMessage(agentsResult.error) and a prefix such as [handleAgentsReconnect].
As per coding guidelines, wrap logged errors with getFullErrorMessage(error) and include a function or class prefix in log messages.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/products/agents/actions/websocket/reconnect.ts` around lines 23 - 25,
Update the error log in the reconnect handler around fetchAgents to wrap
agentsResult.error with getFullErrorMessage and prefix the message with
[handleAgentsReconnect].
Source: Coding guidelines
Comment on lines
369
to
380
| const handleRegenerate = useCallback(async () => { | ||
| // Clear the streaming store so the new stream starts from a clean slate | ||
| // instead of showing the previous round's data. | ||
| // instead of showing the previous round's data, and suppress the prior | ||
| // persisted rounds — the server deletes them on regeneration. | ||
| setIsRegenerating(true); | ||
| streamingStore.removePost(serverUrl, post.id); | ||
| const {error} = await regenerateResponse(serverUrl, post.id); | ||
| if (error) { | ||
| setIsRegenerating(false); | ||
| showSnackBar({barType: SNACK_BAR_TYPE.AGENT_REGENERATE_ERROR}); | ||
| } | ||
| }, [serverUrl, post.id]); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
isRegenerating can stay set if no stream starts.
setIsRegenerating(true) is cleared in only two places: the error branch here, and the stream-end refetch at Line 249. If regenerateResponse succeeds but no streaming session for this post ever begins (dropped websocket, missed events, app resumed after the stream ended), isGenerating never transitions from true to false.
Consequences in that state:
persistedRoundsstays suppressed (Line 190), so the post renders no content.- The loading indicator at Line 420 stays visible.
- The self-heal effect returns early on
isRegenerating(Line 296), so it cannot recover.
The post then stays blank until a remount. Add a bounded timeout that clears isRegenerating, or clear it when the conversation refetch delivers turns for this post.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/products/agents/components/agent_post/agent_post_new.tsx` around lines
369 - 380, Ensure handleRegenerate cannot leave isRegenerating true when
regeneration succeeds without a stream-start or stream-end event. Add a bounded
fallback that clears the state, or clear it when the conversation refetch
receives turns for post.id, while preserving the existing error and normal
stream-completion behavior.
Comment on lines
146
to
+156
| useEffect(() => { | ||
| const loadAgents = async () => { | ||
| setLoadingAgents(true); | ||
| const result = await fetchAgents(serverUrl); | ||
| if (result.data && result.data.length > 0) { | ||
| setAgents(result.data); | ||
| if (result.data) { | ||
| setAgents(result.data.filter((agent) => isAgentAvailableInChannel(agent, channelId))); | ||
| } | ||
| setLoadingAgents(false); | ||
| }; | ||
| loadAgents(); | ||
| }, [serverUrl]); | ||
| }, [serverUrl, channelId]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reset and protect agent state during a channel change.
The effect keeps selectedAgent from the previous channel because Line 161 preserves current. A request can then submit an agent that is unavailable in the new channel. An older fetch can also overwrite the newer channel result. If fetchAgents rejects, loadingAgents remains true.
Clear agents and selectedAgent before loading. Ignore results after cleanup. Reset loadingAgents in a guarded finally block. Also represent a returned fetch error instead of showing the no-agent state.
Proposed fix
useEffect(() => {
+ let active = true;
+
const loadAgents = async () => {
setLoadingAgents(true);
- const result = await fetchAgents(serverUrl);
- if (result.data) {
- setAgents(result.data.filter((agent) => isAgentAvailableInChannel(agent, channelId)));
+ setAgents([]);
+ setSelectedAgent(null);
+ try {
+ const result = await fetchAgents(serverUrl);
+ if (active && result.data) {
+ setAgents(result.data.filter((agent) => isAgentAvailableInChannel(agent, channelId)));
+ }
+ } finally {
+ if (active) {
+ setLoadingAgents(false);
+ }
}
- setLoadingAgents(false);
};
- loadAgents();
+
+ void loadAgents();
+ return () => {
+ active = false;
+ };
}, [serverUrl, channelId]);🧰 Tools
🪛 React Doctor (0.9.3)
[error] 146-146: This setter runs after await, so overlapping re-runs of the effect can resolve out of order and write stale state; gate it behind a cancellation/ignore flag or return a cleanup that cancels the work.
In a useEffect whose dependencies can change, guard any setter call that runs after an await behind a cancellation/ignore flag, or return a cleanup that cancels the async work.
(no-set-state-after-await-in-effect)
[error] 153-153: This resets a loading/busy flag only on the success path: if the awaited call rejects the reset never runs and the flag stays stuck truthy (a spinner that never stops, a button disabled forever). Move the reset into a finally block, or mirror it on every catch, so it clears on rejection too.
A trailing setLoading(false) after an await never runs if the awaited call rejects, so the flag stays stuck truthy; reset it in a finally block (or mirror the reset on every catch) so it clears on both paths.
(no-loading-flag-reset-outside-finally)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/products/agents/components/channel_summary_sheet/channel_summary_sheet.tsx`
around lines 146 - 156, Update the agent-loading useEffect around fetchAgents to
clear agents and selectedAgent before each channel load, ignore results from an
outdated or cleaned-up request, and reset loadingAgents in a guarded finally
block even when fetchAgents rejects. Handle returned fetch errors explicitly so
the UI does not treat them as an empty-agent result.
Source: Linters/SAST tools
Comment on lines
+121
to
+131
| useEffect(() => { | ||
| const loadAgents = async () => { | ||
| setLoadingAgents(true); | ||
| const result = await fetchAgents(serverUrl); | ||
| if (result.data) { | ||
| setAgents(result.data.filter((agent) => isAgentAvailableInChannel(agent, channelId))); | ||
| } | ||
| setLoadingAgents(false); | ||
| }; | ||
| loadAgents(); | ||
| }, [serverUrl, channelId]); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unguarded agent-loading effect duplicated in both analysis sheets. The same agent-loading block was copied into both sheets. It writes state after await without a cancellation flag, and it resets loadingAgents outside a finally block. A stale response can overwrite agents after channelId changes, and a rejected fetchAgents leaves the spinner running forever.
app/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsx#L121-L131: add acancelledflag with a cleanup function, and movesetLoadingAgents(false)into afinallyblock.app/products/agents/components/unreads_summary_sheet/unreads_summary_sheet.tsx#L121-L131: apply the same cancellation flag andfinallyreset.
Both sheets share almost their entire implementation. Consider extracting a useChannelAgents(serverUrl, channelId, selectedAgentId) hook that returns agents, selectedAgent, loadingAgents, and selectAgent. That removes this class of duplicated defect.
🧰 Tools
🪛 React Doctor (0.9.3)
[error] 121-121: This setter runs after await, so overlapping re-runs of the effect can resolve out of order and write stale state; gate it behind a cancellation/ignore flag or return a cleanup that cancels the work.
In a useEffect whose dependencies can change, guard any setter call that runs after an await behind a cancellation/ignore flag, or return a cleanup that cancels the async work.
(no-set-state-after-await-in-effect)
[error] 128-128: This resets a loading/busy flag only on the success path: if the awaited call rejects the reset never runs and the flag stays stuck truthy (a spinner that never stops, a button disabled forever). Move the reset into a finally block, or mirror it on every catch, so it clears on rejection too.
A trailing setLoading(false) after an await never runs if the awaited call rejects, so the flag stays stuck truthy; reset it in a finally block (or mirror the reset on every catch) so it clears on both paths.
(no-loading-flag-reset-outside-finally)
📍 Affects 2 files
app/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsx#L121-L131(this comment)app/products/agents/components/unreads_summary_sheet/unreads_summary_sheet.tsx#L121-L131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsx`
around lines 121 - 131, The agent-loading useEffect in
app/products/agents/components/thread_analysis_sheet/thread_analysis_sheet.tsx:121-131
must guard asynchronous state updates with a cancellation flag and cleanup, and
reset loadingAgents in a finally block; apply the same changes to the
corresponding useEffect in
app/products/agents/components/unreads_summary_sheet/unreads_summary_sheet.tsx:121-131.
Ensure stale fetchAgents responses cannot update agents or loading state after
cleanup, while rejected requests still stop the spinner.
Source: Linters/SAST tools
Comment on lines
112
to
+115
| <FormattedText | ||
| id='agents.threads_list.reply_count' | ||
| defaultMessage='{count, plural, one {# reply} other {# replies}}' | ||
| values={{count: thread.replyCount}} | ||
| id='agents.threads_list.message_count' | ||
| defaultMessage='{count, plural, one {# message} other {# messages}}' | ||
| values={{count: thread.turnCount}} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Show turnCount as turns, not messages.
AiThreadModel.turnCount represents user-agent conversation turns. One turn can include both a user message and an agent message. The current label reports an inaccurate message count.
app/products/agents/screens/agent_threads_list/thread_item.tsx#L112-L115: Change the localized label to “turn” or use an actual message-count field.app/products/agents/screens/agent_threads_list/thread_item.test.tsx#L52-L65: Update the assertions to match the corrected turn label.
📍 Affects 2 files
app/products/agents/screens/agent_threads_list/thread_item.tsx#L112-L115(this comment)app/products/agents/screens/agent_threads_list/thread_item.test.tsx#L52-L65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/products/agents/screens/agent_threads_list/thread_item.tsx` around lines
112 - 115, Update the FormattedText label in thread_item.tsx to describe
thread.turnCount as turns rather than messages, preserving the pluralization
behavior. Update the corresponding assertions in
app/products/agents/screens/agent_threads_list/thread_item.test.tsx lines 52-65
to expect the corrected turn label.
Comment on lines
+165
to
+182
| // Matches the inline "(source: https://…)" clutter some OpenAI models emit | ||
| // alongside structured citations. Mirrors the webapp's openAICitationRegex. | ||
| const openAICitationRegex = /\([^\s:]+\s*:\s*https?:\/\/[\S^)]*\)/g; | ||
|
|
||
| /** | ||
| * Strip inline OpenAI citation clutter from agent response text. The | ||
| * structured citations render as the Sources list, so the inline "(source: | ||
| * https://…)" duplicates are noise. No-op when the pattern is absent. | ||
| */ | ||
| export function stripCitationClutter(text: string): string { | ||
| if (!openAICitationRegex.test(text)) { | ||
| // Reset lastIndex so the global regex starts fresh on the next call. | ||
| openAICitationRegex.lastIndex = 0; | ||
| return text; | ||
| } | ||
| openAICitationRegex.lastIndex = 0; | ||
| return text.replace(openAICitationRegex, '').replace(/\s+\./g, '.'); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the character class in openAICitationRegex.
The class [\S^)] is a union of \S, ^, and ). The ) is inside the class, so it does not terminate the URL part. The URL part therefore matches any run of non-whitespace characters, including ), and the engine backtracks to the last ) in that run.
Result: for input like see (src: https://x)more) the match extends to the final ) and removes text that is not part of the citation. Use [^\s)]* so the URL stops before the closing parenthesis.
Add a test for a citation that is immediately followed by more non-whitespace text and a later ).
🐛 Proposed fix
-const openAICitationRegex = /\([^\s:]+\s*:\s*https?:\/\/[\S^)]*\)/g;
+const openAICitationRegex = /\([^\s:]+\s*:\s*https?:\/\/[^\s)]*\)/g;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Matches the inline "(source: https://…)" clutter some OpenAI models emit | |
| // alongside structured citations. Mirrors the webapp's openAICitationRegex. | |
| const openAICitationRegex = /\([^\s:]+\s*:\s*https?:\/\/[\S^)]*\)/g; | |
| /** | |
| * Strip inline OpenAI citation clutter from agent response text. The | |
| * structured citations render as the Sources list, so the inline "(source: | |
| * https://…)" duplicates are noise. No-op when the pattern is absent. | |
| */ | |
| export function stripCitationClutter(text: string): string { | |
| if (!openAICitationRegex.test(text)) { | |
| // Reset lastIndex so the global regex starts fresh on the next call. | |
| openAICitationRegex.lastIndex = 0; | |
| return text; | |
| } | |
| openAICitationRegex.lastIndex = 0; | |
| return text.replace(openAICitationRegex, '').replace(/\s+\./g, '.'); | |
| } | |
| // Matches the inline "(source: https://…)" clutter some OpenAI models emit | |
| // alongside structured citations. Mirrors the webapp's openAICitationRegex. | |
| const openAICitationRegex = /\([^\s:]+\s*:\s*https?:\/\/[^\s)]*\)/g; | |
| /** | |
| * Strip inline OpenAI citation clutter from agent response text. The | |
| * structured citations render as the Sources list, so the inline "(source: | |
| * https://…)" duplicates are noise. No-op when the pattern is absent. | |
| */ | |
| export function stripCitationClutter(text: string): string { | |
| if (!openAICitationRegex.test(text)) { | |
| // Reset lastIndex so the global regex starts fresh on the next call. | |
| openAICitationRegex.lastIndex = 0; | |
| return text; | |
| } | |
| openAICitationRegex.lastIndex = 0; | |
| return text.replace(openAICitationRegex, '').replace(/\s+\./g, '.'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/products/agents/turn_content.ts` around lines 165 - 182, Update
openAICitationRegex to use [^\s)]* for the URL portion so matching stops at the
citation’s closing parenthesis. Add a test for stripCitationClutter where a
citation is immediately followed by non-whitespace text and a later closing
parenthesis, preserving that trailing text.
Comment on lines
+48
to
+50
| afterEach(() => { | ||
| rewriteStore.setAgents(serverUrl, []); | ||
| }); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Destroy the server database after each test.
Line 43 creates a server database. Line 48 does not destroy it. This can retain database state and resources between tests.
Proposed fix
-afterEach(() => {
+afterEach(async () => {
rewriteStore.setAgents(serverUrl, []);
+ await DatabaseManager.destroyServerDatabase(serverUrl);
});As per coding guidelines, destroy server databases in afterEach.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| afterEach(() => { | |
| rewriteStore.setAgents(serverUrl, []); | |
| }); | |
| afterEach(async () => { | |
| rewriteStore.setAgents(serverUrl, []); | |
| await DatabaseManager.destroyServerDatabase(serverUrl); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/screens/channel/header/quick_actions/index.test.tsx` around lines 48 -
50, Update the afterEach cleanup in the test suite to destroy the server
database created by the setup at line 43, using the existing server database
teardown API and preserving the rewriteStore agent reset.
Source: Coding guidelines
This was referenced Aug 8, 2026
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
3 participants
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
Catches the mobile Agents integration up to the
mattermost-plugin-agents2.x contract across 12 workstreams (12 commits, one per workstream/sub-item):/ai_bots, and adds theis_default/turn_countcolumns (server schema v21 + migration).mcp_bare_name,user_interaction,would_auto_execute, anddecided_atthrough the tool-approval UI (auto-run badge, mixed-batch hiding, no re-prompt after decisions) and persists web-search citation annotations into the Sources list.QuestionCardsupports single-select (immediate submit), multi-select, free-form input, and skip, wired through the newtool_answersfield on the approval endpoint.AgentPostnow renders insideBody, so reactions, acknowledgements, and permalink previews work on agent posts.allowUnsafeLinkssetting; regenerate no longer flashes the old answer; long reasoning is scrollable in-place; a loading placeholder covers the conversation-fetch gap. Also hardens the stream-end conversation refetch with a bounded retry ladder (server-side turn persistence can lag the stream-end signal, previously leaving a blank post until remount).turn_counts, and the agent list refreshes live onbots_invalidatewebsocket events and reconnect.viewedAt) instead of a nonexistentunreads_onlyflag, all requests carryteam_id, agents are filtered by channel scope, entry points are gated on the enterprise license (mirroring web'sisEnterpriseLicensedOrDevelopment), and the sheet gained an empty state and a "Results are only visible to you" footer.@mentionprepend outside agent DMs. Prompts live in a new ephemeral per-server store; authoring/pin management stays on the webapp./channel/{id}/intervalwithend_time: 0), with the same license/agent gating; the separator itself stays non-tappable.All new entry points share the same rules: the agent picker appears only when more than one channel-scoped agent is usable, and the saved agent preference is respected.
Ticket Link
None (internal catch-up plan; each commit message documents its workstream).
Checklist
E2E/Run(orE2E/Run-iOS/E2E/Run-Androidfor platform-specific runs).Device Information
This PR was tested on: iPhone 17 Pro simulator (iOS 26), against a local Mattermost server (source build) running mattermost-ai plugin 2.5.0 with Ollama (qwen3:8b). Every workstream was exercised end-to-end on device, including negative cases (license gate hides affordances, channel-scoped agents filtered, private prompts invisible).
Screenshots
Custom prompts (WS11) — pinned prompt pills on the agent chat intro; the "Custom prompts" entry and panel in the composer AI-actions sheet; a rendered prompt placed into the draft with the agent @mention prepend:
Pinned custom prompt pills on the agent chat intro Composer AI actions sheet with Custom prompts entry Custom prompts panel listing visible prompts Rendered prompt in the draft with @matty prepend
Thread analysis (WS10) — the "Ask Agents" row in the post long-press menu and the three-type analysis sheet:
Ask Agents row in the post options menu Thread analysis sheet with three analysis types
Catch me up (WS14) — the "Ask Agents" pill on the New Messages separator and the unreads summary sheet:
Ask Agents pill on the New Messages separator Unreads summary sheet with three presets
Results in the agent DM — analysis results with collapsible thinking blocks:
Agent DM showing unreads summary and action items results
Release Note
To show artifacts inline, enable in settings.