Sign up
View stargazers13Sign up

fix(clients): reconcile tool activity lifecycles#7617

closed4
main
ui-refresh/tool-activity-turns
openedAug 20, 2026byMmaria
10 files changed
View on GitHubTwo-way sync enabledPR last imported 4d ago

What changed

Moves the deeper behavior out of #7152 into a follow-up layer:

  • reconciles interleaved tool lifecycle events by stable call ID without merging distinct or ambiguous calls
  • preserves terminal outcomes and turn-scoped identity on web and mobile
  • keeps live tool rows in timeline order and updates settled turn-fold anchoring
  • filters duplicate plan and terminal-input activity from work history
  • preserves composer task progress after removing the old active-plan fallback

Why

The one-line tool UI can be reviewed and merged independently. This PR contains the turn, lifecycle, and mobile behavior needed for complete provider histories.

Validation

  • 197 focused web and mobile tests
  • Web and mobile typechecks
  • Changed-file lint and formatting checks

Built with GPT-5.6-sol in the Codex harness.

Note

Reconcile tool activity lifecycles by toolCallId and overhaul MessagesTimeline rows

  • Tool lifecycle entries (tool.started, tool.updated, tool.completed) now collapse by stable toolCallId scoped to a turn, preventing cross-turn merges and ambiguous id-less completions. Applies to both session-logic.ts and threadActivity.ts.
  • deriveWorkLogEntries now surfaces tool.started rows (previously blanket-skipped), filters Codex terminal interactions and unkeyed statusless starts, and infers inProgress/completed statuses when missing.
  • Timeline introduces new row kinds: work-live for active expandable tool batches, enriched work-toggle with summaries and failure icons, and working rows with a showThinking flag. Turn folds anchor near the assistant response when collapsed and above the first hidden entry when expanded.
  • New shell tokenizer (tokenizeShellCommand, commandProgramName) extracts program names for live command labels; workEntryIconName delegates to toolGroupAction for icon selection.
  • deriveActivePlanState removed; activeComposerTaskSteps now sourced from turnPlans for the active turn. MessagesTimeline no longer accepts workingStepLabel or activeTurnInProgress props.
  • Risk: MessagesTimeline prop interface removes workingStepLabel and activeTurnInProgress; any external callers passing these props will break. deriveActiveWorkStartedAt signature gains latestUserMessageAt (defaults to null), and deriveActivePlanState is deleted from session-logic.ts.

Macroscope summarized befa511.

Note

Medium Risk Changes are concentrated in client-side timeline and work-log derivation with broad test coverage, but incorrect collapse or fold anchoring could misrepresent tool history or hide activity in edge cases (parallel id-less tools, steers).

Overview Tool lifecycle reconciliation on web and mobile collapses tool.started / tool.updated / tool.completed by turnId + toolCallId, including interleaved and late events, while refusing to merge ambiguous id-less completions or reuse IDs across turns. Terminal outcomes (completed, failed, etc.) win over stale in-progress updates.

Work log and timeline presentation now surfaces in-flight tool.started rows, filters duplicate plan updates, unkeyed statusless starts, Codex PTY terminal-input noise, and ExitPlanMode markers. Settled turn folds anchor before the next visible assistant reply when collapsed (fixes steer-superseded turns) and before the first hidden entry when expanded. Live tool batches stay expandable as entries append; lifecycle marker filtering and summarizeToolGroup labeling improve settled overflow rows.

Web chat drops deriveActivePlanState / workingStepLabel; composer task steps come from turnPlans for the active turn. The working row no longer shows plan step text. Subagent spawn UI shows coordinator failure states and uses a compact card with an Eye action instead of a full-width button.

Failure display splits metadata vs rendered output via workEntryDisplayIndicatesToolFailure so command text like rg 'command not found' does not show a false failure while still running.

Reviewed by Cursor Bugbot for commit befa5111b6eddbd887b2387f0837058293f4bae4. Bugbot is set up for automated code reviews on this repo. Configure here.

CcoderabbitaiBotAug 20, 2026
Synced from GitHub

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 13bf1df8-e1af-4326-91ef-270f71acb460

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Ggithub-actionsBotAug 20, 2026
Synced from GitHub

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for befa511.

This comment will update automatically after the next completed run.

MmacroscopeappBotAug 20, 2026
Synced from GitHub

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces significant new runtime behavior for tool activity lifecycle reconciliation with complex merging logic. Two Medium severity findings identify bugs in the lifecycle merging code (duplicate completions from approval.requested rows, parallel tools merging incorrectly). The scope and complexity of the changes warrant careful human review.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

apps/web/src/session-logic.tslines 1146–1146Outdated · 29acbf9Open thread
……@@ -1085,12 +1112,65 @@ function collapseDerivedWorkLogEntries(
1143+ (candidate) =>
1144+ candidate.activityKind !== "tool.completed" &&
1145+ candidate.turnId === previous.turnId &&
1146+ candidate.itemType === previous.itemType &&
MmacroscopeappBoton lines 1146–1146Aug 20, 2026

🟡 Medium src/session-logic.ts:1146

An earlier approval.requested row can make an id-less tool.completed appear ambiguous, so the keyed tool.started remains visible as running and a duplicate completion row is emitted. The candidate check must consider only tool.started and tool.updated rows as competing lifecycle targets.

diff
1- candidate.activityKind !== "tool.completed" &&
2+ (candidate.activityKind === "tool.started" ||
3+ candidate.activityKind === "tool.updated") &&
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @apps/web/src/session-logic.ts around line 1146:
2
3An earlier `approval.requested` row can make an id-less `tool.completed` appear ambiguous, so the keyed `tool.started` remains visible as running and a duplicate completion row is emitted. The candidate check must consider only `tool.started` and `tool.updated` rows as competing lifecycle targets.
apps/web/src/session-logic.tsline 1173Outdated · 29acbf9Open thread
……@@ -1085,12 +1112,65 @@ function collapseDerivedWorkLogEntries(
10931170 collapsed.push(entry);
1171+ if (lifecycleKey !== undefined) {
1172+ toolLifecycleRowIndex.set(lifecycleKey, collapsed.length - 1);
1173+ }
CcursorBoton line 1173Aug 20, 2026

Late starts duplicate keyed tool rows

Medium Severity

A delayed tool.started for an already-tracked in-progress toolCallId is rejected by shouldCollapseToolLifecycleEntries, then toolLifecycleRowIndex.delete drops the map entry while the original row stays in collapsed. The start is pushed as a second row, so a later completion merges only into the new row and leaves a ghost in-progress tool in the work log.

Additional Locations (1)

Reviewed by Cursor Bugbot for commit 29acbf910281ef90d4349d61645b6818e2d78359. Configure here.

apps/web/src/session-logic.tsline 1149Outdated · 29acbf9Open thread
……@@ -1085,12 +1112,65 @@ function collapseDerivedWorkLogEntries(
1146+ candidate.itemType === previous.itemType &&
1147+ normalizeCompactToolLabel(candidate.toolTitle ?? candidate.label) ===
1148+ normalizeCompactToolLabel(previous.toolTitle ?? previous.label),
1149+ );
CcursorBoton line 1149Aug 20, 2026

Failed siblings block id-less collapse

Medium Severity

hasCompetingIdlessCompletionTarget treats any earlier non-tool.completed row with the same turn, item type, and label as a competitor, including rows that are already terminal via failed, declined, or stopped. An unambiguous id-less completion then stays unmerged, so the remaining running call never settles.

Additional Locations (1)

Reviewed by Cursor Bugbot for commit 29acbf910281ef90d4349d61645b6818e2d78359. Configure here.

Added 4 commits

  • feat(web): group tool activity5122399
  • fix(web): align thinking activity5addb76
  • fix(web): preserve active task progressbefa511
  • fix(clients): reconcile tool activity lifecyclesd620d7b
CcursorBotreviewed · 1 inline threadAug 20, 2026
Synced from GitHub

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit befa5111b6eddbd887b2387f0837058293f4bae4. Configure here.

apps/web/src/session-logic.tsline 1209Open thread
……@@ -1099,19 +1179,34 @@ function shouldCollapseToolLifecycleEntries(
11121206 return true;
11131207 }
11141208 return (
1209+ (previous.activityKind !== "tool.started" || next.activityKind === "tool.completed") &&
CcursorBoton line 1209Aug 20, 2026

Id-less updates merge parallel tools

Medium Severity

The new id-less collapse rule attaches an unkeyed tool.updated to the previous keyed row whenever label and itemType match. The competing-target check only runs for tool.completed, so a parallel same-label update without a call id can fuse into the adjacent call and hide a distinct in-flight tool.

Additional Locations (1)

Reviewed by Cursor Bugbot for commit befa5111b6eddbd887b2387f0837058293f4bae4. Configure here.

MmacroscopeappBotreviewed · 2 inline threadsAug 20, 2026
Synced from GitHub

Reviewed the changed web UI files for shared-primitive usage, Tailwind/CSS ownership, and accessibility. The new live-activity row, tool-group summary row, and the live-activity-focus mask/animation utilities look consistent with the existing timeline row idiom (the mask + keyframes are exactly the kind of behaviorally complex CSS that belongs in index.css), and the AgentSpawnCtaRow rework correctly routes its action through the shared Button/Tooltip primitives.

Two findings, both in MessagesTimeline.test.tsx, where the tests no longer match the component contract this PR introduces.

Posted via Macroscope — UI Consistency

apps/web/src/components/chat/MessagesTimeline.test.tsxlines 1209–1211Open thread
……@@ -722,10 +1143,81 @@ describe("MessagesTimeline", () => {
1208+ {...buildProps()}
1209+ isWorking
1210+ activeTurnInProgress
1211+ activeTurnStartedAt={MESSAGE_CREATED_AT}
MmacroscopeappBoton lines 1209–1211Aug 20, 2026

activeTurnInProgress was removed from MessagesTimelineProps in this PR, so this explicit JSX attribute no longer type-checks (Property 'activeTurnInProgress' does not exist on type 'IntrinsicAttributes & MessagesTimelineProps'). Consider dropping it — the row now derives its state from showThinking.

suggestion
1 isWorking
2 activeTurnStartedAt={MESSAGE_CREATED_AT}

Posted via Macroscope — UI Consistency

apps/web/src/components/chat/MessagesTimeline.test.tsxlines 343–343Open thread
……@@ -237,6 +246,110 @@ function buildAssistantTimelineEntry(text: string) {
340+ expect(markup).toContain("Thinking");
341+ expect(markup).not.toContain("Thinking ·");
342+ expect(markup).not.toContain('aria-hidden="true" class="size-6 shrink-0"');
343+ expect(markup).toContain("gap-1.5 px-0.5 py-0.5");
MmacroscopeappBoton lines 343–343Aug 20, 2026

This assertion doesn't match what the iconless Thinking row emits. LiveActivityContent builds cn("flex min-h-6 min-w-0 items-center gap-1.5 py-0.5", resolvedIconName ? "px-0.5" : "px-1", …), and with no icon (ThinkingActivityRow) the rendered order is gap-1.5 py-0.5 px-1 — px-0.5 is only emitted when an icon is present. cn (twMerge) does not reorder classes, so this test fails as written; the later "aligns the iconless Thinking row" test asserts the correct string.

suggestion
1 expect(markup).toContain("gap-1.5 py-0.5 px-1");

Posted via Macroscope — UI Consistency