Repository navigation
fix(session-ui): reveal generic tool inputs and output - #58498
as992949791 wants to merge 3 commits into
Conversation
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
I checked this with the component test it adds, which goes through the real ToolDisplay → GenericTool path in Chromium. On the base, its main case fails because the row has no expandable content. With this change all 8 tests in basic-tool.spec.ts pass, and the session-ui unit tests and tsc -b are clean. I couldn't run the issue's full web-UI script end to end in this environment, so I have no before/after screenshots of the app; yours cover that.
The change looks sound. ToolDisplay already passes output. The row stays collapsed by default, can't be opened while the tool is running, and has no arrow when it's hidden or empty. Inputs are shown as plain text or JSON, and so is the output, so nothing in them is rendered as HTML.
Public API: unchanged.
The only request is to make the new component test smaller (see inline).
| }) | ||
|
|
||
| for (const scenario of ["input", "output", "empty", "hidden", "running"] as const) { | ||
| story(`preserves generic tool details for ${scenario} calls`, async ({ mount, page }) => { |
There was a problem hiding this comment.
These six stories, with a fixture that switches between six scenarios and branches inside the loop, are a lot of test for a small change, and they're hard to follow. Could you cut it down to the core case? Mount a tool with a multi-line string input and an output, click to open, then check that the full input and the output are visible and that the output is shown as plain text. You could add one check that an empty tool has no arrow. The running, hidden and long per-field toContainText checks can go.
There was a problem hiding this comment.
Reduced this to the multiline input and literal output case, plus one empty-tool arrow check. Removed the scenario loop and running, hidden, and per-field assertions. Production code is unchanged. The native component suites, unit tests, type checks, and root checks pass.
There was a problem hiding this comment.
My earlier comment is addressed: the test is now one small story that checks the full multi-line input and the output shown as plain text, plus one that checks an empty tool has no arrow. The fix itself is the same as before. With these commits the component tests pass, and the main story still fails on the base. Public API: unchanged. Nothing else from me.
There was a problem hiding this comment.
This push only merges the latest dev, and the fix works the same as before. With it, the component tests, tsc -b and the session-ui unit tests pass. The smaller test from the previous push is still in place. Public API: unchanged. Nothing new from me.
Issue for this PR
Closes #55961
Type of change
What does this PR do?
Restore collapsed input/output details for plugin and MCP tools without a dedicated renderer.
ToolDisplay → GenericTool → BasicToolnow reveals complete input values and literal text output while keeping the compact header.Reversible UI change; scope is fallback tool details and one English label. Codex assisted implementation and verification.
How did you verify your code works?
Merged
devat44fc9d2, retaining its schema-checked input type and the reviewed disclosure behavior. The string-rendering expression now follows the current schema-decoder lint rule.ToolDisplayregression fails on currentdevbecause clicking creates no details. After: all 6 focused cases pass, with 40/40 repetitions and a 4× CPU rehearsal.Screenshots / recordings
Native component fixture with synthetic data.
Before:
After opening:
Checklist