fix(session): delete reverted messages boundary-last and tie-break id…

…s by raw order Revert cleanup removed the boundary message first, so an interrupted cleanup left no marker and the next cleanup cleared the revert while resurrecting the remaining reverted messages. Delete messages and parts newest-first so the boundary is removed last and an interrupted cleanup stays resumable. The TUI and share page tie-broke same-millisecond messages with localeCompare, which disagrees with storage's SQLite BINARY collation (and with the TUI's own binary search) and can treat distinct ids as equal. Compare ids by code unit. Refs #42816 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
austinborn · Oct 1, 2026 · Oct 1, 2026 · Oct 1, 2026 · Oct 1, 2026 · 7cee47051653e73eea52126429bc613a27164001
commit 7cee47051653e73eea52126429bc613a27164001
diff --git a/packages/opencode/src/session/revert.ts b/packages/opencode/src/session/revert.ts
@@ -106,7 +106,10 @@ const layer = Layer.effect(
       const index = msgs.findIndex((msg) => msg.info.id === messageID)
       const target = index < 0 ? undefined : msgs[index]
       const remove = index < 0 ? [] : msgs.slice(index + (session.revert.partID ? 1 : 0))
-      for (const msg of remove) {
+      // Delete newest-first so the revert boundary is removed last. Each removal is its own
+      // durable write, so an interrupted cleanup must leave the boundary in place for the next
+      // cleanup to locate the remaining rows; otherwise clearRevert() would resurrect them.
+      for (const msg of remove.toReversed()) {
         yield* sessions.removeMessage({ sessionID, messageID: msg.info.id })
       }
       if (session.revert.partID && target) {
@@ -115,7 +118,7 @@ const layer = Layer.effect(
         if (idx >= 0) {
           const removeParts = target.parts.slice(idx)
           target.parts = target.parts.slice(0, idx)
-          for (const part of removeParts) {
+          for (const part of removeParts.toReversed()) {
             yield* sessions.removePart({ sessionID, messageID: target.info.id, partID: part.id })
           }
         }

diff --git a/packages/opencode/test/session/revert-compact.test.ts b/packages/opencode/test/session/revert-compact.test.ts
@@ -7,6 +7,7 @@ import path from "path"
 import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
 import { Effect } from "effect"
 import { Session } from "@/session/session"
+import { EventV2Bridge } from "@/event-v2-bridge"
 
 import { SessionRevert } from "../../src/session/revert"
 import { MessageV2 } from "../../src/session/message-v2"
@@ -19,7 +20,14 @@ import { ModelV2 } from "@opencode-ai/core/model"
 
 const it = testEffect(
   LayerNode.compile(
-    LayerNode.group([Session.node, SessionRevert.node, Snapshot.node, SessionProjector.node, CrossSpawnSpawner.node]),
+    LayerNode.group([
+      Session.node,
+      SessionRevert.node,
+      Snapshot.node,
+      SessionProjector.node,
+      CrossSpawnSpawner.node,
+      EventV2Bridge.node,
+    ]),
   ),
 )
 
@@ -438,6 +446,96 @@ describe("revert + compact workflow", () => {
     ),
   )
 
+  it.live(
+    "cleanup removes the revert boundary last so an interrupted cleanup stays resumable",
+    provideTmpdirInstance(
+      (dir) =>
+        Effect.gen(function* () {
+          const session = yield* Session.Service
+          const revert = yield* SessionRevert.Service
+          const events = yield* EventV2Bridge.Service
+
+          const info = yield* session.create({})
+          const sid = info.id
+
+          const u1 = yield* user(sid)
+          const u2 = yield* user(sid)
+          const a2 = yield* assistant(sid, u2.id, dir)
+          const u3 = yield* user(sid)
+
+          const removed: string[] = []
+          const unsubscribe = yield* events.listen((event) => {
+            if (event.type === SessionV1.Event.MessageRemoved.type)
+              removed.push((event.data as typeof SessionV1.Event.MessageRemoved.data.Type).messageID)
+            if (event.type === SessionV1.Event.PartRemoved.type)
+              removed.push((event.data as typeof SessionV1.Event.PartRemoved.data.Type).partID)
+            return Effect.void
+          })
+          yield* Effect.addFinalizer(() => unsubscribe)
+
+          yield* session.setRevert({
+            sessionID: sid,
+            revert: { messageID: u2.id },
+            summary: { additions: 0, deletions: 0, files: 0 },
+          })
+          yield* revert.cleanup(yield* session.get(sid))
+          expect(removed).toEqual([u3.id, a2.id, u2.id])
+          expect((yield* session.messages({ sessionID: sid })).map((msg) => msg.info.id)).toEqual([u1.id])
+
+          const other = yield* session.create({})
+          const o1 = yield* user(other.id)
+          const q1 = yield* text(other.id, o1.id, "first part")
+          const q2 = yield* text(other.id, o1.id, "second part")
+          const q3 = yield* text(other.id, o1.id, "third part")
+          removed.length = 0
+          yield* session.setRevert({
+            sessionID: other.id,
+            revert: { messageID: o1.id, partID: q2.id },
+            summary: { additions: 0, deletions: 0, files: 0 },
+          })
+          yield* revert.cleanup(yield* session.get(other.id))
+          expect(removed).toEqual([q3.id, q2.id])
+          expect((yield* session.messages({ sessionID: other.id }))[0]?.parts.map((part) => part.id)).toEqual([q1.id])
+        }),
+      { git: true },
+    ),
+  )
+
+  it.live(
+    "cleanup resumes after an interruption that left the revert boundary in place",
+    provideTmpdirInstance(
+      (dir) =>
+        Effect.gen(function* () {
+          const session = yield* Session.Service
+          const revert = yield* SessionRevert.Service
+
+          const info = yield* session.create({})
+          const sid = info.id
+
+          const u1 = yield* user(sid)
+          const u2 = yield* user(sid)
+          const a2 = yield* assistant(sid, u2.id, dir)
+          const u3 = yield* user(sid)
+
+          yield* session.setRevert({
+            sessionID: sid,
+            revert: { messageID: u2.id },
+            summary: { additions: 0, deletions: 0, files: 0 },
+          })
+          // Newest-first deletion interrupted after its first removal.
+          yield* session.removeMessage({ sessionID: sid, messageID: u3.id })
+
+          yield* revert.cleanup(yield* session.get(sid))
+
+          const ids = (yield* session.messages({ sessionID: sid })).map((msg) => msg.info.id)
+          expect(ids).toEqual([u1.id])
+          expect(ids).not.toContain(a2.id)
+          expect((yield* session.get(sid)).revert).toBeUndefined()
+        }),
+      { git: true },
+    ),
+  )
+
   it.live(
     "reverts chronological suffixes on both sides of mixed message ID ordering",
     provideTmpdirInstance(

diff --git a/packages/tui/src/context/sync.tsx b/packages/tui/src/context/sync.tsx
@@ -51,8 +51,10 @@ function search<T>(items: T[], target: string, key: (item: T) => string) {
   return { found: false, index: left }
 }
 
-function compareMessage(a: Message, b: Message) {
-  return a.time.created - b.time.created || a.id.localeCompare(b.id)
+// Tie-break by code unit order to match storage (SQLite BINARY collation) and search() below;
+// localeCompare can order same-millisecond ids differently and treats some distinct ids as equal.
+export function compareMessage(a: Message, b: Message) {
+  return a.time.created - b.time.created || (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)
 }
 
 const messageKey = (message: Message) => message.time.created + message.id
@@ -170,7 +172,7 @@ export const {
     function listSessions() {
       return sdk.client.session
         .list({ start: Date.now() - 30 * 24 * 60 * 60 * 1000, ...sessionListQuery() })
-        .then((x) => (x.data ?? []).toSorted((a, b) => a.id.localeCompare(b.id)))
+        .then((x) => (x.data ?? []).toSorted((a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)))
     }
 
     event.subscribe((event, { directory, workspace }) => {

diff --git a/packages/tui/test/context/sync.test.ts b/packages/tui/test/context/sync.test.ts
@@ -0,0 +1,26 @@
+import { expect, test } from "bun:test"
+import type { Message } from "@opencode-ai/sdk/v2"
+import { compareMessage } from "../../src/context/sync"
+
+const message = (id: string, created: number) => ({ id, time: { created } }) as Message
+
+test("orders messages by creation time, then by raw id like storage", () => {
+  const upper = message("msg_A1", 1)
+  const lower = message("msg_a1", 1)
+  const earlier = message("msg_z9", 0)
+  const later = message("msg_00", 2)
+  const expected = [earlier, upper, lower, later].map((item) => item.id)
+
+  // Storage uses SQLite BINARY collation, where "A" < "a"; locale collation puts "a" first.
+  expect([later, lower, earlier, upper].toSorted(compareMessage).map((item) => item.id)).toEqual(expected)
+  expect([upper, later, lower, earlier].toSorted(compareMessage).map((item) => item.id)).toEqual(expected)
+})
+
+test("never treats distinct ids as equal", () => {
+  // Canonically equivalent under locale collation, but distinct primary keys.
+  const composed = message("msg_é", 1)
+  const decomposed = message("msg_é", 1)
+
+  expect(compareMessage(composed, decomposed)).not.toBe(0)
+  expect(compareMessage(composed, decomposed)).toBe(-compareMessage(decomposed, composed))
+})
diff --git a/packages/web/src/components/Share.tsx b/packages/web/src/components/Share.tsx
@@ -76,7 +76,9 @@ export default function Share(props: {
     messages: {},
   })
   const messages = createMemo(() =>
-    Object.values(store.messages).toSorted((a, b) => a.time.created - b.time.created || a.id.localeCompare(b.id)),
+    Object.values(store.messages).toSorted(
+      (a, b) => a.time.created - b.time.created || (a.id < b.id ? -1 : a.id > b.id ? 1 : 0),
+    ),
   )
   const [connectionStatus, setConnectionStatus] = createSignal<[Status, string?]>(["disconnected"])