Skip to content

fix(turn-change): aggregate per-turn changes across all assistants (#428)#435

Merged
Astro-Han merged 14 commits into
devfrom
claude/issue-428-aggregate-turn-changes
May 4, 2026
Merged

fix(turn-change): aggregate per-turn changes across all assistants (#428)#435
Astro-Han merged 14 commits into
devfrom
claude/issue-428-aggregate-turn-changes

Conversation

@Astro-Han

@Astro-Han Astro-Han commented May 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • Aggregate every assistant message that shares a parentID === userMessageID into one turn-level Changes block; the SessionTurn UI now shows the union of file edits with one Undo / Reapply button instead of dropping earlier rounds.
  • Raise the on-disk restore limit from 2 MB to 20 MB so multi-step edits in one user turn fit, while keeping the 2 MB display window for the diff panel.
  • Add aggregate undo / redo HTTP endpoints with explicit force semantics (default abort on conflict, force=true skips the conflicting message and reports skipped[]).
  • Apply two rounds of crosscheck fixes: chained virtual-disk preflight, absolute-path collapse keying, partial-mutate skip handling, mixed-state availability, mid-loop rollback for force=false, and UI error / retry resilience.

Why

Issue #428: SessionTurn used findLast over messages so any user prompt that triggered multiple assistant rounds (compaction, tool retries, multi-tool plans) would silently lose earlier file changes from the Changes block. Users could no longer review or undo most of what the agent had written.

Related Issue

Closes #428.

Human Review Status

Pending. Two rounds of crosscheck (Claude Opus + Codex) drove the refinement; final merge call belongs to a human review.

Review Focus

  • packages/opencode/src/session/turn-change.tsaggregateTurnInternal, preflightTurn virtual disk chaining, mutateTurn rollback / skip semantics, mutatedPaths contract.
  • packages/opencode/src/server/instance/session.ts — new /turn/:userMessageID/changes[/undo|/redo] routes and publishTurnChangeFiles exclusion via mutatedPaths.
  • packages/app/src/pages/session/message-timeline.tsx — fetch error path, retry timer cleanup, response body immutability.
  • packages/ui/src/components/session-turn.tsx + session-turn.css + i18n — aggregate rendering, conflict confirm dialog, skipped notice.

Risk Notes

  • Raises restore quota to 20 MB per file; sessions with very large edits can grow the SQLite turn-change tables. Existing per-message rows still apply, so no migration is needed.
  • Aggregate route is additive; the legacy single-message /turn-change/:messageID/{undo,redo} endpoints are unchanged for compatibility.
  • Force-undo with conflicts is now a partial operation that surfaces a skipped[] array; UI renders a yellow notice and only publishes File events for paths that were actually written.

How To Verify

typecheck (9 packages, tsgo): all green
session + server tests (packages/opencode): 594 passed, 4 skipped, 1 todo, 0 failed (599 total across 44 files)
new regression tests: chained same-file undo / basename collision / mixed-state availability / mutatedPaths exclusion all green

UI verification: started a session with two assistant rounds touching different files plus one shared file, observed the SessionTurn Changes block listing all three with one Undo. Triggered a conflict by tampering one file on disk; the confirm dialog appeared, force-undo applied the rest, the yellow skipped notice rendered.

Screenshots or Recordings

Not attached (manual verification only). The visual change reuses the existing Changes block layout; no new component or styling beyond the skipped-notice surface and the conflict confirm dialog.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has type, scope, and priority labels, or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for desktop, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • I reviewed the final diff for unrelated changes and suspicious dependency changes
  • I am targeting `dev`, and my PR title and commit messages use Conventional Commits in English

Summary by CodeRabbit

Release Notes

  • New Features

    • Added aggregation of file changes across multiple assistants within a conversation turn with improved undo/redo workflow
    • Added "force apply" option for handling file conflicts during undo/redo operations
    • Added partial undo tracking with notifications when files are skipped due to conflicts
  • Bug Fixes

    • Enhanced error messaging for undo/redo failures, including rollback scenarios
  • Style

    • Refined UI spacing and layout for turn changes display, including new status indicators for undone turns

Astro-Han added 8 commits May 4, 2026 19:41
Display limit stays at 2 MB (changes block is not an IDE diff viewer);
restore limit goes up so Word/PDF/PPT typical files remain undoable.
Updates two existing test thresholds that used 2 MB+1 to trigger the
oversize branch.
Verifies that with display limit at 2 MB and restore limit at 20 MB:
- 3 MB file is restorable but not expandable
- 20 MB+ file has restoreAvailable=false
…istants

- TurnChange.aggregateTurn collapses every assistant in a user turn (parentID match) into one Display, merging same-path edits and dropping white-busy files
- aggregateTurnUndo / aggregateTurnRedo apply per-message mutations under the session lock; default aborts on conflict, force=true skips conflicting messages and reports skipped[]
- MutationResult/Schema gain optional skipped[] for partial-success reporting

Closes the SessionTurn findLast bug surface where only the last assistant's changes were visible.
- GET /session/:id/turn/:userMessageID/changes returns the aggregated display
- POST .../undo and .../redo accept { force? } and reuse the session lock; on applied, broadcast file-changed events
- HTTP-level e2e covers happy path, conflict-blocked default, force=true skip behavior
… flow

- SessionTurn drops the findLast lookup; aggregates per user-turn via the new turn-change endpoint and waits for every child assistant to complete before showing the action
- Mark large+unrestorable files inline; show an "undone" badge and switch the action label to "Reapply" when the turn is undone
- Add a conflict confirm dialog (Skip and apply / Cancel) before retrying with force=true; on applied-with-skipped, persist a yellow notice on the changes block
- New i18n keys for unrestorable, undone, reapply, confirm dialog, and skipped notice (zh uses 爪印 voice: 不可撤回 / 已撤销 / 重新应用)
Two assistants editing the same file in one user turn (e.g. +161-126 then +31-20) used to undo as conflict because preflight checked every message's expected state against frozen real disk. Carry a per-path virtual state through the loop so each message's expected_current is matched against the prior step's target, exactly the scenario reported in #428.
- collapseRestoreFiles now keys by absolute path so two assistants
  writing different files with the same basename do not merge.
- aggregateTurnInternal switches to "any-state" availability so a
  partially-undone turn keeps both undo and redo actionable.
- mutateTurn no longer aborts when one actionable message fails;
  instead it appends to skipped[] and keeps going for the rest.
- When all surviving display rows collapse to white-busy after
  partial mutate, synthesize an applied result with empty files.
- Track the exact paths actually written and pass them to
  publishTurnChangeFiles so skipped files do not emit File events.

Adds three regression tests covering the basename collision,
mixed-state availability, and the mutatedPaths exclusion contract.
- mutateTurn now rolls back already-mutated assistants when force=false
  and a later actionable mutate fails. Previously a TOCTOU could leave
  the turn half-applied while the user saw a blocked toast.
- turnChangeFetch catches network errors and JSON parse failures, so a
  thrown HTTP error in mutateTurnChange no longer becomes an unhandled
  rejection and the user sees the standard blocked toast instead.
- Retry path for null-display fetch now stores its setTimeout in a per-
  key map cleared on cleanup, and keeps the cache key so a second event
  burst can no longer race a duplicate GET for the same turn.
- Stop mutating the response body in place when attaching skippedCount;
  spread into a fresh display object before storing.
@coderabbitai

coderabbitai Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 36 minutes and 59 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5034be82-f60e-4717-ab0a-1d14851e8ae4

📥 Commits

Reviewing files that changed from the base of the PR and between 9bcbc55 and cdce256.

📒 Files selected for processing (8)
  • packages/app/src/i18n/en.ts
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/opencode/src/session/turn-change.ts
  • packages/opencode/test/session/turn-change-aggregate.test.ts
  • packages/ui/src/components/session-turn.css
  • packages/ui/src/components/session-turn.tsx
  • packages/ui/src/i18n/en.ts
  • packages/ui/src/i18n/zh.ts
📝 Walkthrough

Walkthrough

This PR implements server-side aggregation of turn-level file changes to resolve issue #428. Previously, the UI only displayed changes from the last assistant message in a turn. The changes introduce a new turn-change aggregation service on the backend, new API endpoints to fetch/undo/redo aggregated changes, client-side turn-fetch helpers to coordinate multi-message operations, and enhanced UI to display cumulative changes with conflict resolution and force-apply capabilities.

Changes

Turn-Level Change Aggregation & Undo/Redo

Layer / File(s) Summary
Data Shape & Types
packages/opencode/src/session/turn-change.ts (lines 57–70), packages/app/src/pages/session/turn-change-fetch.ts
Introduced SkippedMessage type; expanded MutationResult to include optional skipped and mutatedPaths fields; added "rollback_failed" and "restore_missing" to blocked reasons. New turn-fetch types (TurnFetchAssistantLite, TurnFetchTarget, TurnFetchInput) enable deterministic signature generation for multi-message turn identification. Increased RESTORE_LIMIT from 2 MB to 20 MB.
Turn Aggregation Service
packages/opencode/src/session/turn-change.ts (lines 631–712, 808–990, 1012–1068)
Implemented aggregateTurnInternal to collapse restore data and compute cumulative file changes across all assistants under a user message. Added preflightTurn for virtual-state conflict/permission checking and mutateTurn for ordered mutation with optional rollback on failure. Extended TurnChange.Interface with aggregateTurn, aggregateTurnUndo, and aggregateTurnRedo methods; wired them into the service layer with existing per-session locking.
Server API Routes
packages/opencode/src/server/instance/session.ts (lines 39–46, 681–798)
Added GET /:sessionID/turn/:userMessageID/changes to fetch aggregated turn display; added POST .../undo and POST .../redo endpoints with optional { force: boolean } to mutate and publish file events limited to mutatedPaths. Updated publishTurnChangeFiles to filter by optional path set.
Client Turn-Fetch Helpers
packages/app/src/pages/session/turn-change-fetch.ts (lines 17–44)
Implemented turnFetchTargets to identify which parent-message groups are ready for fetch (all siblings completed) and turnFetchSignature to generate a stable signature for reactive refetching. Both functions operate deterministically on lite assistant records without external state.
Message Timeline Integration
packages/app/src/pages/session/message-timeline.tsx (lines 21–26, 363–562)
Updated turnChangeFetch to call new turn-level endpoints with optional force option and to handle conflict scenarios via confirmation dialog. Refactored prefetch effect to use turnFetchSignature and turnFetchTargets with managed retry timers. Extended callback signatures to pass userMessageID and options.
UI Component & Display
packages/ui/src/components/session-turn.tsx (lines 24, 163–173, 304–355, 506–639), packages/ui/src/components/session-turn-changes.ts (lines 23–34)
Updated SessionTurn to handle turnInProgress state and render aggregated changes with new "undone" and "unrestorable" indicators. Added skippedCount to TurnChangeDisplay. Wrapped action buttons in tooltips and added conflict confirmation flow with force-apply option.
Styling & Localization
packages/ui/src/components/session-turn.css, packages/app/src/i18n/en.ts, packages/app/src/i18n/zh.ts, packages/ui/src/i18n/en.ts, packages/ui/src/i18n/zh.ts
Compacted layout dimensions for turn-change header/rows/actions; added session.turnChange.blocked.rollbackFailed messaging (en/zh); added UI strings for unrestorable/undone/reapply/confirmation flows and skipped-files notice (en/zh). Adjusted spacing, icon sizes, and added new styling for state indicators.
Tests & Validation
packages/opencode/test/session/turn-change-aggregate.test.ts (+564 lines), packages/opencode/test/server/turn-change-aggregate-routes.test.ts (+189 lines), packages/app/test/session/turn-change-fetch.test.ts (+149 lines), packages/opencode/test/session/turn-change.test.ts (adjusted thresholds)
Comprehensive coverage for turnFetchTargets/turnFetchSignature determinism and target emission; aggregateTurn merging of multi-assistant displays and filtering of transient files; undo/redo behavior with conflict detection and force-apply skipping; HTTP endpoint routing and file event publishing. Updated size thresholds to 20 MB boundary.

Sequence Diagram

sequenceDiagram
    participant User
    participant Client as Client<br/>(MessageTimeline)
    participant Fetch as Turn-Fetch<br/>Module
    participant Server as Server<br/>(SessionRoutes)
    participant Service as TurnChange<br/>Service
    participant FileSystem as FileSystem

    User->>Client: View session turn with multiple assistant messages
    activate Client
    
    Client->>Fetch: turnFetchSignature(assistants)<br/>→ stable signature from completed targets
    Fetch->>Fetch: turnFetchTargets() groups by parentID,<br/>waits for all siblings completed
    Fetch-->>Client: signature updated
    
    Client->>Server: GET /session/:id/turn/:userMessageID/changes
    activate Server
    Server->>Service: aggregateTurn()
    activate Service
    Service->>Service: listAssistantsForUser()<br/>collapseRestoreFiles()
    Service->>Service: Merge displays across all assistants<br/>for userMessageID
    Service-->>Server: Aggregated TurnChangeDisplay
    deactivate Service
    Server-->>Client: Aggregated changes (all files, all assistants)
    deactivate Server
    
    Client->>Client: Render cumulative changes<br/>(no longer just last message)
    deactivate Client
    
    User->>Client: Click "Undo" on turn
    activate Client
    Client->>Server: POST /session/:id/turn/:userMessageID/changes/undo
    activate Server
    Server->>Service: aggregateTurnUndo()
    activate Service
    
    Service->>Service: preflightTurn()<br/>Check virtual file state, detect conflicts
    
    alt No conflicts
        Service->>Service: mutateTurn()<br/>Call undo for each ordered assistant
        Service->>FileSystem: Remove applied changes
        Service-->>Server: { status: "applied", mutatedPaths, skipped: [] }
    else Conflicts detected (force=false)
        Service-->>Server: { status: "blocked", reason: "conflict", skipped: [...] }
    else Force applied (force=true)
        Service->>Service: mutateTurn(force=true)<br/>Skip conflicted, undo safe changes
        Service->>FileSystem: Remove non-conflicted changes
        Service-->>Server: { status: "applied", mutatedPaths, skipped: [conflicts] }
    end
    
    deactivate Service
    Server->>Server: publishTurnChangeFiles(display, mutatedPaths)<br/>Emit only affected paths
    Server-->>Client: Mutation result with skipped/mutatedPaths
    deactivate Server
    
    Client->>Client: Update turnChanges state<br/>Recompute undoAvailable/redoAvailable<br/>Display skipped notices if any
    deactivate Client
    
    User->>User: Observes cumulative undo result<br/>with per-file conflict resolution
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • feat: track assistant turn changes #408 — Prior turn-change/undo-redo foundation work; this PR extends the same service with turn-level aggregation, expanded mutation outcomes, and client-side fetch coordination.

Suggested labels

bug, P1, app, ui

Poem

🐰 Across many whiskers, the turn now flows whole,
Each assistant's edits roll into a single scroll.
When conflicts arise, we ask "force ahead?"—
No more lonely last messages; cumulative threads instead!
Undo, redo, rewind—the whole turn takes flight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly summarizes the main change: aggregating per-turn changes across all assistants to fix the issue where only the last assistant's changes were shown.
Description check ✅ Passed The PR description is well-structured, covering all required template sections: Summary, Why, Related Issue, Human Review Status, Review Focus, Risk Notes, How To Verify, and a complete Checklist with all items addressed.
Linked Issues check ✅ Passed All coding objectives from issue #428 are met: the PR implements server-side aggregation to show cumulative changes across all assistants in a user turn [#428], adds undo/redo endpoints with proper semantics, includes comprehensive tests, and includes UI updates to render the aggregated display with proper error handling.
Out of Scope Changes check ✅ Passed All changes are directly related to issue #428 objectives: turn aggregation, restore limits, HTTP endpoints, tests, UI rendering, and i18n updates. No unrelated refactoring, dependencies, or generated files were introduced.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-428-aggregate-turn-changes

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
Review rate limit: 0/1 reviews remaining, refill in 36 minutes and 59 seconds.

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces aggregated turn changes, allowing users to undo or redo all assistant edits within a single user turn. It includes backend logic for preflight checks, conflict detection, and forced application, along with frontend updates to handle turn-level actions and display skipped files. A review comment points out that the rollback logic in mutateTurn silently suppresses errors during individual message mutations, which could lead to an inconsistent state.

Comment thread packages/opencode/src/session/turn-change.ts
Astro-Han added 4 commits May 4, 2026 22:17
…e parent

Previously the cache key was sessionID:userMessageID, so once the first
assistant under a turn completed and a GET fired, later siblings under
the same parent would not trigger a re-fetch and the aggregated display
silently dropped their changes.

The signature now incorporates each assistant id + completedAt, so any
new sibling completion produces a new key and triggers re-fetch.

Logic extracted to turn-change-fetch.ts so the cache decision can be
unit tested without SolidJS.
When mutateTurn aborts non-force, the per-message reverse rollback used
to swallow blocked results via .catch(() => undefined). If a rollback
itself failed (e.g. file changed externally between forward write and
reverse), the disk was left half-rolled-back but the response carried
the original failure's reason and files, hiding the inconsistency.

mutateTurn now collects dirty paths from each blocked reverse mutate and
returns reason "rollback_failed" with the affected files when any
rollback step did not complete. UI maps the new reason to a dedicated
toast that asks the user to inspect the listed files manually.
force-partial leaves a turn with both hasApplied and hasUndone. The
single inline button continues in the original direction (undo) by
design; the redo path for a mixed state is intentionally not exposed.
Recovery UX for mixed state is tracked as follow-up.
Reverse mutate can return blocked with files: [] from the catch branch
(unexpected throw) or from internal early returns like restore_missing.
The previous loop iterated over empty files, leaving dirty[] empty, so
the outer call returned the original blocked reason and hid the
rollback failure entirely.

Now a blocked reverse without file-level detail records a message-level
dirty entry, ensuring the outer call returns rollback_failed and the UI
warns the user about the partially-rolled-back message.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
packages/opencode/test/session/turn-change-aggregate.test.ts (1)

48-564: 🏗️ Heavy lift

Move this suite onto testEffect(...)/it.live(...).

These cases drive TurnChange’s Effect services against the real filesystem, so they should use the repo’s Effect-aware test harness instead of plain Bun tests. That avoids ad hoc runtime setup and keeps cleanup semantics aligned with the rest of the test suite.

As per coding guidelines, "Use testEffect(...) from test/lib/effect.ts for tests that exercise Effect services or Effect-based workflows" and "Use it.live(...) when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/session/turn-change-aggregate.test.ts` around lines 48
- 564, The test suite in turn-change-aggregate.test.ts exercises TurnChange's
effectful filesystem behavior; wrap the affected describe blocks (those calling
TurnChange.aggregateTurn, TurnChange.aggregateTurnUndo,
TurnChange.aggregateTurnRedo, TurnChange.undo, and any filesystem setup/cleanup)
with the repo's Effect-aware harness by converting tests to
testEffect(...).it.live(...) (importing testEffect from test/lib/effect.ts) so
they run under the Effect test harness and use it.live for real-OS behavior;
update each test declaration to use testEffect(...) and replace plain async
test(...) calls with it.live(...) (preserving the existing async bodies and
assertions) and remove any ad-hoc lifecycle so cleanup follows the effect
harness conventions.
packages/opencode/test/server/turn-change-aggregate-routes.test.ts (1)

54-189: 🏗️ Heavy lift

Use the Effect test harness for these live route cases.

This file exercises Effect-backed session/server flows with live filesystem and git state, so it should go through testEffect(...)/it.live(...) instead of raw Bun test(...). That keeps runtime wiring and cleanup consistent with the rest of packages/opencode/test.

As per coding guidelines, "Use testEffect(...) from test/lib/effect.ts for tests that exercise Effect services or Effect-based workflows" and "Use it.live(...) when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/server/turn-change-aggregate-routes.test.ts` around
lines 54 - 189, The two tests use live filesystem/git and should be converted to
the Effect test harness: replace Bun's test(...) with testEffect(...) or
it.live(...) from test/lib/effect.ts, import the helper(s) at the top, and
ensure the async test function body remains unchanged (including calls to
resetDatabase, tmpdir, Instance.provide, SessionNs.create, makeUser,
makeAssistant, TurnChange.recordWrite/finalize, and Server.Default().app and
app.request). Update the test declarations for both "GET aggregates two
assistant edits, undo applies and redo restores" and "POST undo without force is
blocked on conflict; force=true applies and reports skipped" to use the effect
harness so the runtime wiring/cleanup and live OS behavior (git, fs, child
processes) are handled correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/app/src/pages/session/message-timeline.tsx`:
- Around line 475-478: The overflow label is hardcoded as
"+{conflictPaths.length - 6} more" in the Show block and must be localized;
update the JSX inside the Show that renders "+{conflictPaths.length - 6} more"
to call the translation function (e.g. language.t) with a key like
"session.conflicts.more" and pass the count (conflictPaths.length - 6) as a
variable so the string is translated and pluralized correctly; locate the Show
block around the conflictPaths usage in message-timeline.tsx (the div with class
"px-3 py-1.5 text-12-regular text-text-weak border-t border-border-base") and
replace the hardcoded text with language.t(...) referencing the new i18n key.
- Around line 329-333: The pending turn-change retry timers in
turnChangeRetryTimers are only cleared on unmount via onCleanup, so retries can
fire after sessionID() has changed and target the wrong session; modify the
logic that schedules retries to also clear all timers whenever sessionID()
changes (e.g., add a reactive watcher/createEffect that iterates
turnChangeRetryTimers.values(), calls clearTimeout on each, and then
turnChangeRetryTimers.clear()), and apply the same change to the other retry
scheduling block referenced (the code around the retry callback used at the
other location) so no stale timers remain when sessionID() switches.

In `@packages/opencode/src/session/turn-change.ts`:
- Around line 650-667: collapseRestoreFiles is merging RestoreRow.data by
basename only, leaving displayPath ambiguous when different absolute paths share
the same basename; after building merged (Map keyed by row.data.path) run a
second disambiguation pass to recompute unique displayPath values across the
entire returned RestoreFile[] using absolute-path uniqueness for the whole user
turn (i.e., call the same uniqueness algorithm used by nextDisplayPath but
seeded with absolute paths of all merged files) and set each
RestoreFile.displayPath accordingly; apply the same second-pass disambiguation
to the analogous aggregation function around lines 697-710 so turn-level Changes
lists are unambiguous.
- Around line 849-865: In preflightTurn (in
packages/opencode/src/session/turn-change.ts) the blocked array currently turns
all non-permission failures into a generic "conflict" when building skipped
entries; change the logic so that if any blocked item has reason
"restore_unavailable" (i.e., target state is non-restorable/oversized/binary)
you immediately treat the entire turn as fatal and return/emit an
unsupported_size result instead of collapsing to "conflict" or continuing to
process other messages; adjust the same handling in the analogous block around
the code referenced at lines 894-897 so mutateTurn()/aggregate undo-redo cannot
partially apply changes when a non-restorable file is present. Ensure you
reference the blocked array, messageID/skipped construction, and
preflightTurn/mutateTurn locations when applying the change.

---

Nitpick comments:
In `@packages/opencode/test/server/turn-change-aggregate-routes.test.ts`:
- Around line 54-189: The two tests use live filesystem/git and should be
converted to the Effect test harness: replace Bun's test(...) with
testEffect(...) or it.live(...) from test/lib/effect.ts, import the helper(s) at
the top, and ensure the async test function body remains unchanged (including
calls to resetDatabase, tmpdir, Instance.provide, SessionNs.create, makeUser,
makeAssistant, TurnChange.recordWrite/finalize, and Server.Default().app and
app.request). Update the test declarations for both "GET aggregates two
assistant edits, undo applies and redo restores" and "POST undo without force is
blocked on conflict; force=true applies and reports skipped" to use the effect
harness so the runtime wiring/cleanup and live OS behavior (git, fs, child
processes) are handled correctly.

In `@packages/opencode/test/session/turn-change-aggregate.test.ts`:
- Around line 48-564: The test suite in turn-change-aggregate.test.ts exercises
TurnChange's effectful filesystem behavior; wrap the affected describe blocks
(those calling TurnChange.aggregateTurn, TurnChange.aggregateTurnUndo,
TurnChange.aggregateTurnRedo, TurnChange.undo, and any filesystem setup/cleanup)
with the repo's Effect-aware harness by converting tests to
testEffect(...).it.live(...) (importing testEffect from test/lib/effect.ts) so
they run under the Effect test harness and use it.live for real-OS behavior;
update each test declaration to use testEffect(...) and replace plain async
test(...) calls with it.live(...) (preserving the existing async bodies and
assertions) and remove any ad-hoc lifecycle so cleanup follows the effect
harness conventions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2f1654ba-a58f-4db9-8aab-d4a8c9d2a148

📥 Commits

Reviewing files that changed from the base of the PR and between 4615cc7 and 9bcbc55.

📒 Files selected for processing (15)
  • packages/app/src/i18n/en.ts
  • packages/app/src/i18n/zh.ts
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session/turn-change-fetch.ts
  • packages/app/test/session/turn-change-fetch.test.ts
  • packages/opencode/src/server/instance/session.ts
  • packages/opencode/src/session/turn-change.ts
  • packages/opencode/test/server/turn-change-aggregate-routes.test.ts
  • packages/opencode/test/session/turn-change-aggregate.test.ts
  • packages/opencode/test/session/turn-change.test.ts
  • packages/ui/src/components/session-turn-changes.ts
  • packages/ui/src/components/session-turn.css
  • packages/ui/src/components/session-turn.tsx
  • packages/ui/src/i18n/en.ts
  • packages/ui/src/i18n/zh.ts

Comment thread packages/app/src/pages/session/message-timeline.tsx Outdated
Comment thread packages/app/src/pages/session/message-timeline.tsx
Comment thread packages/opencode/src/session/turn-change.ts
Comment thread packages/opencode/src/session/turn-change.ts
Astro-Han added 2 commits May 4, 2026 23:36
- Cancel pending turn-change retry timers and clear fetched-turn cache when
  the active session changes, so a stale 500 ms retry from the previous
  session cannot land on a freshly opened one.
- Move the conflict dialog overflow label ("+N more") behind an i18n key
  (`ui.sessionTurn.turnChanges.confirmListMore`) for both en and zh.
- Add a second-pass disambiguation over aggregated restore files so two
  sibling assistants editing different opaque external paths that share a
  basename do not collide on `path: "config.json"` in the turn display.
- Treat `restore_unavailable` as fatal `unsupported_size` in
  `preflightTurn`, matching the single-message `mutate()` behavior so an
  aggregated undo/redo with one oversized/binary file blocks the whole
  turn instead of partially mutating other assistant messages with
  force=true.
- Cover the two backend cases with regression tests in
  `turn-change-aggregate.test.ts`.
@Astro-Han Astro-Han force-pushed the claude/issue-428-aggregate-turn-changes branch from 9bcbc55 to cdce256 Compare May 4, 2026 15:36
@Astro-Han Astro-Han merged commit 3b61dca into dev May 4, 2026
20 checks passed
@Astro-Han Astro-Han deleted the claude/issue-428-aggregate-turn-changes branch May 4, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] turn changes summary only shows last assistant message, not cumulative changes

1 participant