Skip to content

added plugin logs in mcp logs - #5740

Closed
Madhuvod wants to merge 1 commit into
07-20-feat_mcp-guardrails_ui_changesfrom
07-23-added_plugin_logs_in_mcp_logs
Closed

Madhuvod wants to merge 1 commit into
07-20-feat_mcp-guardrails_ui_changesfrom
07-23-added_plugin_logs_in_mcp_logs

Conversation

@Madhuvod

Copy link
Copy Markdown
Contributor

Revives #5489 on the restored MCP guardrails stack. This PR is stacked above #5739; the stack bottom targets dev.

## Summary

Extends MCP tool log entries to capture and persist plugin execution logs, making them visible in the MCP log detail sheet alongside the existing execution data.

## Changes

- Added a `plugin_logs` column (`text`) to the `mcp_tool_logs` table via a new idempotent migration (`mcp_tool_logs_add_plugin_logs_column`).
- Added `PluginLogs` field to the `MCPToolLog` struct and included its byte length in the memory size estimator for the batch writer.
- Extracted a `serializePluginLogs` helper in the logging plugin to avoid duplicating the group-and-marshal logic; called it in both `Inject` (LLM logs) and `PostMCPHook` (MCP tool logs) so plugin logs accumulated during a request are serialized and attached before the entry is enqueued.
- Added `plugin_logs` to the `MCPToolLogEntry` TypeScript type.
- Replaced the flat execution detail layout in the MCP log detail sheet with a two-tab layout: **Execution** (arguments, result, metadata, error details) and **Plugin Logs** (rendered via the existing `PluginLogsView` component). A badge on the Plugin Logs tab shows the total log entry count.
- Added a `formatPluginName` utility in `PluginLogsView` to display plugin names in title case (e.g., `guardrails` → `Guardrails`, `my_plugin` → `My Plugin`).
- Added and updated tests covering the migration idempotency, round-trip serialization of `PluginLogs`, `PrepareMCPToolDBEntry` field preservation, and end-to-end persistence through `PostMCPHook`.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

```sh
# Core/Transports
go test ./framework/logstore/... ./plugins/logging/...

# UI
cd ui
pnpm i
pnpm build
```

To validate end-to-end:
1. Configure a plugin (e.g., guardrails) that emits plugin logs during an MCP tool call.
2. Invoke an MCP tool through Bifrost.
3. Open the MCP Logs view in the UI, select the log entry, and verify the **Plugin Logs** tab shows the expected entries grouped by plugin name with a correct count badge.
4. Re-run the migration against an existing database to confirm idempotency (no error, no data loss).

## Screenshots/Recordings

_Add before/after screenshots of the MCP log detail sheet showing the new tabbed layout._

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

Plugin logs may contain redacted argument values or intermediate processing details. The `plugin_logs` column is subject to the same content-logging enable/disable controls as arguments and results. No new PII surface is introduced beyond what is already captured in existing plugin log flows.

## Checklist

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

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • MCP tool logs now capture and retain plugin execution logs.
    • Added a dedicated Plugin Logs tab to MCP log details.
    • Plugin names are displayed in a more readable, title-cased format.
    • Execution details are organized separately from plugin logs for easier review.
  • Bug Fixes

    • Improved the accessibility of the log detail reveal control.
    • Preserved existing log entries during storage updates and migrations.

Walkthrough

MCP tool logs now persist serialized plugin logs, preserve them through payload and database operations, estimate their batch size, and display them in a dedicated UI tab with formatted plugin names.

Changes

MCP plugin log lifecycle

Layer / File(s) Summary
Plugin log persistence contract
framework/logstore/tables.go, framework/logstore/migrations.go, framework/logstore/migrations_test.go, framework/logstore/payload_test.go
MCPToolLog now stores plugin_logs. The migration adds the column idempotently. Payload and migration tests verify persistence and row preservation.
Plugin log capture and write path
plugins/logging/main.go, plugins/logging/writer.go, plugins/logging/operations_test.go
Plugin logs are serialized from request context and attached to completed MCP tool logs. Batch-size estimation includes the serialized field. Persistence tests validate the stored JSON.
Plugin log display
ui/lib/types/logs.ts, ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx, ui/app/workspace/logs/views/pluginLogsView.tsx, ui/app/workspace/logs/sheets/logDetailView.tsx
The MCP details sheet adds Execution and Plugin Logs tabs. It counts and renders plugin logs, formats plugin names, and links the reveal switch to its label.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PostMCPHook
  participant serializePluginLogs
  participant mcp_tool_logs
  participant MCPLogDetailsSheet
  participant PluginLogsView
  PostMCPHook->>serializePluginLogs: collect and group plugin logs
  serializePluginLogs->>mcp_tool_logs: persist plugin_logs JSON
  MCPLogDetailsSheet->>mcp_tool_logs: read MCP tool log
  MCPLogDetailsSheet->>PluginLogsView: render plugin log entries
Loading

Possibly related PRs

Suggested reviewers: pratham-mishra04, akshaydeo

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description only references related issues and stack context; it omits the required summary, changes, testing, affected areas, and checklist details. Complete the repository template with the purpose, changes, type, affected areas, test steps, UI screenshots, breaking-change status, security considerations, and checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding plugin logs to MCP logs.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 07-23-added_plugin_logs_in_mcp_logs

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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 `@framework/logstore/migrations.go`:
- Around line 2873-2875: Update the error wrapping in the Migrate call to use
the %w verb with the original err value instead of formatting err.Error(),
preserving the underlying error chain for errors.Is and errors.As while
retaining the existing context message.

In `@plugins/logging/main.go`:
- Line 1919: Update the assignment to entry.PluginLogs using the content policy
from p.resolveContentPolicy(ctx): only assign
serializePluginLogs(ctx.GetPluginLogs()) when the policy is visible. Preserve
MCPToolLog.PluginLogs persistence and JSON visibility through both MCP log
endpoints.

In `@ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx`:
- Around line 429-436: Update the plugin logs rendering in the `TabsContent` for
`plugins` to normalize `displayLog.plugin_logs` into a status distinguishing
valid log arrays, valid empty data, and malformed or unusable values. Render the
existing “No plugin logs” empty state only for valid empty data, and add a clear
error state for invalid nonempty strings or objects lacking log arrays; pass
only validated data to `PluginLogsView`.
- Around line 351-361: Add stable data-testid attributes to both TabsTrigger
elements in the tab section, using unique values that follow the existing
“mcplogdetails-*” naming pattern for the execution and plugins tabs. Preserve
all existing labels, classes, and plugin log count behavior.
🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 16577b18-4b87-44cd-8c99-fcf9658d1438

📥 Commits

Reviewing files that changed from the base of the PR and between baab059 and c4164c8.

📒 Files selected for processing (11)
  • framework/logstore/migrations.go
  • framework/logstore/migrations_test.go
  • framework/logstore/payload_test.go
  • framework/logstore/tables.go
  • plugins/logging/main.go
  • plugins/logging/operations_test.go
  • plugins/logging/writer.go
  • ui/app/workspace/logs/sheets/logDetailView.tsx
  • ui/app/workspace/logs/views/pluginLogsView.tsx
  • ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx
  • ui/lib/types/logs.ts

Comment on lines +2873 to +2875
if err := m.Migrate(); err != nil {
return fmt.Errorf("error while adding MCP plugin logs column: %s", err.Error())
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n 'error while adding MCP plugin logs column' framework/logstore/migrations.go

Repository: maximhq/bifrost

Length of output: 240


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- migration context ---'
sed -n '2825,2890p' framework/logstore/migrations.go
printf '%s\n' '--- migration symbol and callers ---'
rg -n -C 3 'MCP plugin logs|MigrateMCP|mcp.*logs|MCP.*Logs' framework/logstore --glob '*.go'
printf '%s\n' '--- Go module files ---'
find . -name go.mod -print

Repository: maximhq/bifrost

Length of output: 50371


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- migration registration and related error handling ---'
sed -n '205,305p' framework/logstore/migrations.go
sed -n '1068,1088p' framework/logstore/migrations.go
printf '%s\n' '--- exact string-formatting pattern in migration errors ---'
rg -n 'fmt\.Errorf\(.*err\.Error\(\)' framework/logstore/migrations.go
printf '%s\n' '--- repository stack state ---'
git status --short --branch
git log --oneline -5
printf '%s\n' '--- module declarations ---'
find . -name go.mod -print -exec sed -n '1,12p' {} \;

Repository: maximhq/bifrost

Length of output: 29352


Preserve the migration error chain.

Use %w instead of %s with err.Error() so callers can inspect the underlying migration, database, or context error with errors.Is and errors.As.

🤖 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 `@framework/logstore/migrations.go` around lines 2873 - 2875, Update the error
wrapping in the Migrate call to use the %w verb with the original err value
instead of formatting err.Error(), preserving the underlying error chain for
errors.Is and errors.As while retaining the existing context message.

Source: Coding guidelines

Comment thread plugins/logging/main.go
callback := p.mcpToolLogCallback
p.mu.Unlock()
attachMCPLogRedactionData(ctx, entry, p.contentLoggingEnabled(ctx))
entry.PluginLogs = serializePluginLogs(ctx.GetPluginLogs())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C 4 'GetPluginLogs|PluginLogEntry|contentLoggingEnabled|DisableContentLogging|\.Log\(' plugins core framework

Repository: maximhq/bifrost

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- logging implementation ---'
rg -n -C 12 'entry\.PluginLogs|serializePluginLogs|func \(.*\) contentLoggingEnabled|func \(.*\) resolveContentPolicy|PluginLogEntry|GetPluginLogs' plugins/logging --glob '*.go'

printf '%s\n' '--- context log implementations ---'
rg -n -C 10 'func .*Log\(|PluginLogs|WithPluginScope|ReleasePluginScope' --glob '*.go' core framework plugins | head -n 1200

printf '%s\n' '--- plugin_logs response/API exposure ---'
rg -n -C 8 'plugin_logs|PluginLogs' --glob '*.go' --glob '*.sql' --glob '*.json' .

Repository: maximhq/bifrost

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- exact update path ---'
sed -n '1820,1970p' plugins/logging/main.go

printf '%s\n' '--- policy helpers and nearby tests ---'
rg -n -C 20 'contentLoggingEnabled|resolveContentPolicy|ContentPolicy|DisableContentLogging' plugins/logging/main.go plugins/logging/*.go

printf '%s\n' '--- log entry schema and serializer ---'
rg -n -C 15 'type PluginLogEntry|func serializePluginLogs|serializePluginLogs\(' --glob '*.go' .

Repository: maximhq/bifrost

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import pathlib, re

roots = [pathlib.Path("plugins"), pathlib.Path("core"), pathlib.Path("framework")]
patterns = [
    re.compile(r'\.Log\s*\('),
    re.compile(r'PluginLogEntry'),
    re.compile(r'GetPluginLogs'),
]
for root in roots:
    for path in root.rglob("*.go"):
        try:
            text = path.read_text()
        except Exception:
            continue
        hits = []
        for i, line in enumerate(text.splitlines(), 1):
            if any(p.search(line) for p in patterns):
                hits.append((i, line.strip()))
        if hits:
            print(f"\n## {path}")
            for i, line in hits:
                print(f"{i}: {line}")
PY

Repository: maximhq/bifrost

Length of output: 24365


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- disabled-content tests and plugin-log tests ---'
rg -n -C 12 'PluginLogs|plugin logs|content.*log|log.*content|DisableContentLogging' plugins/logging/*_test.go

printf '%s\n' '--- all production plugin log writes ---'
rg -n -C 8 '\.Log\s*\(' plugins --glob '*.go' --glob '!**/*_test.go'

Repository: maximhq/bifrost

Length of output: 49224


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- repository stack metadata ---'
git status --short
git branch --all --no-color | head -n 100
find . -maxdepth 3 -type f \( -iname '*stack*' -o -name 'stack.yaml' -o -name 'stack.yml' \) -print

Repository: maximhq/bifrost

Length of output: 319


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- PluginLogEntry and context behavior ---'
sed -n '440,475p' core/schemas/bifrost.go
sed -n '730,815p' core/schemas/context.go

printf '%s\n' '--- MCP log schema, serialization, and hidden-content handling ---'
rg -n -C 12 'type MCPToolLog|PluginLogs.*string|ContentHidden|content_hidden|SerializeFields|DeserializeFields' framework/logstore core --glob '*.go'

printf '%s\n' '--- MCP log retrieval/API paths ---'
rg -n -C 10 'GetMCPToolLog|SearchMCPToolLogs|MCPToolLog' --glob '*.go' --glob '!**/*_test.go' . | head -n 1600

Repository: maximhq/bifrost

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- PostMCPHook tests around content policy ---'
sed -n '1030,1155p' plugins/logging/operations_test.go
rg -n -C 15 'PostMCPHook|MCPToolLog|PluginLogs' plugins/logging/*_test.go | grep -E '^[^:]+:[0-9]+:|content|disabled|PluginLogs|PostMCPHook|MCPToolLog' | head -n 1200

printf '%s\n' '--- all production ctx.Log producers, excluding test files ---'
rg -n -C 5 '\.Log\s*\(' plugins --glob '*.go' --glob '!**/*_test.go'

Repository: maximhq/bifrost

Length of output: 15064


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

path = Path("plugins/logging/main.go")
text = path.read_text()
start = text.index("func (p *LoggerPlugin) PostMCPHook")
end = text.index("\n}", start) + 2
body = text[start:end]

assignment = "entry.PluginLogs = serializePluginLogs(ctx.GetPluginLogs())"
print("post_mcp_hook_contains_unconditional_plugin_log_assignment:",
      assignment in body)
print("assignment_count_in_post_mcp_hook:", body.count(assignment))
print("assignment_context:")
for line_no, line in enumerate(body.splitlines(), 1):
    if "PluginLogs" in line or "contentLoggingEnabled" in line or "resolveContentPolicy" in line:
        print(f"{line_no}: {line}")

policy = text[text.index("type contentPolicy"):text.index("// applyMCPGovernanceFieldsToEntry")]
print("content_logging_enabled_returns_store_content:",
      "return p.resolveContentPolicy(ctx).storeContent" in text)
print("disabled_without_object_storage_returns_empty_policy:",
      "return contentPolicy{}" in policy)
PY

Repository: maximhq/bifrost

Length of output: 699


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- exact logstore definitions ---'
rg -l 'type MCPToolLog|ContentHidden|PluginLogs' framework/logstore core | sort
for f in $(rg -l 'type MCPToolLog|ContentHidden|PluginLogs' framework/logstore core | sort | head -n 20); do
  echo "## $f"
  rg -n -C 20 'type MCPToolLog|PluginLogs|ContentHidden|content_hidden|SerializeFields|DeserializeFields' "$f"
done

Repository: maximhq/bifrost

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- API route and response serialization references ---'
rg -n -C 12 'mcp.*log|MCP.*log|plugin_logs|PluginLogs' . \
  --glob '*.go' \
  --glob '!**/*_test.go' \
  --glob '!plugins/logging/main.go' \
  --glob '!framework/logstore/*.go' | head -n 1800

Repository: maximhq/bifrost

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- MCP payload preparation and serialization ---'
sed -n '1057,1135p' framework/logstore/tables.go
sed -n '385,435p' framework/logstore/payload.go

printf '%s\n' '--- MCP handler response paths ---'
sed -n '2555,2665p' transports/bifrost-http/handlers/logging.go
rg -n -A 45 -B 8 'func \(h \*LoggingHandler\) getMCPLogByID|func \(h \*LoggingHandler\) getMCPLogs' transports/bifrost-http/handlers/logging.go

Repository: maximhq/bifrost

Length of output: 18069


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

tables = Path("framework/logstore/tables.go").read_text()
payload = Path("framework/logstore/payload.go").read_text()
handler = Path("transports/bifrost-http/handlers/logging.go").read_text()

mcp_start = tables.index("type MCPToolLog struct")
mcp_end = tables.index("\n}", mcp_start) + 2
mcp = tables[mcp_start:mcp_end]

prep_start = payload.index("func PrepareMCPToolDBEntry")
prep_end = payload.index("\n}", prep_start) + 2
prep = payload[prep_start:prep_end]

print("mcp_plugin_logs_has_json_field:", '`json:"plugin_logs,omitempty"`' in mcp)
print("mcp_has_content_hidden_field:", "ContentHidden" in mcp)
print("prepare_mcp_clears_plugin_logs:", "PluginLogs =" in prep or "PluginLogs:" in prep)
print("handler_serializes_mcp_search_result:", "SendJSON(ctx, result)" in handler)
print("handler_serializes_mcp_detail_log:", "SendJSON(ctx, log)" in handler)
print("mcp_detail_route_present:", '"/api/mcp-logs/{id}"' in handler)
PY

Repository: maximhq/bifrost

Length of output: 379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- exact MCP handler methods ---'
rg -n 'func \(h \*LoggingHandler\) (getMCPLogs|getMCPLogByID)' transports/bifrost-http/handlers/logging.go
for n in $(rg -n 'func \(h \*LoggingHandler\) (getMCPLogs|getMCPLogByID)' transports/bifrost-http/handlers/logging.go | cut -d: -f1); do
  start=$((n-2)); end=$((n+100))
  sed -n "${start},${end}p" transports/bifrost-http/handlers/logging.go
done

Repository: maximhq/bifrost

Length of output: 15591


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- MCP content-hidden regression coverage ---'
sed -n '1,240p' framework/logstore/contenthidden_test.go
rg -n -C 12 'PluginLogs|plugin_logs|ContentHidden|content_hidden' framework/logstore/*_test.go plugins/logging/*_test.go transports/bifrost-http/handlers/*_test.go

Repository: maximhq/bifrost

Length of output: 50371


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

tables = Path("framework/logstore/tables.go").read_text()
payload = Path("framework/logstore/payload.go").read_text()
handler = Path("transports/bifrost-http/handlers/logging.go").read_text()

mcp_start = tables.index("type MCPToolLog struct")
mcp_end = tables.index("\n}", mcp_start) + 2
mcp = tables[mcp_start:mcp_end]

prep_start = payload.index("func PrepareMCPToolDBEntry")
prep_end = payload.index("\n}", prep_start) + 2
prep = payload[prep_start:prep_end]

print("mcp_plugin_logs_json_visible:", 'json:"plugin_logs,omitempty"' in mcp)
print("mcp_has_content_hidden_field:", "ContentHidden" in mcp)
print("prepare_mcp_clears_plugin_logs:", "PluginLogs" in prep)
print("handler_serializes_mcp_search_result:", "SendJSON(ctx, result)" in handler)
print("handler_serializes_mcp_detail_log:", "SendJSON(ctx, log)" in handler)
print("mcp_detail_route_present:", '"/api/mcp-logs/{id}"' in handler)
PY

Repository: maximhq/bifrost

Length of output: 376


Apply the visible-content policy to MCP plugin logs.

When content is disabled or hidden, assign PluginLogs only if p.resolveContentPolicy(ctx).visible() is true. MCPToolLog.PluginLogs remains persisted and JSON-visible through both MCP log endpoints.

🤖 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 `@plugins/logging/main.go` at line 1919, Update the assignment to
entry.PluginLogs using the content policy from p.resolveContentPolicy(ctx): only
assign serializePluginLogs(ctx.GetPluginLogs()) when the policy is visible.
Preserve MCPToolLog.PluginLogs persistence and JSON visibility through both MCP
log endpoints.

Sources: Coding guidelines, Path instructions

Comment on lines +351 to +361
<TabsTrigger value="execution" className="px-3">
Execution
</TabsTrigger>
<TabsTrigger value="plugins" className="px-3">
Plugin Logs
{pluginLogCount > 0 ? (
<span className="bg-background text-muted-foreground ml-1.5 rounded-sm border px-2 py-0.5 text-[10px] tabular-nums">
{pluginLogCount}
</span>
) : null}
</TabsTrigger>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add stable test IDs to both tab triggers.

The new tab triggers are interactive elements. Add data-testid values that follow the existing mcplogdetails-* pattern.

Proposed change
- <TabsTrigger value="execution" className="px-3">
+ <TabsTrigger value="execution" className="px-3" data-testid="mcplogdetails-execution-tab">
    Execution
  </TabsTrigger>
- <TabsTrigger value="plugins" className="px-3">
+ <TabsTrigger value="plugins" className="px-3" data-testid="mcplogdetails-plugin-logs-tab">

As per coding guidelines, “Add data-testid attributes to new interactive elements and preserve existing values because E2E tests depend on them.”

📝 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
<TabsTrigger value="execution" className="px-3">
Execution
</TabsTrigger>
<TabsTrigger value="plugins" className="px-3">
Plugin Logs
{pluginLogCount > 0 ? (
<span className="bg-background text-muted-foreground ml-1.5 rounded-sm border px-2 py-0.5 text-[10px] tabular-nums">
{pluginLogCount}
</span>
) : null}
</TabsTrigger>
<TabsTrigger
value="execution"
className="px-3"
data-testid="mcplogdetails-execution-tab"
>
Execution
</TabsTrigger>
<TabsTrigger
value="plugins"
className="px-3"
data-testid="mcplogdetails-plugin-logs-tab"
>
Plugin Logs
{pluginLogCount > 0 ? (
<span className="bg-background text-muted-foreground ml-1.5 rounded-sm border px-2 py-0.5 text-[10px] tabular-nums">
{pluginLogCount}
</span>
) : null}
</TabsTrigger>
🤖 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 `@ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx` around lines 351 -
361, Add stable data-testid attributes to both TabsTrigger elements in the tab
section, using unique values that follow the existing “mcplogdetails-*” naming
pattern for the execution and plugins tabs. Preserve all existing labels,
classes, and plugin log count behavior.

Source: Coding guidelines

Comment on lines +429 to +436
<TabsContent value="plugins" className="space-y-3">
{displayLog.plugin_logs ? (
<PluginLogsView pluginLogs={displayLog.plugin_logs} />
) : (
<div className="text-muted-foreground rounded-sm border border-dashed p-5 text-center text-sm">
No plugin logs for this request.
</div>
)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Render an empty or error state for unusable plugin logs.

When plugin_logs is a nonempty malformed string, or an object without log arrays, this branch renders PluginLogsView. PluginLogsView then returns null, so the tab is blank.

Parse the value into a status that distinguishes valid empty data from invalid data. Render “No plugin logs” for valid empty data and an error message for invalid data.

As per coding guidelines, “For ui/**, check interactive workflows for loading, empty, error, and success states.”

🤖 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 `@ui/app/workspace/mcp-logs/views/mcpLogDetailsSheet.tsx` around lines 429 -
436, Update the plugin logs rendering in the `TabsContent` for `plugins` to
normalize `displayLog.plugin_logs` into a status distinguishing valid log
arrays, valid empty data, and malformed or unusable values. Render the existing
“No plugin logs” empty state only for valid empty data, and add a clear error
state for invalid nonempty strings or objects lacking log arrays; pass only
validated data to `PluginLogsView`.

Source: Coding guidelines

@Madhuvod Madhuvod closed this Jul 31, 2026
@Madhuvod
Madhuvod deleted the 07-23-added_plugin_logs_in_mcp_logs branch July 31, 2026 11:43
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