Skip to content

fixed mcp logs app icon - #7167

Merged
akshaydeo merged 7 commits into
devfrom
09-15-fixed_mcp_logs_app_icon
Sep 15, 2026
Merged

akshaydeo merged 7 commits into
devfrom
09-15-fixed_mcp_logs_app_icon

Conversation

@akshaydeo

@akshaydeo akshaydeo commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Increases the app icon size in the MCP logs table to improve visibility and prevent icons from shrinking when space is constrained.

Changes

  • Increased app icon dimensions from 14×14 to 20×20 in the MCP logs columns view
  • Added shrink-0 class to prevent the icon from being compressed in flex layouts

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

Navigate to the MCP logs view in the workspace and verify that app icons appear larger and do not shrink when the column is narrow.

cd ui
pnpm i || npm i
pnpm build || npm run build

Screenshots/Recordings

Before/after screenshots of the MCP logs table showing the app icon column at both sizes would confirm the change.

Breaking changes

  • Yes
  • No

Related issues

Security considerations

None.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 98197937-7076-4dd5-b4c7-a5720d787927

📥 Commits

Reviewing files that changed from the base of the PR and between 7907a3e and 6fd110e.

📒 Files selected for processing (1)
  • ui/app/workspace/mcp-logs/views/columns.tsx

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Style
    • Increased the app icon size in the MCP logs table from 14×14 to 20×20 pixels for improved visibility.

Walkthrough

The MCP logs table now renders app icons at 20×20 pixels instead of 14×14 pixels. Existing lazy loading, async decoding, and accessible alt text remain unchanged.

Changes

MCP log icon sizing

Layer / File(s) Summary
Update app icon dimensions
ui/app/workspace/mcp-logs/views/columns.tsx
The MCP log app icon size changes from 14×14 to 20×20 pixels. Other image attributes remain unchanged.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: impoiler

Merge Risk: ⚪ Minimal · up to ace36

The MCP log app icons will render larger without an identified impact on functionality or availability.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Issue #123 requires File API support for provider file uploads, including workflows such as fine-tuning and RAG, through an endpoint such as POST /v1/files. The reviewed PR summary shows only a chan… Implement the File API behavior required by #123 and add automated tests for file upload and the supported workflows. Verify the implementation in the reviewed PR changes.
Out of Scope Changes check ⚠️ Warning The PR changes the MCP logs app icon from 14×14 pixels to 20×20 pixels. This UI adjustment has no demonstrated connection to the File API requirements in issue #123. The available evidence contains no… Remove the unrelated MCP logs app icon change, or link it to a separate applicable issue and submit it separately from the File API work.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Title check ✅ Passed The title identifies the MCP logs app icon change and matches the pull request scope. It could specify the size adjustment, but it remains concise and understandable.
Description check ✅ Passed The description explains the purpose, implementation, affected UI area, testing steps, and lack of breaking or security changes. Screenshots are not provided, and checklist items remain unchecked, but…
Full details: Linked Issues check

Explanation

Issue #123 requires File API support for provider file uploads, including workflows such as fine-tuning and RAG, through an endpoint such as POST /v1/files. The reviewed PR summary shows only a change to the MCP logs app icon size in ui/app/workspace/mcp-logs/views/columns.tsx. It provides no File API implementation or automated tests. A repository diff read failed because the required object was unavailable, but the available PR evidence does not show the required coding changes.

Full details: Out of Scope Changes check

Explanation

The PR changes the MCP logs app icon from 14×14 pixels to 20×20 pixels. This UI adjustment has no demonstrated connection to the File API requirements in issue #123. The available evidence contains no related File API objective that would justify the change.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 09-15-fixed_mcp_logs_app_icon

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 14, 2026
@akshaydeo
akshaydeo force-pushed the fix/bedrock-invoke-tool-search-replay branch from 7907a3e to 67e6a96 Compare September 15, 2026 07:23
@akshaydeo
akshaydeo force-pushed the 09-15-fixed_mcp_logs_app_icon branch 2 times, most recently from 4ef9e75 to 8e76535 Compare September 15, 2026 08:21
@akshaydeo
akshaydeo force-pushed the fix/bedrock-invoke-tool-search-replay branch from 67e6a96 to 7b90b4f Compare September 15, 2026 08:21

akshaydeo commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor Author

Merge activity

Anthropic's tool_search_tool_result blocks were lost on the way back, on
every provider, in three separate ways.

1. AnthropicContentBlock.ToolReferences is declared flat, but Anthropic
   nests tool_references inside a tool_search_tool_search_result "content"
   object. AnthropicContent.UnmarshalJSON's single-object fallback parked
   that object in Content.ContentBlocks, one level below where every reader
   looked, so live traffic always read an empty list. The existing tests
   missed it because they build the block as a Go struct and set the flat
   field directly, which real traffic never does. DiscoveredToolReferences
   reads both shapes, flat first, so Bifrost's own rebuild in
   convertBifrostToolSearchCallToAnthropicBlocks keeps working.

2. The non-streaming converter's server_tool_use dispatch handled
   web_search, web_fetch, advisor and code_execution only, so a tool-search
   server_tool_use and its paired result block were both dropped and the
   caller saw no tool_search_call at all. Only the streaming state machine
   ever emitted one.

3. The grouped (replay) converter accumulated the tool-search
   server_tool_use with the client tool_use run, turning it into a
   function_call. A caller replaying that returns a tool_result for a
   srvtoolu_ id, which the API rejects outright. Anthropic requires the
   server_tool_use and tool_search_tool_result blocks to be echoed back
   unchanged, so the pair now round-trips through the neutral layer.

The tool_search_call item is emitted for request messages too, not just
output - unlike the web_search sibling - because replay depends on it.

Ref: https://platform.claude.com/docs/en/agents-and-tools/tool-use/tool-search-tool
A Claude request carrying a tool_search_tool_* server tool and per-tool
defer_loading was served eagerly over Converse with HTTP 200 and no error
when it arrived on POST /bedrock/model/{modelId}/invoke. The same request
on /anthropic/v1/messages routed to InvokeModel as #6908 intended.

The ingress must convert the InvokeModel-shaped body into the Converse-shaped
internal request before anything else runs, and convertAnthropicTools dropped
both signals there: tool_search_tool_* was skipped outright and BedrockToolSpec
had no defer_loading field. The egress predicate that picks InvokeModel reads
the neutral request, and that same conversion is what builds it - so the
predicate was being asked to detect a feature whose every trace had already
been erased upstream. It could never fire on this ingress, which is why the
fix belongs here and not in the predicate.

Both signals now ride across on json:"-" carriers. Converse has no wire slot
for either, and BedrockConverseRequest doubles as the egress type, so keeping
them off the JSON entirely leaves real Converse bodies byte-identical.

Also stop manufacturing a cachePoint for a deferred tool: Anthropic returns a
400 for defer_loading together with cache_control, so the invoke ingress must
not synthesise the combination out of a cache_control the client did attach.

toolSearchVariantName is exported as schemas.ToolSearchVariantName so the
rebuild here resolves regex vs bm25 through the canonical rule instead of a
second copy of it.

Ref: https://platform.claude.com/docs/en/agents-and-tools/tool-use/tool-search-tool
Fixes #7155
…ress

Once tool search reaches Claude on the Bedrock-native invoke ingress, the
response direction misreported it. toBedrockInvokeAnthropicResponse branches
on item.ResponsesToolMessage != nil, and a tool_search_call sets that field,
so the server-side search came back as an ordinary tool_use carrying the
srvtoolu_ id.

That is the one thing Anthropic tells callers never to do: "Never return a
tool_result for its srvtoolu_... ID." A client seeing a tool_use does exactly
that, and the API rejects the following turn. The paired
tool_search_tool_result was dropped entirely, so the discovered tool
references never reached the caller either.

Emit the documented pair instead - server_tool_use plus
tool_search_tool_result with its nested tool_search_tool_search_result
payload - ahead of the generic tool-message branch.
BedrockInvokeMessagesContentBlock gains tool_use_id and a typed content
object; typed rather than a map so the keys marshal in a stable order.

stop_reason is unaffected: it scans for blocks of type tool_use, and neither
new type matches, so the discovered tool's own call still drives it.

Ref: https://platform.claude.com/docs/en/agents-and-tools/tool-use/tool-search-tool
The streaming twin of the previous commit. toAnthropicInvokeStreamBytes opens
a content_block_start of type tool_use for any output_item.added carrying a
tool message, and a tool_search_call carries one - so a streamed server-side
search arrived at the caller as an invocable tool holding the srvtoolu_ id.
The caller then executes it and returns a tool_result for that id, which
Anthropic rejects on the next turn.

Open a server_tool_use block instead, which callers correctly leave alone.

The converter also remembers the content index each item opened its
content_block_start at, so output_item.done closes that same block. The neutral
stream collapses Anthropic's server_tool_use and tool_search_tool_result into one
tool_search_call item, so the done event carries the result block's index: copying
it through opened block N and closed N+1, leaving N open for the rest of the
stream. The per-stream map lives on the request context as a pointer, the same
bookkeeping the Anthropic egress already keeps in blockIndexFor/allocBlockIndex.

The paired tool_search_tool_result block is still deliberately not re-emitted
here; the non-streaming path emits the full pair. Dropping it no longer leaves an
unclosed block behind.

Ref: https://platform.claude.com/docs/en/agents-and-tools/tool-use/tool-search-tool
Anthropic requires the client to echo the assistant's server_tool_use and
tool_search_tool_result back unchanged on the next turn: "On the next request,
pass the assistant's content back unchanged." BedrockContentBlock.UnmarshalJSON
decoded only image, tool_use, tool_result and thinking, so both blocks hit no
case, fell through to an empty struct, and vanished.

The model was then shown a turn in which it called a tool it had never
discovered - the same silent-drop shape as the request-side bug in #7155, in
the other direction.

Decode both onto json:"-" carriers and rebuild the neutral tool_search_call
from the pair, matching on server_tool_use.id == result.tool_use_id via a
pre-scan, the way nova_code_interpreter results are already paired. The
references are read from the nested content object, with the flat spelling
accepted as a fallback, mirroring DiscoveredToolReferences on the Anthropic
side.

Only tool search is decoded here. Every other Anthropic server tool is either
Converse-representable or unsupported on this ingress, and reshaping one would
be a behaviour change well beyond this issue.

Ref: https://platform.claude.com/docs/en/agents-and-tools/tool-use/tool-search-tool
@akshaydeo
akshaydeo force-pushed the fix/bedrock-invoke-tool-search-replay branch from 7b90b4f to 7ca45eb Compare September 15, 2026 08:36
@akshaydeo
akshaydeo force-pushed the 09-15-fixed_mcp_logs_app_icon branch from 8e76535 to ace36d0 Compare September 15, 2026 08:36
@akshaydeo
akshaydeo changed the base branch from fix/bedrock-invoke-tool-search-replay to graphite-base/7167 September 15, 2026 08:50
@akshaydeo
akshaydeo changed the base branch from graphite-base/7167 to dev September 15, 2026 08:52
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review September 15, 2026 08:52

The base branch was changed.

@akshaydeo
akshaydeo merged commit 4d7cc80 into dev Sep 15, 2026
11 checks passed
@akshaydeo
akshaydeo deleted the 09-15-fixed_mcp_logs_app_icon branch September 15, 2026 08:53
@akshaydeo akshaydeo mentioned this pull request Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant