Conversation
There was a problem hiding this comment.
Pull request overview
Adds child-session tool activity to task tool rendering and ensures child-session tool parts are tracked and can trigger re-rendering of the owning task block.
Changes:
- Track and upsert tool parts received for child sessions, and re-render the parent task tool in the active session when child activity updates.
- Pass a child-session part fetcher into the formatter so task tools can render “live” child tool activity.
- Standardize action-line formatting (icon + label + value) and add status icons (running/pending/completed).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/manual/renderer_replay.lua | Stabilizes replay notifications by using a consistent notify id. |
| tests/data/explore.expected.json | Updates expected render output to include child-session selection/action rendering. |
| lua/opencode/ui/renderer.lua | Stores child-session tool parts and re-renders the owning task tool part when relevant child parts update. |
| lua/opencode/ui/render_state.lua | Adds storage + accessors for child-session parts. |
| lua/opencode/ui/icons.lua | Adds icons for child-tool status rendering (completed/pending/running). |
| lua/opencode/ui/formatter.lua | Refactors action formatting, adds child-session activity list to task tool rendering, and wires in child-session part retrieval. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function M._resolve_file_name(file_path) | ||
| if not file_path then | ||
| return '' | ||
| end | ||
| local cwd = vim.fn.getcwd() | ||
| local absolute = vim.fn.fnamemodify(file_path, ':p') | ||
| if vim.startswith(absolute, cwd .. '/') then | ||
| return absolute:sub(#cwd + 2) | ||
| end | ||
| return absolute | ||
| end |
There was a problem hiding this comment.
M._resolve_file_name('') will fall through and call fnamemodify('', ':p'), which typically resolves to the current directory, causing empty/unknown file paths to render as the CWD. Treat '' as empty input (return '') similarly to nil.
| function M._tool_action_line(part, status) | ||
| local tool = part.tool | ||
| local input = part.state and part.state.input or {} | ||
| local handler = tool_summary_handlers[tool] or tool_summary_handlers['tool'] | ||
| local icon_name, tool_value = handler(part, input) | ||
| if status ~= 'completed' then | ||
| icon_name = status | ||
| end | ||
|
|
||
| return M._build_action_line(icon_name, tool or 'tool', tool_value) | ||
| end |
There was a problem hiding this comment.
The rendered label uses part.tool as the action label (tool or 'tool'), which produces incorrect/less-meaningful labels for tools where the display action differs from the tool name (e.g. bash should display run, webfetch should display fetch, apply_patch should likely display apply patch). Adjust the handler contract to return both the icon name and the display action label (plus value), and pass that display label into _build_action_line.
| ---Find and re-render the task tool part in the active session that owns a given child session | ||
| ---@param child_session_id string The child session ID to look up | ||
| function M._rerender_task_tool_for_child_session(child_session_id) | ||
| if not state.messages then | ||
| return | ||
| end | ||
|
|
||
| for _, message in ipairs(state.messages) do | ||
| for _, part in ipairs(message.parts or {}) do | ||
| if | ||
| part.tool == 'task' | ||
| and part.state | ||
| and part.state.metadata | ||
| and part.state.metadata.sessionId == child_session_id | ||
| then | ||
| M._rerender_part(part.id) | ||
| return | ||
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
This does a full scan of state.messages/message.parts on every relevant child-session part update. Consider maintaining an index (e.g., child_session_id -> owning_task_part_id) when task parts are first rendered/seen, so the re-render can be O(1) rather than O(total parts).
| ---Build the action line string for a part (icon + meaningful value, no duration) | ||
| ---Used to show per-tool icon+label in child session activity lists. | ||
| ---@param part OpencodeMessagePart | ||
| ---@param status string Optional icon name to use for the status (e.g., 'running', 'completed', 'error'). If not provided, will use the default icon for the tool. | ||
| ---@return string | ||
| function M._tool_action_line(part, status) |
There was a problem hiding this comment.
The status param is documented as optional, but the function treats it as required (and callers pass it). Either update the annotation/documentation to reflect it is required, or make the implementation robust to nil (so it truly is optional).
No description provided.