diff --git a/litellm-rust/crates/traces-clickhouse/query/span_detail.sql b/litellm-rust/crates/traces-clickhouse/query/span_detail.sql index daecf048811..af26edeb17c 100644 --- a/litellm-rust/crates/traces-clickhouse/query/span_detail.sql +++ b/litellm-rust/crates/traces-clickhouse/query/span_detail.sql @@ -34,13 +34,21 @@ outputs AS ( AND JSONType(result, 'content') = 'String' GROUP BY TeamId, ApiKeyHash, session_id, call_id ), -executions AS ( +invocations AS ( SELECT TeamId, ApiKeyHash, SpanAttributes['session.id'] AS session_id, groupUniqArray(coalesce(nullIf(SpanAttributes['gen_ai.tool.call.id'], ''), SpanAttributes['tool_use_id'])) AS call_ids FROM scoped WHERE ObservationType = 'tool' AND Framework IN ('claude-code', 'claude-agent-sdk') GROUP BY TeamId, ApiKeyHash, session_id ), +execution_spans AS ( + SELECT TeamId, ApiKeyHash, SpanAttributes['session.id'] AS session_id, + ParentSpanId AS tool_span_id, min(SpanId) AS execution_span_id + FROM scoped + WHERE Framework IN ('claude-code', 'claude-agent-sdk') + AND (SpanAttributes['span.type'] = 'tool.execution' OR SpanName = 'claude_code.tool.execution') + GROUP BY TeamId, ApiKeyHash, session_id, tool_span_id +), answers AS ( SELECT TeamId, ApiKeyHash, ParentSpanId AS parent_span_id, argMax(Output, Timestamp) AS output, argMax(SpanId, Timestamp) AS source_span @@ -58,6 +66,8 @@ SELECT o.SpanId AS span_id, concat('[', arrayStringConcat(arrayFilter(result -> JSONExtractString(result, 'id') != '' AND NOT has(e.call_ids, JSONExtractString(result, 'id')), JSONExtractArrayRaw(o.Output, 'tool_results')), ','), ']'), '[]'), 'lens.content.input_source', multiIf(o.native_tool AND length(i.values) = 1, i.source_span, o.Input != '', o.SpanId, ''), + 'lens.content.execution_status', if(o.native_tool AND o.SpanAttributes['span.type'] = 'tool', + if(x.execution_span_id != '', 'recorded', 'not_recorded'), ''), 'lens.content.output_source', multiIf(o.Output != '', o.SpanId, o.native_tool AND length(r.values) = 1, r.source_span, o.ObservationType = 'agent' AND a.output != '', a.source_span, ''), 'lens.content.input_status', multiIf(o.native_tool AND length(i.values) > 1, 'conflicting', input != '', 'recorded', 'not_recorded'), 'lens.content.output_status', multiIf(o.Output != '', 'recorded', o.native_tool AND length(r.values) > 1, 'conflicting', o.native_tool AND length(r.values) = 1, 'recorded', output != '', 'recorded', 'not_recorded'), @@ -66,7 +76,8 @@ SELECT o.SpanId AS span_id, FROM selected AS o LEFT JOIN inputs AS i ON o.native_tool AND o.TeamId = i.TeamId AND o.ApiKeyHash = i.ApiKeyHash AND o.session_id = i.session_id AND o.call_id = i.call_id LEFT JOIN outputs AS r ON o.native_tool AND o.TeamId = r.TeamId AND o.ApiKeyHash = r.ApiKeyHash AND o.session_id = r.session_id AND o.call_id = r.call_id -LEFT JOIN executions AS e ON e.TeamId = o.TeamId AND e.ApiKeyHash = o.ApiKeyHash AND e.session_id = o.session_id +LEFT JOIN invocations AS e ON e.TeamId = o.TeamId AND e.ApiKeyHash = o.ApiKeyHash AND e.session_id = o.session_id +LEFT JOIN execution_spans AS x ON o.native_tool AND x.TeamId = o.TeamId AND x.ApiKeyHash = o.ApiKeyHash AND x.session_id = o.session_id AND x.tool_span_id = o.SpanId LEFT JOIN answers AS a ON a.parent_span_id = o.SpanId AND a.TeamId = o.TeamId AND a.ApiKeyHash = o.ApiKeyHash ORDER BY indexOf(requested_ids, o.SpanId) LIMIT length(requested_ids) diff --git a/litellm-rust/crates/traces-clickhouse/tests/reads.rs b/litellm-rust/crates/traces-clickhouse/tests/reads.rs index bafdbaacaf9..c4474a52eb1 100644 --- a/litellm-rust/crates/traces-clickhouse/tests/reads.rs +++ b/litellm-rust/crates/traces-clickhouse/tests/reads.rs @@ -844,12 +844,14 @@ async fn native_cost_correlation_survives_session_grouping_and_excludes_other_ow } #[rstest] -#[case::recorded(false)] -#[case::conflicting(true)] +#[case::recorded(false, true)] +#[case::conflicting(true, true)] +#[case::invocation_without_execution(false, false)] #[tokio::test] async fn native_tool_content_agrees_between_single_and_bulk_reads( #[future(awt)] migrated_database: TestResult, #[case] conflict: bool, + #[case] executed: bool, ) -> TestResult { use litellm_storage_clickhouse::fetch; use litellm_traces::query::named::{SpanDetailParams, SpanDetailsParams}; @@ -960,6 +962,7 @@ async fn native_tool_content_agrees_between_single_and_bulk_reads( DATABASE, InsertTable::OtelTraces, rows.into_iter() + .chain(executed.then_some(("execution", "team-a", "key-a", "s", "claude_code.tool.execution", "framework", "tool", "", ""))) .map( |(id, team, key, session, name, kind, parent, input, output)| { BTreeMap::from([ @@ -974,7 +977,7 @@ async fn native_tool_content_agrees_between_single_and_bulk_reads( ("Output".into(), json!(output)), ( "SpanAttributes".into(), - json!({"session.id":session,"tool_use_id":"call"}), + json!({"session.id":session,"tool_use_id":"call", "span.type": if kind == "tool" { "tool" } else { "" }}), ), ("TeamId".into(), json!(team)), ("ApiKeyHash".into(), json!(key)), @@ -1027,6 +1030,10 @@ async fn native_tool_content_agrees_between_single_and_bulk_reads( assert_eq!(bulk.len(), 3); assert_eq!(bulk[0].input, r#"{"command":"exit 7"}"#); assert_eq!(bulk[0].attributes["lens.content.input_source"], "input"); + assert_eq!( + bulk[0].attributes["lens.content.execution_status"], + if executed { "recorded" } else { "not_recorded" } + ); assert_eq!(bulk[0].output, if conflict { "" } else { "Exit code 7" }); assert_eq!( bulk[0].attributes["lens.content.output_status"], diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index d02d1f29ded..a61f293ec1c 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -537,29 +537,36 @@ class LiteLLMRoutes(enum.Enum): ######################################################### passthrough_routes_wildcard = [f"{route}/*" for route in mapped_pass_through_routes] - litellm_native_routes = [ - "/rag/ingest", - "/v1/rag/ingest", - "/rag/query", - "/v1/rag/query", - "/lens", - "/lens/{lens_id}", - "/lens/{lens_id}/runs", - "/lens/{lens_id}/runs/{job_id}", - "/lens/{lens_id}/executions/{execution_id}", - "/lens/{lens_id}/cancel", - "/lens/{lens_id}/findings/{finding_id}", - "/lens/preview/sample", - "/lens/workers/register", - "/lens/workers/{worker_id}", + trace_read_routes = [ "/v1/traces", - "/v1/logs", - "/v1/traces/query", "/v1/traces/query/help", "/v1/traces/{trace_id}", "/v1/traces/{trace_id}/spans/{span_id}", "/v1/traces/{trace_id}/spans/{span_id}/error", ] + trace_query_routes = ["/v1/traces/query", "/v1/traces/{trace_id}/spans"] + + litellm_native_routes = ( + trace_read_routes + + trace_query_routes + + [ + "/rag/ingest", + "/v1/rag/ingest", + "/rag/query", + "/v1/rag/query", + "/lens", + "/lens/{lens_id}", + "/lens/{lens_id}/runs", + "/lens/{lens_id}/runs/{job_id}", + "/lens/{lens_id}/executions/{execution_id}", + "/lens/{lens_id}/cancel", + "/lens/{lens_id}/findings/{finding_id}", + "/lens/preview/sample", + "/lens/workers/register", + "/lens/workers/{worker_id}", + "/v1/logs", + ] + ) anthropic_routes = [ "/v1/messages", diff --git a/litellm/proxy/auth/route_checks.py b/litellm/proxy/auth/route_checks.py index 6445f0d7b05..e0c954cf416 100644 --- a/litellm/proxy/auth/route_checks.py +++ b/litellm/proxy/auth/route_checks.py @@ -860,6 +860,14 @@ class RouteChecks: global_spend_tracking_routes (legacy explicit-allow set). - Else 403. """ + method: Final = request.method.upper() if request is not None else "GET" + trace_reads: Final = ( + LiteLLMRoutes.trace_query_routes.value if method == "POST" else LiteLLMRoutes.trace_read_routes.value + ) + if (method in RouteChecks._SAFE_HTTP_METHODS or method == "POST") and RouteChecks.check_route_access( + route=route, allowed_routes=trace_reads + ): + return if RouteChecks.is_llm_api_route(route=route): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -888,7 +896,6 @@ class RouteChecks: detail=f"user not allowed to access this route, role= {_user_role}. Trying to access: {route}", ) # Allow read operations on management routes (like /user/info, /team/info, /model/info) - method: Final = request.method.upper() if request is not None else "GET" is_safe_method: Final = method in RouteChecks._SAFE_HTTP_METHODS # ── Safe HTTP method: default-allow ────────────────────────────── diff --git a/tests/unit/proxy/auth/test_route_checks.py b/tests/unit/proxy/auth/test_route_checks.py index e60025eb0eb..bd486c453e0 100644 --- a/tests/unit/proxy/auth/test_route_checks.py +++ b/tests/unit/proxy/auth/test_route_checks.py @@ -99,9 +99,7 @@ def _daily_activity_route_outcome(route: str, user_role: LitellmUserRoles) -> st def test_daily_activity_routes_preserve_route_access_outcomes( existing_path: str, new_path: str, user_role: LitellmUserRoles ) -> None: - assert _daily_activity_route_outcome(new_path, user_role) == _daily_activity_route_outcome( - existing_path, user_role - ) + assert _daily_activity_route_outcome(new_path, user_role) == _daily_activity_route_outcome(existing_path, user_role) def test_non_admin_config_update_route_rejected(): @@ -212,7 +210,9 @@ def test_user_banner_read_open_to_non_admin_roles(role): LitellmUserRoles.INTERNAL_USER_VIEW_ONLY.value, ], ) -def test_latest_release_info_read_open_to_non_admin_roles(role): # test-quality-ok: allowed path returns None, not raising is the observable +def test_latest_release_info_read_open_to_non_admin_roles( + role, +): # test-quality-ok: allowed path returns None, not raising is the observable user_obj = LiteLLM_UserTable( user_id="test_user", user_email="test@example.com", @@ -1026,9 +1026,7 @@ _CLAUDE_CODE_GATEWAY_ROUTES: Final = ( @pytest.mark.parametrize("route", _CLAUDE_CODE_GATEWAY_ROUTES) -@pytest.mark.parametrize( - "role", [LitellmUserRoles.INTERNAL_USER.value, LitellmUserRoles.INTERNAL_USER_VIEW_ONLY.value] -) +@pytest.mark.parametrize("role", [LitellmUserRoles.INTERNAL_USER.value, LitellmUserRoles.INTERNAL_USER_VIEW_ONLY.value]) def test_claude_code_gateway_routes_open_to_signed_in_cli_users(role: str, route: str): user_obj: Final = LiteLLM_UserTable(user_id="test_user", user_email="test@example.com", user_role=role) valid_token: Final = UserAPIKeyAuth(user_id="test_user", user_role=role) @@ -1356,8 +1354,7 @@ def test_non_proxy_admin_allows_auth_pass_through_with_team_allowlist(): ) def test_jwt_team_routes_grant_pass_through_only_for_explicit_paths(route, team_allowed_routes, expected): assert ( - RouteChecks.jwt_team_routes_grant_pass_through(route=route, team_allowed_routes=team_allowed_routes) - is expected + RouteChecks.jwt_team_routes_grant_pass_through(route=route, team_allowed_routes=team_allowed_routes) is expected ) @@ -2329,9 +2326,7 @@ def test_logs_drawer_detail_route_in_every_route_group(route_group_name): from litellm.proxy._types import LiteLLMRoutes allowed_routes = getattr(LiteLLMRoutes, route_group_name).value - assert RouteChecks.check_route_access( - route="/spend/logs/ui/req-34099", allowed_routes=allowed_routes - ) + assert RouteChecks.check_route_access(route="/spend/logs/ui/req-34099", allowed_routes=allowed_routes) def test_logs_drawer_detail_route_allowed_for_scoped_virtual_key(): @@ -2343,9 +2338,7 @@ def test_logs_drawer_detail_route_allowed_for_scoped_virtual_key(): user_id="scoped_key_user", allowed_routes=["spend_tracking_routes"], ) - assert RouteChecks.is_virtual_key_allowed_to_call_route( - route="/spend/logs/ui/req-34099", valid_token=valid_token - ) + assert RouteChecks.is_virtual_key_allowed_to_call_route(route="/spend/logs/ui/req-34099", valid_token=valid_token) @pytest.mark.parametrize("route", ADMIN_VIEWER_LOGS_PAGE_ROUTES) @@ -4259,7 +4252,9 @@ def test_claude_code_marketplace_routes_open_to_internal_users(route): assert _gate(route, LitellmUserRoles.INTERNAL_USER.value) == "allowed" -@pytest.mark.parametrize("user_role", [None, LitellmUserRoles.INTERNAL_USER.value, LitellmUserRoles.INTERNAL_USER_VIEW_ONLY.value]) +@pytest.mark.parametrize( + "user_role", [None, LitellmUserRoles.INTERNAL_USER.value, LitellmUserRoles.INTERNAL_USER_VIEW_ONLY.value] +) @pytest.mark.parametrize("allowed_routes", [None, ["llm_api_routes"]]) def test_auto_router_session_is_reachable_by_any_key_but_benchmarks_stays_admin_only( user_role: str | None, allowed_routes: list[str] | None @@ -4451,16 +4446,34 @@ def test_legacy_sse_respects_virtual_key_route_permissions(route: str, route_gro @pytest.mark.parametrize( - "route", - ("/v1/traces", "/v1/traces/trace-id", "/v1/traces/trace-id/spans/span-id", - "/v1/traces/trace-id/spans/span-id/error"), + "route,method", + ( + ("/v1/traces", "GET"), + ("/v1/traces/trace-id", "GET"), + ("/v1/traces/trace-id/spans/span-id", "GET"), + ("/v1/traces/trace-id/spans/span-id/error", "GET"), + ("/v1/traces/trace-id/spans", "POST"), + ("/v1/traces/query", "POST"), + ), ) -def test_non_admin_trace_reads_reach_endpoint_visibility_checks(route: str) -> None: - user_role: Final = LitellmUserRoles.INTERNAL_USER +@pytest.mark.parametrize("user_role", (LitellmUserRoles.INTERNAL_USER, LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY)) +def test_non_admin_trace_reads_reach_endpoint_visibility_checks( + route: str, method: str, user_role: LitellmUserRoles +) -> None: user: Final = LiteLLM_UserTable(user_id="reader", user_role=user_role.value) auth: Final = UserAPIKeyAuth(user_id="reader", user_role=user_role) - request: Final = Request({"type": "http", "method": "GET", "query_string": b""}) + request: Final = Request({"type": "http", "method": method, "query_string": b""}) assert RouteChecks.is_llm_api_route(route) RouteChecks.non_proxy_admin_allowed_routes_check( user_obj=user, _user_role=user_role.value, route=route, request=request, valid_token=auth, request_data={} ) + + +@pytest.mark.parametrize( + "route,method", (("/v1/traces", "POST"), ("/v1/logs", "POST"), ("/v1/traces/trace-id/spans", "PUT")) +) +def test_trace_read_access_does_not_allow_viewer_writes(route: str, method: str) -> None: + request: Final = Request({"type": "http", "method": method, "query_string": b""}) + with pytest.raises(HTTPException) as caught: + RouteChecks._check_proxy_admin_viewer_access(route, LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, {}, request) + assert caught.value.status_code == 403 diff --git a/ui/litellm-dashboard/src/components/lens/traces/detail/content/ContentTab.tsx b/ui/litellm-dashboard/src/components/lens/traces/detail/content/ContentTab.tsx index 18cd99baecb..563d968db65 100644 --- a/ui/litellm-dashboard/src/components/lens/traces/detail/content/ContentTab.tsx +++ b/ui/litellm-dashboard/src/components/lens/traces/detail/content/ContentTab.tsx @@ -76,7 +76,11 @@ interface ContentTabProps { export function ContentTab({ accessToken, traceId, traceRef, span }: ContentTabProps) { const detailQuery = useSpanDetail(accessToken, traceId, span.span_id, traceRef); const detail = detailQuery.data; - const empty = detail && !detail.input && !detail.output; + const empty = + detail && + !detail.input && + !detail.output && + !["input", "output"].some((field) => detail.attributes[`lens.content.${field}_status`] === "conflicting"); return (
{(span.status === "error" || span.error) && ( diff --git a/ui/litellm-dashboard/src/components/lens/traces/detail/conversation/TraceConversation.integration.test.tsx b/ui/litellm-dashboard/src/components/lens/traces/detail/conversation/TraceConversation.integration.test.tsx index ea1e1b649d0..eb66ce65bba 100644 --- a/ui/litellm-dashboard/src/components/lens/traces/detail/conversation/TraceConversation.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/lens/traces/detail/conversation/TraceConversation.integration.test.tsx @@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { ComponentProps } from "react"; import { renderWithProviders, testQueryClient } from "../../../../../../tests/test-utils"; import { RunView } from "../run/RunView"; +import { ContentTab } from "../content/ContentTab"; import { useOpenTraceRouting } from "../../routing"; import { TraceConversation } from "./TraceConversation"; import type { SpanDetail, Trace } from "../../types"; @@ -129,6 +130,39 @@ describe("TraceConversation", () => { expect(within(result).getByText(/"exit_code": 1/, { selector: "pre" })).toHaveTextContent('"error": null'); }); + it.each([ + ["recorded", "The user rejected this tool use", "Result recorded"], + ["recorded", "", "Result recorded"], + ["not_recorded", "", "Result not recorded"], + ["conflicting", "", "Conflicting results"], + ])("does not label an unexecuted invocation completed with %s output %j", async (status, output, badge) => { + const user = userEvent.setup(); + vi.mocked(agentTraceSpanCall).mockResolvedValue({ + ...toolDetail, + output, + attributes: { + "lens.content.execution_status": "not_recorded", + "lens.content.output_status": status, + "lens.content.tool_result_is_error": output ? "1" : "0", + }, + }); + renderWithProviders( + , + ); + expect(await screen.findByText("Execution not recorded")).toBeVisible(); + expect(screen.getByText(badge)).toBeVisible(); + expect(screen.queryByText("Completed")).not.toBeInTheDocument(); + const expand = screen.queryByRole("button", { name: "Expand read_file tool call" }); + if (expand) await user.click(expand); + if (output) expect(screen.getByText(output)).toBeVisible(); + else if (status === "recorded") expect(screen.getByText("The recorded result is empty.")).toBeVisible(); + else if (status === "not_recorded") expect(screen.getByText("No result content recorded.")).toBeVisible(); + else + expect( + screen.getByText("Conflicting tool outputs were recorded. Inspect the capture source spans."), + ).toBeVisible(); + }); + it("loads twenty span details initially and pages conversation entries on demand", async () => { const user = userEvent.setup(); const spans = [ @@ -477,6 +511,37 @@ describe("TraceConversation", () => { expect(screen.queryByText("End of conversation")).not.toBeInTheDocument(); }); + it("keeps loaded conversation content visible when a background refresh fails", async () => { + const user = userEvent.setup(); + vi.mocked(agentTraceSpansCall) + .mockResolvedValueOnce([rootDetail, toolDetail]) + .mockRejectedValueOnce(new Error("refresh failed")); + renderWithProviders(); + expect(await screen.findByText("The release is ready")).toBeVisible(); + await user.click(screen.getByRole("button", { name: "Expand read_file tool call" })); + await act(async () => { + await testQueryClient.invalidateQueries({ queryKey: ["agentTraceContents"] }); + }); + expect(await screen.findByRole("alert")).toHaveTextContent("Retry this batch"); + expect(screen.getByText("Read the release notes")).toBeVisible(); + expect(screen.getByText("All checks passed")).toBeVisible(); + expect(screen.getByText("The release is ready")).toBeVisible(); + }); + + it.each(["input", "output"])("distinguishes conflicting %s content from absent evidence", async (field) => { + vi.mocked(agentTraceSpanCall).mockResolvedValue({ + ...toolDetail, + input: "", + output: "", + attributes: { [`lens.content.${field}_status`]: "conflicting" }, + }); + renderWithProviders(); + expect( + await screen.findByText(`Conflicting ${field} content was recorded. Inspect the capture source spans.`), + ).toBeVisible(); + expect(screen.queryByText("No content recorded for this span.")).not.toBeInTheDocument(); + }); + it("retains readable split results and retries only the oversized span", async () => { const user = userEvent.setup(); const oversized = { ...tool, span_id: "oversized", name: "Large result", start_offset_ms: 2 }; diff --git a/ui/litellm-dashboard/src/components/lens/traces/detail/conversation/TraceConversation.tsx b/ui/litellm-dashboard/src/components/lens/traces/detail/conversation/TraceConversation.tsx index c44d54b557b..452f3306f45 100644 --- a/ui/litellm-dashboard/src/components/lens/traces/detail/conversation/TraceConversation.tsx +++ b/ui/litellm-dashboard/src/components/lens/traces/detail/conversation/TraceConversation.tsx @@ -303,7 +303,13 @@ function ConversationTool({ item, onOpenStep }: { item: ConversationItem; onOpen const failed = item.toolAttempt?.isError ?? item.span.status === "error"; const executionStatus = failed ? "Failed" : "Completed"; const [open, setOpen] = useState(failed); - const name = item.toolAttempt ? "Tool attempt" : item.span.name; + const name = item.toolCall?.name ?? item.span.name; + const attemptStatus = + item.toolAttempt?.resultStatus === "recorded" + ? "Result recorded" + : item.toolAttempt?.resultStatus === "conflicting" + ? "Conflicting results" + : "Result not recorded"; const summary = toolSummary(item.toolCall?.args); return (
@@ -329,7 +335,7 @@ function ConversationTool({ item, onOpenStep }: { item: ConversationItem; onOpen )} - {item.toolAttempt ? "Result recorded" : executionStatus} + {item.toolAttempt ? attemptStatus : executionStatus}