Merge master into bridge-service-independence - #1021
Draft
crspeller wants to merge 7 commits into
Draft
Conversation
* webapp: pass rootId to the Agents RHS composer Mattermost 11.11 syncs AdvancedTextEditor drafts during render. Omitting rootId made that check loop forever and crash the RHS on open. Co-authored-by: Cursor <cursoragent@cursor.com> * webapp: use single quotes for the RHS composer rootId prop Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…iews, path-identical metadata (#881) * Phase 1 server: carry tool Title/Description on both paths, drop ToolCall.Schema Capture MCP title/annotations (sanitized) in mcp GetTools, resolve effective display title (title > annotations.title). Add Title/Description to ToolCall, ContentBlock, and both block writers; keep them visible to non-requesters on both the live (redactToolCalls) and persisted (FilterForNonRequester) paths so a call renders identically live and after reload. Remove the unused ToolCall.Schema from the wire (providers build tool defs from Tool.Schema). Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 1 webapp: plumb tool title/mcp_bare_name/server_origin; add tool_identity Map server_origin, mcp_bare_name, title, description on persisted rounds (toolUseBlockToToolCall). Add tool_identity helpers (originKind, canonicalToolKey, toolDisplayName) and use toolDisplayName in ToolCard, preferring the MCP-supplied title and ellipsizing long titles; built-in and embedded display names are unchanged. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 1 tests: drift-guard parity, GetTools title/annotations, tool_identity Add streaming/tool_call_parity_test.go: a per-field policy table over llm.ToolCall enforced by reflection (exhaustiveness), plus writer-completeness and live-vs-persisted redaction-parity tests (verified to catch the historical Description drop-by-omission). Add mcp GetTools title/annotation mapping + Unicode-sanitization test; conversation SanitizeForDisplay + FilterForNonRequester title/description coverage; ContentBlock title/description round-trip; TS tool_identity.test.ts and turn_content_utils new-field mappings. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 1: fix no-undefined lint in tool_identity.test.ts Split the undefined-origin case out of the test.each table into its own test so ESLint's no-undefined rule passes. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Rich tool call rendering (2/3): generic arguments field list + view raw Replace the pretty-printed JSON blob in ToolCard with a labeled, readable field list driven purely by the arguments object (insertion key order, so the layout is identical on the live and persisted paths). Values render by JS type: strings as plain wrapped text, numbers/booleans/primitive-arrays as compact value pills, nested objects/arrays as a small inline JSON block. All values render as plain text via styled-components, never through formatText/markdown. A single card-level Show more expands all clamped long values, and a required View raw toggle reveals the exact pretty-printed JSON payload so the approval surface always lets the user inspect what they are approving. Empty-object args keep the verbatim No parameters required message; null/redacted args render nothing. Value pills never carry an icon/avatar (reserved for Phase 3 entity chips). Header, status, buttons, result rendering, and the result-review callout are untouched. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 3: extract ToolCardShell and host View raw on the shell Extract the shared approval chrome (header, collapse, decision buttons, result section, result-review callout) into tool_renderers/tool_card_shell.tsx, and move the View raw affordance onto the shell so rich cards inherit it. ToolCard becomes a thin shell + ToolArguments field-list body. ToolArguments keeps the field list + Show more; its raw view is exposed via ToolArgumentsRaw. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 3: renderer registry + rich cards + entity chips Add tool_renderers/registry.tsx routing each tool call to a rich card or QuestionCard (migrated as the proving entry), falling back to the generic ToolCard; matching is on canonical identity (origin kind + bare name) plus a strict parse. Add entity_chips.tsx (ChannelChip/UserChip: resolve via store or API, icon/avatar on success, plain-text raw id/username on failure). Add rich cards for create_post, dm, group_message, search_posts, search_users, read_post, get_channel_info with strict parse-or-fallback. tool_approval_set routes through the registry. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 3 tests: parsers, entity chips, registry routing, view-raw inheritance Extract pure parsers into rich_card_parsers.ts (testable without React deps). Add rich_card_parsers.parsers.test (valid/malformed/redacted -> null), entity chip tests (store + API resolution, failure degradation to plain text), and registry routing tests (QuestionCard, embedded rich cards, external/unknown/ malformed fall back to generic; rich cards expose View raw with the exact payload). Migrate tool_approval_set.test to mock the registry. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 3 e2e: rich create_post card spec (mock LLM) + shard assignment Add tests/tool-config/mock-api/rich-create-post-card.spec.ts asserting the rich create_post card renders the resolved channel and message body and that View raw exposes the exact payload; assign it to the lightest e2e shard (shard-1). Mirrors the existing mock-api tool-config specs. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 1: restore webapp/package-lock.json to master (no dependency change) The lockfile was inadvertently rewritten by a local npm install (npm dropped "peer" markers); no webapp dependency actually changed. Restore master's lockfile so the CI lockfile-drift check passes. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 2: restore webapp/package-lock.json to master (no dependency change) Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 3: restore webapp/package-lock.json to master (no dependency change) Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 1: table-driven parity test for the toolUseBlocks writer Completes the drift-guard's writer-completeness check for the conversation-side auto-run writer (toolUseBlocks is unexported, so streaming/tool_call_parity_test cannot reach it): assert toolUseBlocks and PostToBlocks emit every persisted tool_use identity/metadata field for a fully-populated call. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 1: table-driven parity test for the toolUseBlocks writer (stacked) Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Phase 1: table-driven parity test for the toolUseBlocks writer (stacked) Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Trim verbose comments; remove phase references Comments now target the future reader concisely; no development-phase narrative. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Simplify from review: single-parse registry, drop dead code Registry matchers now match on identity only — the cards already fall back to the generic ToolCard when their strict parse fails, so arguments were parsed twice per render. Replace the field-by-field toRichProps copy with rest destructuring. Remove unused canonicalToolKey, the EmbeddedServerOrigin export, and the speculative channel-icon branch. Use the UserInteractionSelect constant instead of a string literal. Rename the parser test to match its source file. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Address PR review feedback - Treat whitespace-only MCP titles as absent so cards never render a blank header (Codex); covered in the GetTools test, now table-driven (CodeRabbit). - Localize the remaining hardcoded pill labels (filter names, ID, Name, Team): parsers emit stable keys, labels render via FormattedMessage. - Make the card header toggle keyboard-accessible (role/tabIndex/Enter/Space, aria-expanded) and expose ellipsized titles via a hover tooltip. - Expire cached entity-chip failures after 30s so transient fetch errors recover without a reload; note that omitted destructiveHint means destructive per the MCP spec. - Parity drift-guards report every drifted field via per-field subtests instead of stopping at the first failure. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Restyle tool cards to match the card designs Bordered rounded card container with a larger header: chevron, tool name, and a muted plain-text context (target channel, recipients, or query) supplied by the rich cards — the name element keeps its exact text. Arguments render as a two-column grid (bold labels left, values right) in both the generic field list and the rich cards. Accept is now the filled primary action. All existing behavior is unchanged: entity chips, value pills, Show more, View raw, result section, review callout, statuses, and redaction. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * e2e: disambiguate channel-name assertions on the rich create_post card The redesigned header shows the channel context and the chip shows 'name · team'; assert each specifically instead of a substring that now matches both. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Align card styling with Mattermost conventions Channel chips use the globe/lock icons like the channel UI (no '#', which is not a Mattermost convention); the card container matches QuestionCard's border radius and shadow. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * ci: retrigger after unrelated container-startup flake in e2e-shard-2 Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Generic response rendering, post preview card, prune future-use code Tool results now use the same generic treatment as arguments: JSON-object results render as the labeled field list, everything else as clamped plain text — replacing the markdown code-block pipeline (and its formatText dependency) in the card shell. Replace the seven thin per-tool cards with one that adds real value: read_post renders a permalink-style preview of the referenced post (via PostPreview / PostMessagePreview), falling back to the generic card when the arguments don't parse or the post can't be fetched. The registry keeps QuestionCard routing. Prune code that had no consumer: MCP annotation safety-hint capture, the header-context slot, entity chips, filter-label i18n strings, and getProfilesByUsernames. Swap the create_post e2e spec for a read_post preview spec. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * e2e: allow seeded content to match both the post preview and the response The read_post card now shows a permalink-style preview, so the referenced post's text legitimately appears twice on the expanded card. The image-count and image-request assertions are unchanged and now also cover the preview. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Scope the read_post preview to undecided calls The permalink-style preview renders the post with Mattermost's normal markdown, which is redundant once the call has executed (the response shows the content) and would sit next to the deliberately-unrendered result text. Executed calls render the generic card, restoring the original guarantees of the unsafe-post result spec, which is reverted to its strict assertions. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * create_post: preview the post-to-be before approval While a create_post call awaits approval, render its message as a permalink-style post preview (authored by the requesting user, whose session the embedded tool executes with) — the same rendering the message gets once posted — so the user sees exactly what they are approving. Executed or malformed calls render the generic card. Extend the preview e2e spec to cover both cards with a shared container. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Make the create_post preview non-interactive The previewed post does not exist yet, so its permalink/avatar/username affordances cannot lead anywhere; suppress pointer events on the preview. The read_post preview stays interactive (its post is real). Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Fix toolUseBlocks call after merging master server-tool support The master merge added a serverTools argument to toolUseBlocks; pass nil from the writer-parity test so the package still compiles. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Address review feedback: split post preview cards per tool, drop redundant ?? undefined --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
* Add MCP service account headers config and agent service account auth flag Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Add service account auth mode to MCP connection layer Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Thread service account catalog selection and attribution through LLM context Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Add service account auth UI to system console and agent settings Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Verify bridge behavior for service account agents and reject tool_hooks Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Document service account authentication for MCP servers Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Trim redundant comments across service account feature Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Simplify service account diff: drop SA hooks accounting and redundant tests Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Address CodeRabbit review: trim SA headers, sweep activity map, preserve header row order, doc fixes Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Require system admin to enable agent service account auth; address bot review findings Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Canonicalize service account header names so case-only duplicates fail closed Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Require system admin to save agents with service account auth enabled Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Refuse cross-origin redirects and origin-gate MCP credential injection Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Retrigger CI after runner infrastructure cancelled all jobs Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * Allow agent managers to edit non-sensitive fields while SA auth is on Replace the all-or-nothing system-admin save lock with a sensitive-field ACL so Access, MCP grants, and provider changes stay admin-only while custom instructions, model, and other day-to-day settings remain editable. Co-authored-by: Nick Misasi <nick13misasi@gmail.com> * Refactor SA field ACL to fail closed and cover real UI locks Apply-then-compare BotConfig diffs with a manager-editable allowlist, move policy into agent_acl.go, require tab lock props, and add real Config/Access tab tests so soft-locks cannot drift unnoticed. Co-authored-by: Nick Misasi <nick13misasi@gmail.com> * fix: satisfy eslint lines-around-comment in config_tab test Co-authored-by: Nick Misasi <nick13misasi@gmail.com> * Allow managers to edit AI service, tools, and dynamic loading on SA agents Widen the manager-editable allowlist while service account auth stays on so Access and MCP grants remain admin-only without freezing day-to-day config. Co-authored-by: Nick Misasi <nick13misasi@gmail.com> * docs: clarify SA soft-lock comment for manager-editable config Co-authored-by: Nick Misasi <nick13misasi@gmail.com> * fix: sync i18n catalog and isolate svc-2 seeding in SA tests Regenerate en.json for the updated SA soft-lock banner, and avoid polluting the shared agent test fixture with an extra AI service. Co-authored-by: Nick Misasi <nick13misasi@gmail.com> * Show the service-account MCP catalog when configuring SA agents. The MCPs tab was always loading the editing user's connections, so servers that only have service-account headers looked disconnected even when the agent could use them. Also clarify the SA header editor so the header name is not repeated in the value. Co-authored-by: Cursor <cursoragent@cursor.com> * Mark SA-only MCP servers as unavailable in user mode. Servers that only have service-account headers were still listed with an ON toggle for non-SA agents. Show them as Unavailable with a disabled off toggle, without persisting that off, on both the MCPs tab and the RHS Tools menu. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: unblock webapp lint and rhs_header tests Type the mcpServerStatus table so tsc accepts test.each, and mock the host webapp's react-bootstrap in rhs_header tests. Co-authored-by: Nick Misasi <nick13misasi@gmail.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com>
…ccount mode (#1012) * mcp: run embedded and plugin MCP servers as the invoking user in service account mode Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * mcp: address quality review - single catalog boundary, typed CatalogRequest, honest server kind Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * mcp: simplify catalog request and pool plumbing per review Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * mcp: convert catalog request and bag kind tests to table-driven Co-authored-by: nick.misasi <nick.misasi@mattermost.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
…#967) * Capture sandbox output file ids on server tool activity Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Track observed sandbox file ids on the request context Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Add provider file download through Bifrost's Files API Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Add AttachSandboxFile tool: model-chosen attachment of sandbox output files Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * docs: describe sandbox file attachment behavior Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Attach sandbox output files automatically instead of via a model-chosen tool * webapp: start a new round when provider tool activity follows text * fix: address sandbox file attachment review feedback * chore: drop leftover AttachSandboxFile comments and span/godoc nits Move ServerToolUse.Sanitize godoc onto the method, end sandbox download/upload spans with defer, and stop referring to the removed AttachSandboxFile tool in capture-path comments. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * chore: trim restating comments on sandbox file attachment Keep comments that capture invariants a future reader would miss (arrival order vs grouping, $OUTPUT_DIR as the share gesture, ProviderServices vs type-asserting LanguageModel, OpenAI sandbox vs file retrieval). Drop comments that only restate the code. Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * fix: apply code-review and go-simplify feedback on sandbox file attachment * fix: update tool-use parity test after merge Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * build: update mattermost-govet for Go 1.26 Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * build: exclude npm sources from Go license checks Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
* Bump Go to 1.27.0 across all modules * Modernize with go fix and gofmt for Go 1.27 * Drop go-multierror in favor of stdlib errors * Use stdlib uuid in config migrations * Replace pkg/errors with stdlib error wrapping * Move test-only LLM helpers to llm/llmtest * Remove dead code and obsolete pointer helpers * Make config Container accessors nil-safe consistently * Extract stream draining from toolrunner runLoop * Dedupe timestamp and header helpers in format package * Table-drive eval providers and dedupe evalviewer * Make i18n Bundle a type alias * Simplify pluginctl and loadtest controller helpers * Dedupe store JSON and squirrel query helpers * Dedupe streaming post creation and event senders * Dedupe indexer job failure and chunking/search helpers * Remove unused search options and dedupe pgvector helpers * Dedupe MCP session adoption and plugin transport * Dedupe MCP server auth providers * Simplify pluginmcp lazy handler init * Dedupe LLM bridge completion handlers * Dedupe agent, search, and admin API handlers * Generalize cluster event publishing * Extract shared loadtest profile package * Embed LanguageModel in wrappers and collapse context delegation * Dedupe bifrost streaming helpers and split bifrost.go by concern * Embed Store in conversation Service and export UnmarshalBlocks * Dedupe bots lookup and reconcile helpers * Dedupe conversations tool loop and handler preambles * Replace error-string matching with exported sentinel errors * Drop redundant AgentStore compile-time assertion * Bump golangci-lint to v2.13.1 and fix lint findings * Update AGENTS.md: Go bump procedure and corrected gotchas * Generic MCP tool registration and mcpserver dedup * Flatten embedded struct literal via go fix * Clear remaining staticcheck findings * Fix type-switch case order in controller hook helper * Document FIPS image bump; 1.27 image not yet published * Point FIPS build at newly published 1.27.0-dev image * Remove accidentally committed evalviewer binary * Hold Go at 1.26.7 until a 1.27 FIPS toolchain image exists * Pin FIPS image digest resolved by CI * Modernize code merged from master * conversation: unexport Service store and delegate explicitly * config: never store a nil configuration in Container
Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
🤖 LLM Evaluation ResultsOpenAI
❌ Failed EvaluationsShow 7 failuresOPENAI1. TestReactEval/[openai]_react_cat_message
2. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
3. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
4. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
5. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
6. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
7. TestDirectMessageConversations/[openai]_bot_dm_tool_introspection
Anthropic
❌ Failed EvaluationsShow 7 failuresANTHROPIC1. TestReactEval/[anthropic]_react_cat_message
2. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
3. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
4. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
5. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
6. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
7. TestDirectMessageConversations/[anthropic]_bot_dm_tool_introspection
This comment was automatically generated by the eval CI pipeline. |
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
Merges
origin/master(through "Modernize and simplify the Go codebase" #995) intocursor/bridge-service-independence-06d6and resolves the conflicts following the new patterns from the modernization PR:api/api_llm_bridge.go: kept master'sllmResponder/bridgeCompletionPlandedupe, but with the branch's model-based signature (model llm.LanguageModel, maxToolTurns int) since service completions have no agent.handleServiceCompletionnow wrapsprepareServiceBridgeCompletionand the leaserelease(). Dropped the stalesortimport (master moved toslices.SortFunc).bots/bots.go: master'sgetLLMnow returns*llm.ProviderServices(sandbox file attachment, Attach Anthropic sandbox-created files to agent replies automatically #967) while the branch'sbuildLLMreturns ashutdownhandle for the service LLM registry. Introduced a smallbuiltLLMstruct carryingmodel,providerServices, andshutdownso both are preserved; updatedservice_llm_registry.goand tests.llm/structured_output_fallback.go,llm/token_tracking.go: switched the wrapper structs to master's embeddedLanguageModelpattern while keeping the branch'snativeAlloweddecision andTokenUsageIdentity.api/api_agents.go: adopted master'sAgentRequestFields/applyTodedupe, preserving the branch's deprecation note and//nolint:staticcheckonStructuredOutputEnabled.config/config_test.go: kept the newGetServicestests, droppedboolPtr/stringPtr(master usesnew(v)).WithModel(...)(removed on master) andmap[string]interface{}in branch tests; added//nolint:staticcheckwhere master's newer golangci-lint (v2.13.1) now flags the deprecatedStructuredOutputEnabledfield in tests/store.structuredOutputEnabledfrom master's newAgentDrafttest fixtures (the branch moved structured output to a per-service policy). Docs updated accordingly.Verified:
make check-stylepasses,go testpasses for all packages that don't require Docker, webapptsc/jestpass,make check-i18nreports no drift.Ticket Link
N/A
Release Note