[3609] Expose mutating transcript breadcrumbs in interactive TUI#3610
[3609] Expose mutating transcript breadcrumbs in interactive TUI#3610
Conversation
There was a problem hiding this comment.
njfio has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40954508ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .rev() | ||
| .find(|entry| entry.has_mutating_target()) | ||
| .map(|entry| entry.detail.as_str()) |
There was a problem hiding this comment.
Reject stale mutating targets when newest mutation has no path
latest_successful_mutating_target scans backward until it finds any successful mutation with a non-empty detail, so if the newest successful write/edit entry has an empty detail it will incorrectly return an older path. In that case /copy-target reports success for a stale file instead of failing loudly, which breaks the “latest successful mutating target” contract and can mislead users into copying the wrong file path.
Useful? React with 👍 / 👎.
| return child; | ||
| } | ||
|
|
||
| Command::new("pbcopy") |
There was a problem hiding this comment.
Add non-macOS clipboard fallback for copy-target
The default clipboard command is hardcoded to pbcopy, which is macOS-specific. On Linux/Windows (both first-class environments in this repo), /copy-target will always fail to start the clipboard process unless users preconfigure TAU_TUI_CLIPBOARD_COMMAND, so the new command is effectively broken out of the box for those platforms.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds clearer visibility and export of the latest mutating (write/edit) target path in the interactive tau-tui transcript and command system, addressing issue #3609.
Changes:
- Render a dedicated “Mutating target:” breadcrumb for
write/edittool transcript entries when detail is present. - Add a new
/copy-targetinteractive command to copy the most recent successful mutating target via a clipboard command. - Add integration-style TUI tests covering transcript breadcrumbs and the
/copy-targetcommand behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/3609-mutating-transcript-breadcrumbs.md | Adds a spec for the feature and test plan. |
| crates/tau-tui/src/interactive/ui_overlays.rs | Exposes /copy-target in the help overlay. |
| crates/tau-tui/src/interactive/ui_mutating_transcript_target_tests.rs | Adds render/command-path tests for breadcrumbs and /copy-target. |
| crates/tau-tui/src/interactive/ui_chat_tool_lines.rs | Implements mutating transcript breadcrumb rendering. |
| crates/tau-tui/src/interactive/ui_chat_mutating_progress.rs | Refactors mutating checks to ToolEntry helpers. |
| crates/tau-tui/src/interactive/tools.rs | Adds ToolEntry helpers and ToolPanel::latest_successful_mutating_target. |
| crates/tau-tui/src/interactive/mod.rs | Wires in new module and test module. |
| crates/tau-tui/src/interactive/build_status.rs | Reuses ToolEntry::is_successful_mutation for build evidence classification. |
| crates/tau-tui/src/interactive/app_copy_target.rs | Implements clipboard copy behavior and user-facing system messages. |
| crates/tau-tui/src/interactive/app_commands.rs | Registers the copy-target command handler. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # 3609 Mutating Transcript Breadcrumbs | ||
|
|
||
| ## Objective | ||
| Make mutating file activity obvious inside the transcript itself and expose a command that copies the latest successful mutating target path from the interactive TUI. | ||
|
|
There was a problem hiding this comment.
This spec file doesn’t follow the repo spec contract/tooling: AGENTS.md requires per-issue specs at specs/<issue-id>/spec.md with a line starting Status: (and scripts/dev/spec-archive-index.sh only scans specs/*/spec.md). As written (specs/3609-mutating-transcript-breadcrumbs.md) and without a Status: line, it won’t be picked up by the archive tooling.
| lines.push(Line::from(Span::styled( | ||
| " Mutating target:", | ||
| Style::default() | ||
| .fg(Color::Yellow) | ||
| .add_modifier(Modifier::BOLD), | ||
| ))); | ||
| lines.push(Line::from(Span::styled( | ||
| format!(" {}", entry.detail), | ||
| Style::default().fg(Color::DarkGray), | ||
| ))); | ||
| return; |
There was a problem hiding this comment.
push_transcript_detail_lines treats ToolEntry.detail as a target path for all write/edit statuses. For failed/timeout mutations, detail is often an error message (e.g. “permission denied”), so labeling it as a “Mutating target” breadcrumb is misleading. Consider only rendering this breadcrumb for statuses where detail is actually a path (e.g. Running/Success), and fall back to the generic detail line for Failed/Timeout.
| lines.push(Line::from(Span::styled( | |
| " Mutating target:", | |
| Style::default() | |
| .fg(Color::Yellow) | |
| .add_modifier(Modifier::BOLD), | |
| ))); | |
| lines.push(Line::from(Span::styled( | |
| format!(" {}", entry.detail), | |
| Style::default().fg(Color::DarkGray), | |
| ))); | |
| return; | |
| match entry.status { | |
| ToolStatus::Running | ToolStatus::Success => { | |
| lines.push(Line::from(Span::styled( | |
| " Mutating target:", | |
| Style::default() | |
| .fg(Color::Yellow) | |
| .add_modifier(Modifier::BOLD), | |
| ))); | |
| lines.push(Line::from(Span::styled( | |
| format!(" {}", entry.detail), | |
| Style::default().fg(Color::DarkGray), | |
| ))); | |
| return; | |
| } | |
| ToolStatus::Failed | ToolStatus::Timeout => { | |
| // For failed/timeout mutations, `detail` is often an error message, | |
| // so render it as a generic detail line instead of a mutating target. | |
| push_tool_detail_line(lines, &entry.detail); | |
| return; | |
| } | |
| } |
| return child; | ||
| } | ||
|
|
||
| Command::new("pbcopy") |
There was a problem hiding this comment.
The default clipboard fallback is pbcopy, which is macOS-specific. On Linux/Windows this will always fail unless TAU_TUI_CLIPBOARD_COMMAND is set, making /copy-target effectively non-functional out of the box. Consider selecting sensible platform defaults (e.g. wl-copy/xclip/clip) or making the error message explicitly instruct users to set TAU_TUI_CLIPBOARD_COMMAND when no supported clipboard command is available.
| Command::new("pbcopy") | |
| // Platform-specific sensible defaults when TAU_TUI_CLIPBOARD_COMMAND is not set. | |
| #[cfg(target_os = "windows")] | |
| { | |
| // Use the built-in Windows clipboard command. | |
| let mut child = Command::new("cmd"); | |
| child.arg("/C").arg("clip"); | |
| child | |
| } | |
| #[cfg(target_os = "macos")] | |
| { | |
| // Preserve existing macOS default. | |
| Command::new("pbcopy") | |
| } | |
| #[cfg(all(unix, not(target_os = "macos")))] | |
| { | |
| // On other Unix-like systems, try wl-copy (Wayland) first, then xclip (X11). | |
| let mut child = Command::new("sh"); | |
| child.arg("-c").arg("wl-copy || xclip -selection clipboard"); | |
| child | |
| } |
| std::env::set_var( | ||
| "TAU_TUI_CLIPBOARD_COMMAND", | ||
| format!("cat > {}", export_path.display()), | ||
| ); |
There was a problem hiding this comment.
This test builds TAU_TUI_CLIPBOARD_COMMAND using format!("cat > {}", export_path.display()), which is not shell-escaped. If the temp dir path contains spaces or shell metacharacters, the command can fail (or behave unexpectedly) under sh -c. Consider quoting/escaping the path for the shell, or avoiding the shell entirely in tests (e.g., by using a helper script/binary that takes a filepath argument safely).
Closes #3609
Issue: #3609
Spec:
specs/3609-mutating-transcript-breadcrumbs.mdWhat changed:
Mutating target:transcript breadcrumbs forwrite/edittool entries/copy-targetto export the latest successful mutating target path via the clipboard command/copy-targetin the interactive help overlayWhy:
Test evidence:
cargo test -p tau-tuicargo clippy -p tau-tui --all-targets --all-features -- -D warningscargo run -p tau-tui -- interactive --profile ops-interactiveand/copy-target