Add expandable command and file-change activity boxes - #3173
Conversation
…activity-boxes # Conflicts: # apps/web/src/components/chat/MessagesTimeline.browser.tsx
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
vouch:unvouched
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces substantial new UI capability (expandable command/file-change rows) with ~1400 lines of new parsing and display logic across two new files. Multiple unresolved review comments identify potential correctness bugs in the output merging and quote handling logic. You can customize Macroscope's approvability policy. Learn more. |
- Treat whitespace-only stdout as absent\n- Fall back to aggregated command output
- Keep blank stream chunks for live command updates - Avoid synthetic separators around whitespace chunks
- Preserve incremental chunks without injecting separators - Keep blank-only raw output content during tool updates - Avoid trim allocation when checking command output
This reverts commit de5480b.
This reverts commit c0fe7fa.
This reverts commit d1583db.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
- Move expandable work entry logic into a shared module - Re-export the moved helpers from MessagesTimeline.logic - Keep the timeline component focused on rendering
- Remove duplicate timeline helper re-exports - Stream command-output tail and dedupe calculations - Safely serialize MCP tool data and cover edge cases
- Match absolute changed files against workspace-relative diffs - Cover basename root diffs without hiding nested namesakes
- Keep changed-file-only reads in generic detail panels - Preserve repeated MCP argument objects while redacting cycles
- Preserve expandable file and command activity details - Bring in mobile, relay, legal, and web performance updates
- Keep cumulative and oversized patch payloads renderable - Reflect non-zero exits and exact zero durations in row details - Add focused regressions for all four review findings
- Document file and command activity behavior - Record focused tests and development ports
- Clarify current behavior and conflict-resolution guidance - Document ownership of activity parsing, timeline ordering, and rendering
| function trimMatchingOuterQuotes(value: string): string { | ||
| const trimmed = value.trim(); | ||
| if ( | ||
| (trimmed.startsWith("'") && trimmed.endsWith("'")) || | ||
| (trimmed.startsWith('"') && trimmed.endsWith('"')) | ||
| ) { | ||
| const unquoted = trimmed.slice(1, -1).trim(); | ||
| return unquoted.length > 0 ? unquoted : trimmed; | ||
| } | ||
| return trimmed; |
There was a problem hiding this comment.
🟡 Medium lib/workLogActivity.ts:197
trimMatchingOuterQuotes strips a pair of quotes whenever the string begins and ends with the same quote character, even when those quotes belong to separate argument tokens. For a wrapper remainder like "Write-Output" "hello world", it removes the opening quote of the first argument and the closing quote of the last, producing Write-Output" "hello world — a malformed command that corrupts the displayed output. Consider only stripping quotes when they form a single enclosing pair (e.g., by verifying no other same-quote character appears between them).
function trimMatchingOuterQuotes(value: string): string {
const trimmed = value.trim();
if (
- (trimmed.startsWith("'") && trimmed.endsWith("'")) ||
- (trimmed.startsWith('"') && trimmed.endsWith('"'))
+ (trimmed.startsWith("'") && trimmed.endsWith("'") && trimmed.indexOf("'", 1) === trimmed.length - 1) ||
+ (trimmed.startsWith('"') && trimmed.endsWith('"') && trimmed.indexOf('"', 1) === trimmed.length - 1)
) {
const unquoted = trimmed.slice(1, -1).trim();
return unquoted.length > 0 ? unquoted : trimmed;
}🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/lib/workLogActivity.ts around lines 197-206:
`trimMatchingOuterQuotes` strips a pair of quotes whenever the string begins and ends with the same quote character, even when those quotes belong to separate argument tokens. For a wrapper remainder like `"Write-Output" "hello world"`, it removes the opening quote of the first argument and the closing quote of the last, producing `Write-Output" "hello world` — a malformed command that corrupts the displayed output. Consider only stripping quotes when they form a single enclosing pair (e.g., by verifying no other same-quote character appears between them).
| if (previous.startsWith(next)) { | ||
| if (shouldKeepLongerOutputSnapshot(previous, next, nextActivityKind)) { | ||
| return previous; | ||
| } | ||
| if (nextActivityKind === "tool.updated" && (next.length === 1 || previous.includes("\n"))) { | ||
| return `${previous}${next}`; | ||
| } | ||
| return next; | ||
| } |
There was a problem hiding this comment.
🟡 Medium lib/workLogActivity.ts:130
mergeCumulativeOutput drops accumulated output when a multi-character incremental chunk is also a prefix of the previous output. For example, mergeCumulativeOutput("hello", "he", "tool.updated") returns "he" instead of "hellohe", erasing already-displayed output. The previous.startsWith(next) branch treats any next that is a prefix of previous as a shorter snapshot, but for tool.updated an incremental chunk should be appended unless it is a single character or previous already contains a newline. Consider guarding the single-character/line-based conditions before falling back to next so multi-character incremental prefixes are concatenated.
if (previous.startsWith(next)) {
- if (shouldKeepLongerOutputSnapshot(previous, next, nextActivityKind)) {
- return previous;
- }
- if (nextActivityKind === "tool.updated" && (next.length === 1 || previous.includes("\n"))) {
- return `${previous}${next}`;
- }
- return next;
+ if (nextActivityKind === "tool.updated" && (next.length === 1 || previous.includes("\n"))) {
+ return `${previous}${next}`;
+ }
+ if (shouldKeepLongerOutputSnapshot(previous, next, nextActivityKind)) {
+ return previous;
+ }
+ return next;
}🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/lib/workLogActivity.ts around lines 130-138:
`mergeCumulativeOutput` drops accumulated output when a multi-character incremental chunk is also a prefix of the previous output. For example, `mergeCumulativeOutput("hello", "he", "tool.updated")` returns `"he"` instead of `"hellohe"`, erasing already-displayed output. The `previous.startsWith(next)` branch treats any `next` that is a prefix of `previous` as a shorter snapshot, but for `tool.updated` an incremental chunk should be appended unless it is a single character or `previous` already contains a newline. Consider guarding the single-character/line-based conditions before falling back to `next` so multi-character incremental prefixes are concatenated.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 74152eb. Configure here.
| const stdout = hasRenderableCommandOutput(workEntry.stdout) ? workEntry.stdout : null; | ||
| const stderr = hasRenderableCommandOutput(workEntry.stderr) ? workEntry.stderr : null; | ||
| const output = | ||
| !stdout && !stderr && hasRenderableCommandOutput(workEntry.output) ? workEntry.output : null; |
There was a problem hiding this comment.
Partial stdout hides completed output
Medium Severity
The command output display logic prioritizes stdout and stderr over the output field. This can result in incomplete or missing command output in expanded details when stdout contains an earlier, partial, or blank streamed version, while output holds the complete final result.
Reviewed by Cursor Bugbot for commit 74152eb. Configure here.
| }} | ||
| > | ||
| {outputDisplay.visibleValue} | ||
| </button> |
There was a problem hiding this comment.
Output block blocks text copy
Medium Severity
CommandOutputBlock renders stdout/stderr inside a button, and disables that button whenever output is not truncated. Short command streams therefore become hard or impossible to select and copy, which undercuts the main reason to expand command rows for debugging. Longer streams stay clickable for truncation toggles, so drag-to-select can also flip expansion unexpectedly.
Reviewed by Cursor Bugbot for commit 74152eb. Configure here.
|
this will conflict a lot w/ orchestrator #2829 so we can revisit it later |
2 participants
Summary
This adds compact, expandable activity rows for commands, file changes, and generic tool calls in the chat timeline. Command rows expose execution metadata and streamed output, while file-change rows expose changed paths and inline diffs without making the collapsed timeline noisy.
The implementation normalizes provider activity in a dedicated parser, composes it into public work-log entries, and keeps expansion, scrolling, and keyboard behavior predictable as activity streams arrive.
What Changed
0msdurations.Why
Command and file-change events are otherwise mostly opaque in the timeline. Expandable details make it possible to debug command output, inspect intermediate or git-ignored file changes, and follow an agent's work more closely while preserving a compact default view.
Validation
pnpm exec vp test run --passWithNoTests --project unit src/lib/workLogActivity.test.ts src/session-logic.test.ts src/components/chat/MessagesTimeline.logic.test.ts src/components/chat/MessagesTimeline.test.tsxpassed: 4 files, 169 tests.git diff --check upstream/main...HEADpassed.Proof
Before:
After:
Expanded command output with tail truncation:
Expanded inline file-change diffs:
Note
Medium Risk
Large UI and session-timeline change with complex provider payload merging; mistakes could mislabel tool types or corrupt streamed command output, but behavior is heavily unit-tested and scoped to display/work-log composition.
Overview
Chat work-log rows for commands and file changes are now clickable and expandable instead of a single generic
<pre>dump. Collapsed rows keep compact previews (Ran command - …,Changed files - …); expansion shows structured panels.Work-log pipeline:
session-logicgains fields for stdout/stderr, exit code, duration, patches, and changed files, populated via newworkLogActivityparsing and cumulative merge rules acrosstool.updated/tool.completed(incremental output, patch prefixes, Codex-style diffs, bounded patch size). Non-zero exit codes count as tool failure;0msdurations render explicitly.UI:
MessagesTimelinerenders command blocks (raw command, metadata, tail-truncated stdout/stderr with expand) and file-change blocks (FileDiffinline, path chips, files without renderable diffs). Generic/MCP details use safe JSON serialization; file-read rows with only paths stay generic. Row Enter/Space toggles only when the row itself is focused.Tests cover extraction, merging, display derivation, path matching, and static markup for expandable rows.
Reviewed by Cursor Bugbot for commit 74152eb. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add expandable command and file-change activity boxes to work log entries
WorkLogEntrygains new optional fields (output,stdout,stderr,exitCode,durationMs,patch) populated via a new unified payload parser in workLogActivity.ts.mergeCumulativeOutput,mergeCumulativePatch) so streaming updates display coherently.SimpleWorkEntryRowin MessagesTimeline.tsx wires these together with ARIA expand/collapse attributes.shouldToggleWorkEntryRowFromKeyDown, and non-zeroexitCodenow marks a tool entry as a failure.formatDurationnow returns"0ms"for zero-millisecond durations instead of rounding up to"1ms".Macroscope summarized 74152eb.