feat(inspector): add bounded tool execution details - #7279
Conversation
|
🚅 Deployed to the ironclaw-pr-7279 environment in ironclaw-ci-preview
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds bounded, redacted tool diagnostics across runtime capability execution, Inspector storage, and the WebUI. Tool lifecycle events are correlated by identifiers, persisted in diagnostic storage, fetched through a scoped API, and displayed on demand. ChangesTool diagnostic inspection
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ToolExecution
participant DiagnosticEmitter
participant InspectorStore
participant InspectorAPI
participant InspectorPanel
ToolExecution->>DiagnosticEmitter: record input, start, and result
DiagnosticEmitter->>InspectorStore: forward bounded diagnostic events
InspectorStore->>InspectorAPI: expose scoped tool details
InspectorPanel->>InspectorAPI: fetch details on expansion
InspectorAPI-->>InspectorPanel: return bounded tool metadata
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…to issue-7225-bounded-tool-details # Conflicts: # crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.test.tsx # crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.tsx
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@crates/app/ironclaw_composition/src/runtime/capability_host.rs`:
- Around line 926-933: Add the trait-required record_running_invocation method
to the UnavailableCapabilityIo implementation, matching the signature used by
LoopCapabilityResultWriter and leaving it as a no-op. Preserve the existing
write_capability_result behavior and update only this production implementation
to satisfy the expanded trait surface.
In `@crates/product/ironclaw_assistant/src/inspector_store.rs`:
- Around line 1348-1367: Propagate the tool start timestamp from
HostManagedToolStartedDiagnosticCapture through PendingToolInput and the Started
record, then update the terminal ToolExecutionDiagnostic construction to compute
duration_ms from that timestamp and the current completion time. Ensure
DiagnosticUpdateKind::ToolExecutionUpdated receives the calculated duration for
completed tool executions instead of None.
- Around line 930-942: Update record_prompt and record_model_call to obtain
their DiagnosticScope exclusively through diagnostic_scope_for_context instead
of duplicating actor/explicit-owner derivation. At each call site, preserve the
existing per-sink debug messages by checking the returned Option before
recording; leave diagnostic_scope_for_context as the single source of tenant,
user, thread, and run identity.
In
`@crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.test.tsx`:
- Around line 200-244: Update the Inspector tool-detail assertions to verify
sanitized tool arguments are rendered: in
crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.test.tsx
lines 200-244, assert that arguments.content '{"path":"safe.txt"}' appears in
detail.textContent, or remove the mock field if arguments are intentionally
omitted; in tests/e2e/scenarios/test_reborn_webui_v2_tool_gates.py lines
322-373, add a detail assertion for the echoed tool arguments alongside the
existing capability, status, and truncation checks.
In
`@crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.tsx`:
- Around line 346-360: Remove the duplicate ToolDetailText interface and reuse
the existing BoundedDiagnosticText type for all corresponding fields in
ToolDetail and ToolDetailBlock, preserving the current wire shape and behavior.
- Around line 375-389: Replace the fetch logic in load with a React.useEffect
keyed by open, threadId, runId, and activityId so it creates an AbortController,
aborts it during cleanup, and ignores aborted requests. Reset unavailable when
opening and allow each reopen to retry regardless of its prior value; update the
button handler to use toggle as proposed, while preserving the existing loading,
tool, and unavailable state updates for active requests.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4aad9239-b2d3-4327-9718-d29275cbc645
📒 Files selected for processing (13)
crates/app/ironclaw_composition/src/runtime.rscrates/app/ironclaw_composition/src/runtime/capability_host.rscrates/app/ironclaw_composition/src/runtime/capability_host/tests.rscrates/app/ironclaw_composition/src/runtime/capability_host/tests/tests/display_preview.rscrates/contracts/ironclaw_product_contracts/src/inspector.rscrates/loop/ironclaw_loop_host/src/lib.rscrates/loop/ironclaw_loop_host/src/tool_diagnostics.rscrates/product/ironclaw_assistant/src/inspector_store.rscrates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-api.tscrates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.test.tsxcrates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.tsxtests/CLAUDE.mdtests/e2e/scenarios/test_reborn_webui_v2_tool_gates.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.tsx`:
- Around line 395-399: Validate and decode the unknown result from
fetchInspectorTool before updating state in the request callback. Replace the
broad response cast around setTool with the existing ToolDetail
validation/decoding mechanism, and treat malformed payloads as unavailable or
failed detail data so Inspector rendering never receives an object missing
required fields such as capability_name.content.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 92f7971f-350a-46e5-acb8-c63ce8c6fa75
📒 Files selected for processing (2)
crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.test.tsxcrates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.tsx (1)
390-399: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winResponse is still cast without validation.
(response as { tool?: ToolDetail | null })?.toolaccepts any object shape. A malformed successful response reachestool.capability_name.contentat Line 424 with no guard. SinceInspectorErrorBoundarywraps the whole panel (Line 793), this crash disables the entire Inspector, not just one row.Decode the response into
ToolDetailbeforesetTool, and treat an invalid payload asunavailable.🤖 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 `@crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.tsx` around lines 390 - 399, Validate and decode the successful response in the inspector request’s .then handler before calling setTool, rather than using the unchecked response cast. Ensure malformed payloads are treated as unavailable by setting the tool state to null and setUnavailable(true), while preserving the existing controller and abort checks and valid ToolDetail behavior.
🤖 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 `@crates/loop/ironclaw_loop_host/src/tool_diagnostics.rs`:
- Around line 41-69: Update record_started_at and take_duration_ms_at to log
poisoned mutex errors with tracing::debug!, including the lock error and clear
timing-diagnostics context, before returning. Replace the silent let-else and
.ok()? handling while preserving their existing fallback behavior and duration
calculation.
---
Duplicate comments:
In
`@crates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.tsx`:
- Around line 390-399: Validate and decode the successful response in the
inspector request’s .then handler before calling setTool, rather than using the
unchecked response cast. Ensure malformed payloads are treated as unavailable by
setting the tool state to null and setUnavailable(true), while preserving the
existing controller and abort checks and valid ToolDetail 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4bb73d15-f1cf-4228-9bb1-8404680d8a1f
📒 Files selected for processing (8)
crates/app/ironclaw_composition/src/runtime/capability_host/tests.rscrates/app/ironclaw_composition/src/runtime/production.rscrates/loop/ironclaw_loop_host/src/lib.rscrates/loop/ironclaw_loop_host/src/tool_diagnostics.rscrates/product/ironclaw_assistant/src/inspector_store.rscrates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.test.tsxcrates/product/ironclaw_webui/frontend/src/pages/chat/inspector/inspector-panel.tsxtests/e2e/scenarios/test_reborn_webui_v2_tool_gates.py
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * feat(inspector): add prompt inspection * fix(inspector): follow current webui ownership * feat(inspector): add model call statistics * test(inspector): cover model statistics e2e * fix(inspector): avoid uncollected tool metrics * test(inspector): cover prompt diagnostics e2e * test(inspector): align statistics e2e scope * fix(inspector): redact prompt metadata * fix(inspector): preserve per-call model identity * fix(inspector): classify prompt instruction sources * test(inspector): assert reported token usage * feat(inspector): add activity timeline and turn navigation * test(inspector): cover activity timeline in browser * fix(inspector): read current run before publishing activity * feat(inspector): add bounded tool execution details * test(inspector): cover bounded tool details in browser * fix(inspector): validate retained tool result sizes * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): address prompt diagnostic review findings * fix(inspector): follow debug query navigation * fix(inspector): preserve stream terminal state * fix(inspector): capture full capability surface * fix(inspector): scope projection activity to its run * fix(inspector): harden activity diagnostics * fix(inspector): bound tool result diagnostic capture * fix(inspector): harden tool diagnostic pipeline * fix(inspector): address prompt diagnostic review feedback * fix inspector model call stats review findings * fix inspector refresh and truncation regressions * fix(inspector): address activity timeline review feedback * fix(inspector): harden activity lifecycle handling * fix(composition): move tool diagnostics to loop host * fix(inspector): cancel stale tool detail requests * fix(inspector): address bounded tool detail review * fix(inspector): validate tool detail responses
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * feat(inspector): add prompt inspection * fix(inspector): follow current webui ownership * feat(inspector): add model call statistics * test(inspector): cover model statistics e2e * fix(inspector): avoid uncollected tool metrics * test(inspector): cover prompt diagnostics e2e * test(inspector): align statistics e2e scope * fix(inspector): redact prompt metadata * fix(inspector): preserve per-call model identity * fix(inspector): classify prompt instruction sources * test(inspector): assert reported token usage * feat(inspector): add activity timeline and turn navigation * test(inspector): cover activity timeline in browser * fix(inspector): read current run before publishing activity * feat(inspector): add bounded tool execution details * test(inspector): cover bounded tool details in browser * fix(inspector): validate retained tool result sizes * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): address prompt diagnostic review findings * fix(inspector): follow debug query navigation * fix(inspector): preserve stream terminal state * fix(inspector): capture full capability surface * fix(inspector): scope projection activity to its run * fix(inspector): harden activity diagnostics * fix(inspector): bound tool result diagnostic capture * fix(inspector): harden tool diagnostic pipeline * fix(inspector): address prompt diagnostic review feedback * fix inspector model call stats review findings * fix inspector refresh and truncation regressions * fix(inspector): address activity timeline review feedback * fix(inspector): harden activity lifecycle handling * fix(composition): move tool diagnostics to loop host * fix(inspector): cancel stale tool detail requests * fix(inspector): address bounded tool detail review * fix(inspector): validate tool detail responses
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * feat(inspector): add prompt inspection * fix(inspector): follow current webui ownership * feat(inspector): add model call statistics * test(inspector): cover model statistics e2e * fix(inspector): avoid uncollected tool metrics * test(inspector): cover prompt diagnostics e2e * test(inspector): align statistics e2e scope * fix(inspector): redact prompt metadata * fix(inspector): preserve per-call model identity * fix(inspector): classify prompt instruction sources * test(inspector): assert reported token usage * feat(inspector): add activity timeline and turn navigation * test(inspector): cover activity timeline in browser * fix(inspector): read current run before publishing activity * feat(inspector): add bounded tool execution details * test(inspector): cover bounded tool details in browser * fix(inspector): validate retained tool result sizes * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): address prompt diagnostic review findings * fix(inspector): follow debug query navigation * fix(inspector): preserve stream terminal state * fix(inspector): capture full capability surface * fix(inspector): scope projection activity to its run * fix(inspector): harden activity diagnostics * fix(inspector): bound tool result diagnostic capture * fix(inspector): harden tool diagnostic pipeline * fix(inspector): address prompt diagnostic review feedback * fix inspector model call stats review findings * fix inspector refresh and truncation regressions * fix(inspector): address activity timeline review feedback * fix(inspector): harden activity lifecycle handling * fix(composition): move tool diagnostics to loop host * fix(inspector): cancel stale tool detail requests * fix(inspector): address bounded tool detail review * fix(inspector): validate tool detail responses
Summary
Linked Issue
Closes #7225
Depends on #7224
Part of #7218
Validation
cargo fmt --all -- --checkTest Strategy
User behavior:
Risk areas:
Tests added or updated:
What the tests prove:
Security Impact
Limited but security-sensitive. Tool arguments and results are sanitized and secret-scanned before bounded process-local retention. Detailed content requires operator access and an exact authenticated scope.
Database Impact
None. Tool diagnostics remain bounded and process-local.
Blast Radius
Limited to capability diagnostic capture, the Inspector store, operator-only WebUI routes, and Inspector presentation.
Rollback Plan
Revert this PR to remove verbose tool details while retaining the activity timeline and existing prompt/statistics views.
Review Follow-Through
Risk level: Medium. Review should focus on redaction boundaries, source-size validation, exact invocation scoping, and ensuring capture remains best-effort.
Review track: C