diff --git a/packages/opencode/src/cli/cmd/tui/context/sync.tsx b/packages/opencode/src/cli/cmd/tui/context/sync.tsx index a24660d5c..d4699d3a1 100644 --- a/packages/opencode/src/cli/cmd/tui/context/sync.tsx +++ b/packages/opencode/src/cli/cmd/tui/context/sync.tsx @@ -157,6 +157,85 @@ export function bucketMessages( return out } +/** + * Client-side mirror of MessageV2.compare — chronological order of two messages. + * + * Message ids are NOT a clock. The id encoder packs `Date.now() * 0x1000 + counter` + * into 6 bytes, so the sortable prefix wraps every 2^36 ms (~2.18 years); the last + * boundary was 2026-08-14T11:19:55Z. Across a wrap a NEWER message gets a SMALLER + * id, so `a.id < b.id` inverts for any session whose history straddles it. + * + * This matters more here than it looks: the store keeps each bucket sorted and uses + * Binary.search over that order both to locate an existing message and to pick the + * splice index for a new one. With ids as the key, a fresh message lands at index 0 + * — which is what put new messages at the top of the transcript — and the >100 + * trim then treats `list[0]` as "oldest" and deletes it along with its parts. + * + * `time.created` is an independent field, unaffected by the packing, so it is the + * primary key; the id only breaks ties inside the same millisecond. + */ +export function compareMessages( + a: { id: string; time: { created: number } }, + b: { id: string; time: { created: number } }, +) { + return a.time.created !== b.time.created ? a.time.created - b.time.created : a.id < b.id ? -1 : a.id > b.id ? 1 : 0 +} + +/** + * Compare a message against a MARKER id (revert point, pending watermark) held by + * the session rather than by a message object. + * + * Returns `< 0` if `msg` precedes the marker, `> 0` if it follows, `0` if it IS the + * marker. When the marker is not present in `list` there is no anchor to compare + * against and the caller gets `undefined` — callers decide what that means rather + * than silently treating the marker as position 0 or infinity. + * + * Resolving the marker through the list is what makes this wrap-safe: the marker id + * alone carries no time, and comparing id strings inverts across a wrap boundary + * (see compareMessages). + */ +export function compareToMarker( + list: M[], + msg: M, + markerID: string, +): number | undefined { + const marker = list.find((m) => m.id === markerID) + if (!marker) return undefined + return compareMessages(msg, marker) +} + +/** + * Locate `id` in a chronologically-sorted message list, or the index it should be + * inserted at. Same contract as Binary.search, but keyed on (time.created, id) — + * Binary.search compares id strings only and cannot express this order. + * + * `probe` is the incoming message when inserting. Lookups that only have an id + * (message.removed) pass none and fall back to a linear scan: without a time there + * is nothing to bisect on, and a wrong bisect here would splice out an unrelated + * message. + */ +export function searchMessages( + list: M[], + id: string, + probe?: { id: string; time: { created: number } }, +): { found: boolean; index: number } { + if (!probe) { + const index = list.findIndex((m) => m.id === id) + return index === -1 ? { found: false, index: list.length } : { found: true, index } + } + let left = 0 + let right = list.length + while (left < right) { + const mid = Math.floor((left + right) / 2) + if (compareMessages(list[mid], probe) < 0) left = mid + 1 + else right = mid + } + // `left` is the first element not ordered before probe — an exact id match can + // only be there, since compare is a total order and probe carries its own id. + if (left < list.length && list[left].id === id) return { found: true, index: left } + return { found: false, index: left } +} + /** * A `session.status` event is authoritative for the WHOLE status object. * @@ -193,7 +272,7 @@ export function nextSessionStatus(status: SessionStatus) { // read-only local-DB snapshot and they drift — this arm's population grew // 1294 → 1313 across this branch's own revisions — so trust the split's shape, // not the absolute numbers. -export function selectMessages( +export function selectMessages( buckets: Record | undefined, agentID: string, sessionID: string, @@ -202,7 +281,10 @@ export function selectMessages( if (buckets?.[sessionID]?.length) return buckets[sessionID] const newest = Object.entries(buckets ?? {}) .filter(([key, msgs]) => key !== "main" && msgs.length > 0) - .sort(([, a], [, b]) => (b.at(-1)?.id ?? "").localeCompare(a.at(-1)?.id ?? "")) + // "Newest bucket" by its last message's time, not its id — an id compare + // inverts across an id wrap (see compareMessages) and would pick the bucket + // that happens to hold a pre-wrap tail. + .sort(([, a], [, b]) => (b.at(-1)?.time.created ?? 0) - (a.at(-1)?.time.created ?? 0)) .at(0) return newest?.[1] ?? [] } @@ -567,7 +649,7 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({ break } const messages = store.message[sid][aid] - const result = Binary.search(messages, event.properties.info.id, (m) => m.id) + const result = searchMessages(messages, event.properties.info.id, event.properties.info) if (result.found) { setStore("message", sid, aid, result.index, reconcile(event.properties.info)) break @@ -608,7 +690,7 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({ if (!buckets) break for (const aid of Object.keys(buckets)) { const messages = buckets[aid] - const result = Binary.search(messages, event.properties.messageID, (m) => m.id) + const result = searchMessages(messages, event.properties.messageID) if (result.found) { setStore( "message", @@ -1005,8 +1087,16 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({ draft.todo[sessionID] = todo.data ?? [] draft.task[sessionID] = task.data ?? [] const flat = (messages.data ?? []).map((x) => x.info) - // Server returns messages id-ordered and message.updated keeps that order; the footer's post-/rebuild pending-detection deliberately does NOT depend on it (it keys off checkpoint coveredUpTo, model.ts), so reordering here won't resurface the stale-context bug. - draft.message[sessionID] = bucketMessages(flat) + // Server returns messages in (time_created, id) order and + // message.updated keeps that order via searchMessages. Sorting here + // too is belt-and-braces: it makes the store's invariant hold from + // its own code rather than from a property of the endpoint, so a + // paging/ordering change server-side can't silently corrupt the + // splice indices the trim and message.removed both rely on. The + // footer's post-/rebuild pending-detection deliberately does NOT + // depend on this order (it keys off checkpoint coveredUpTo, model.ts), + // so ordering here won't resurface the stale-context bug. + draft.message[sessionID] = bucketMessages(flat.toSorted(compareMessages)) for (const message of messages.data ?? []) { draft.part[message.info.id] = message.parts } diff --git a/packages/opencode/src/cli/cmd/tui/routes/session/index.tsx b/packages/opencode/src/cli/cmd/tui/routes/session/index.tsx index 788132fc2..bdfeeb918 100644 --- a/packages/opencode/src/cli/cmd/tui/routes/session/index.tsx +++ b/packages/opencode/src/cli/cmd/tui/routes/session/index.tsx @@ -15,7 +15,7 @@ import { Dynamic } from "solid-js/web" import path from "path" import { useCurrentAgentID, useRoute, useRouteData } from "@tui/context/route" import { useProject } from "@tui/context/project" -import { selectMessages, useSync } from "@tui/context/sync" +import { compareMessages, compareToMarker, selectMessages, useSync } from "@tui/context/sync" import { useEvent } from "@tui/context/event" import { SplitBorder } from "@tui/component/border" import { Spinner } from "@tui/component/spinner" @@ -188,8 +188,11 @@ export function Session() { ) const disabled = createMemo(() => permissions().length > 0 || questions().length > 0) + // The in-flight assistant message itself, not just its id: `queued` compares + // against it chronologically (see compareMessages) and an id compare inverts + // across an id wrap, which would mark settled messages as queued. const pending = createMemo(() => { - return messages().findLast((x) => x.role === "assistant" && !x.time.completed)?.id + return messages().findLast((x) => x.role === "assistant" && !x.time.completed) }) const lastAssistant = createMemo(() => { @@ -733,7 +736,17 @@ export function Session() { const status = sync.data.session_status?.[route.sessionID] if (status?.type !== "idle") await sdk.client.session.abort({ sessionID: route.sessionID }).catch(() => {}) const revert = session()?.revert?.messageID - const message = messages().findLast((x) => (!revert || x.id < revert) && x.role === "user") + const all = messages() + // Ordered against the revert marker via compareToMarker, not by raw id: an + // id compare inverts across an id wrap and would undo the wrong turn. + // An unresolvable marker (outside the loaded window) yields undefined, which + // excludes the message rather than defaulting it into the "before" side. + const before = (x: (typeof all)[number]) => { + if (!revert) return true + const rel = compareToMarker(all, x, revert) + return rel !== undefined && rel < 0 + } + const message = all.findLast((x) => x.role === "user" && before(x)) if (!message) return void sdk.client.session .revert({ @@ -772,7 +785,13 @@ export function Session() { dialog.clear() const messageID = session()?.revert?.messageID if (!messageID) return - const message = messages().find((x) => x.role === "user" && x.id > messageID) + const all = messages() + // Ordered via compareToMarker, not by raw id (inverts across an id wrap). + const message = all.find((x) => { + if (x.role !== "user") return false + const rel = compareToMarker(all, x, messageID) + return rel !== undefined && rel > 0 + }) if (!message) { void sdk.client.session.unrevert({ sessionID: route.sessionID, @@ -1019,9 +1038,14 @@ export function Session() { category: "session", onSelect: (dialog) => { const revertID = session()?.revert?.messageID - const lastAssistantMessage = messages().findLast( - (msg) => msg.role === "assistant" && (!revertID || msg.id < revertID), - ) + const all = messages() + // Ordered via compareToMarker, not by raw id (inverts across an id wrap). + const lastAssistantMessage = all.findLast((msg) => { + if (msg.role !== "assistant") return false + if (!revertID) return true + const rel = compareToMarker(all, msg, revertID) + return rel !== undefined && rel < 0 + }) if (!lastAssistantMessage) { toast.show({ message: "No assistant messages found", variant: "error" }) dialog.clear() @@ -1225,7 +1249,13 @@ export function Session() { const revertRevertedMessages = createMemo(() => { const messageID = revertMessageID() if (!messageID) return [] - return messages().filter((x) => x.id >= messageID && x.role === "user") + const all = messages() + // Ordered via compareToMarker, not by raw id (inverts across an id wrap). + return all.filter((x) => { + if (x.role !== "user") return false + const rel = compareToMarker(all, x, messageID) + return rel !== undefined && rel >= 0 + }) }) const revert = createMemo(() => { @@ -1405,7 +1435,7 @@ export function Session() { ) })()} - = revert()!.messageID}> + = 0}> <> @@ -1521,7 +1551,7 @@ function UserMessage(props: { parts: Part[] onMouseUp: () => void index: number - pending?: string + pending?: { id: string; time: { created: number } } }) { const ctx = use() const local = useLocal() @@ -1562,7 +1592,7 @@ function UserMessage(props: { const { theme } = useTheme() const t = useLanguage().t const [hover, setHover] = createSignal(false) - const queued = createMemo(() => props.pending && props.message.id > props.pending) + const queued = createMemo(() => !!props.pending && compareMessages(props.message, props.pending) > 0) const color = createMemo(() => local.agent.color(props.message.agent)) const queuedFg = createMemo(() => selectedForeground(theme, color())) const metadataVisible = createMemo(() => queued() || ctx.showTimestamps()) diff --git a/packages/opencode/src/history/backfill.ts b/packages/opencode/src/history/backfill.ts index f7314ece1..1afc0ac5b 100644 --- a/packages/opencode/src/history/backfill.ts +++ b/packages/opencode/src/history/backfill.ts @@ -1,5 +1,5 @@ import { Context, Effect, Layer } from "effect" -import { and, asc, desc, eq, gt, sql } from "drizzle-orm" +import { and, asc, desc, eq, gt, or, sql } from "drizzle-orm" import { Database } from "../storage" import { Config } from "../config" import { PartTable, SessionTable } from "../session/session.sql" @@ -49,7 +49,12 @@ function scanSession( enabled: ReadonlySet, ) { return Effect.gen(function* () { - let cursor = "" + // Composite (time_created, id) cursor. Part ids share the message-id clock, + // which wraps every ~2.18 years (see MessageV2.compare), so a bare + // `gt(id, cursor)` walk stops dead at a wrap boundary: every post-wrap part + // has a SMALLER id than the pre-wrap tail and would never be visited, leaving + // the newest history permanently unindexed. + let cursor: { time: number; id: string } | undefined while (true) { const parts = Database.use((db) => db @@ -58,18 +63,26 @@ function scanSession( .where( and( eq(PartTable.session_id, session.id as any), - gt(PartTable.id, cursor as any), + ...(cursor + ? [ + or( + gt(PartTable.time_created, cursor.time), + and(eq(PartTable.time_created, cursor.time), gt(PartTable.id, cursor.id as any)), + )!, + ] + : []), sql`NOT EXISTS (SELECT 1 FROM history_fts WHERE history_fts.part_id = ${PartTable.id})`, ), ) - .orderBy(asc(PartTable.id)) + .orderBy(asc(PartTable.time_created), asc(PartTable.id)) .limit(BATCH) .all(), ) if (parts.length === 0) return yield* writeBatch(parts, session.project_id, resolver, enabled) - cursor = parts[parts.length - 1]!.id + const last = parts[parts.length - 1]! + cursor = { time: last.time_created, id: last.id } yield* Effect.sleep("10 millis") } }) diff --git a/packages/opencode/src/session/classify.ts b/packages/opencode/src/session/classify.ts index 48d251ba8..b7ae8e1c1 100644 --- a/packages/opencode/src/session/classify.ts +++ b/packages/opencode/src/session/classify.ts @@ -71,7 +71,7 @@ export function classifyAssistantStep(input: { if ( assistant.finish === "tool-calls" && !assistant.error && - input.lastUser.id < assistant.id && + MessageV2.compare(input.lastUser, assistant) < 0 && !input.parts.some((part) => part.type === "tool") && input.parts.some( (part) => @@ -87,7 +87,13 @@ export function classifyAssistantStep(input: { if (assistant.finish === "tool-calls") return { type: "continue" } // 4. Stale assistant predating the current user turn — don't terminate on it. - if (input.phase === "existing-assistant" && !(input.lastUser.id < assistant.id)) + // Ordered by (time.created, id) via MessageV2.compare, not by raw id: message + // ids encode a 48-bit timestamp that wraps every ~2.18 years, so across a wrap + // a NEWER assistant gets a SMALLER id and a bare compare calls a fresh reply + // "stale" (or a genuinely stale one fresh). Both misjudgements are load-bearing + // here and at #3a — this is the same guard prompt.ts applies before + // auto-continue, and it must agree with it. + if (input.phase === "existing-assistant" && !(MessageV2.compare(input.lastUser, assistant) < 0)) return { type: "continue" } // 5. Errored step — checked before content so an errored message that also diff --git a/packages/opencode/src/session/message-v2.ts b/packages/opencode/src/session/message-v2.ts index 949bae61f..578e6f7f0 100644 --- a/packages/opencode/src/session/message-v2.ts +++ b/packages/opencode/src/session/message-v2.ts @@ -607,6 +607,27 @@ const part = (row: typeof PartTable.$inferSelect) => const older = (row: Cursor) => or(lt(MessageTable.time_created, row.time), and(eq(MessageTable.time_created, row.time), lt(MessageTable.id, row.id))) +/** + * Chronological order of two messages: `< 0` if a precedes b, `> 0` if a follows + * b, `0` if they are the same message. + * + * Message IDs are NOT a usable clock. `Identifier.create` packs + * `Date.now() * 0x1000 + counter` into 6 bytes, so the sortable prefix wraps + * every 2^36 ms (~2.18 years); the last wrap was 2026-08-14 12:39:55 UTC and the + * next is ~Oct 2028. Across a wrap, a NEWER message gets a SMALLER id — post-wrap + * ids restart near `msg_000…` while pre-wrap ids sit near `msg_fff…`. Any bare + * `a.id > b.id` therefore inverts for every session whose history straddles a + * boundary, which silently wedged the session loop (a fresh user prompt sorted + * before all history, so the loop saw no new work and exited at step 0). + * + * `time.created` is an independent integer column and is not affected, so it is + * the primary key of the order; the id only breaks ties inside the same + * millisecond, which is exactly the ambiguity the counter was added to resolve. + */ +export function compare(a: { id: string; time: { created: number } }, b: { id: string; time: { created: number } }) { + return a.time.created !== b.time.created ? a.time.created - b.time.created : a.id < b.id ? -1 : a.id > b.id ? 1 : 0 +} + function hydrate(rows: (typeof MessageTable.$inferSelect)[]) { const ids = rows.map((row) => row.id) const partByMessage = new Map() diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 1696ecb37..578803f63 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -3283,13 +3283,27 @@ NOTE: At any point in time through this workflow you should feel free to ask the } if (!lastUser) throw new Error("No user message found in stream. This should never happen.") + // Deliberately NOT MessageV2.compare, and not a raw id compare either. + // + // `msgs` comes from filterCompacted (message-v2.ts:1077), which walks + // newest-first and STOPS at the first checkpoint/compaction marker, then + // reverses. So the slice contains at most one marker and it is always + // msgs[0] — the boundary the slice starts at. "A marker exists in this + // slice" is therefore exactly "this turn's context was already rebuilt", + // with no ordering question to get wrong. + // + // The two orderings that look plausible here are both wrong: a position + // compare against lastFinished never fires (the marker precedes every + // message in the slice, so nothing follows it), and a time compare never + // fires either, because the marker carries the SYNTHETIC time + // `boundary.time.created + 1` (checkpoint.ts:1599) and so sorts earlier + // than lastFinished rather than later. The original `id > lastFinished.id` + // only worked by accident — a freshly minted marker id happens to sort + // highest, which stops being true across an id wrap (see MessageV2.compare). + // Either mistake un-guards the overflow path and rebuilds the turn twice. const usageRecovered = !!lastFinished && - msgs.some( - (msg) => - msg.info.id > lastFinished.id && - msg.parts.some((part) => part.type === "checkpoint" || part.type === "compaction"), - ) + msgs.some((msg) => msg.parts.some((part) => part.type === "checkpoint" || part.type === "compaction")) // Per-user-message active recall reminder. Once the session has // any memory artifacts (memory dir populated OR tasks recorded), @@ -3340,7 +3354,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the if ( lastAssistant?.finish === "length" && !hasToolCalls && - lastUser.id < lastAssistant.id && + MessageV2.compare(lastUser, lastAssistant) < 0 && (yield* autoContinueOutputLength({ lastUser, assistant: lastAssistant })) ) { continue @@ -3720,7 +3734,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the if (step > 1 && lastFinished) { for (const m of msgs) { - if (m.info.role !== "user" || m.info.id <= lastFinished.id) continue + if (m.info.role !== "user" || MessageV2.compare(m.info, lastFinished) <= 0) continue for (const p of m.parts) { if (p.type !== "text" || p.ignored || p.synthetic) continue if (!p.text.trim()) continue @@ -3774,8 +3788,16 @@ NOTE: At any point in time through this workflow you should feel free to ask the .pipe(Effect.ignore) return "break" as const } + // Resolve the watermark to its message so the comparison can use + // (time.created, id) — a bare id compare inverts across an id wrap + // (see MessageV2.compare). If the watermark message is no longer in + // the list, fall back to the agent_id check alone, which the + // ForkContext.watermarkMsgID JSDoc documents as sufficient on its own. + const watermarkMsg = msgs.find((m) => m.info.id === forkCtx.watermarkMsgID)?.info const ownNew = msgs.filter( - (m) => m.info.id > forkCtx.watermarkMsgID && m.info.agentID === lastUser.agentID, + (m) => + m.info.agentID === lastUser.agentID && + (!watermarkMsg || MessageV2.compare(m.info, watermarkMsg) > 0), ) const ownNewModelMsgs = yield* MessageV2.toModelMessagesEffect(ownNew, model) const prebuiltSystem = forkCtx.system diff --git a/packages/opencode/src/session/revert.ts b/packages/opencode/src/session/revert.ts index 0387e213a..c1dae7cf8 100644 --- a/packages/opencode/src/session/revert.ts +++ b/packages/opencode/src/session/revert.ts @@ -74,7 +74,8 @@ export const layer = Layer.effect( if (session.revert?.snapshot) yield* snap.restore(session.revert.snapshot) yield* snap.revert(patches) if (rev.snapshot) rev.diff = yield* snap.diff(rev.snapshot as string) - const range = all.filter((msg) => msg.info.id >= rev!.messageID) + const revertPoint = all.find((msg) => msg.info.id === rev!.messageID)?.info + const range = revertPoint ? all.filter((msg) => MessageV2.compare(msg.info, revertPoint) >= 0) : [] const diffs = yield* summary.computeDiff({ messages: range }) yield* storage.write(["session_diff", input.sessionID], diffs).pipe(Effect.ignore) yield* bus.publish(Session.Event.Diff, { sessionID: input.sessionID, diff: diffs }) @@ -107,9 +108,16 @@ export const layer = Layer.effect( const messageID = session.revert.messageID const remove = [] as MessageV2.WithParts[] let target: MessageV2.WithParts | undefined - for (const msg of msgs) { - if (msg.info.id < messageID) continue - if (msg.info.id > messageID) { + // Order by (time.created, id): a bare id compare inverts across an id wrap + // (see MessageV2.compare), and this loop DELETES everything it classifies as + // "after" the revert point — an inverted compare would delete the session's + // history instead of the reverted tail. If the revert point is missing from + // the list there is nothing to anchor on, so remove nothing. + const anchor = msgs.find((msg) => msg.info.id === messageID)?.info + for (const msg of anchor ? msgs : []) { + const rel = MessageV2.compare(msg.info, anchor!) + if (rel < 0) continue + if (rel > 0) { remove.push(msg) continue } diff --git a/packages/opencode/src/session/session.ts b/packages/opencode/src/session/session.ts index 319436b9d..ecd6d2a05 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -664,9 +664,15 @@ export const layer: Layer.Layer() + // Fork cuts the history at input.messageID. Compare on (time.created, id) — + // a bare id compare inverts across an id wrap (see MessageV2.compare), which + // would either truncate at message 0 (empty fork) or never trigger (full + // copy). `messages` already returns chronological order, so the break is + // still a single forward scan. + const cutoff = input.messageID ? msgs.find((m) => m.info.id === input.messageID)?.info : undefined for (const msg of msgs) { - if (input.messageID && msg.info.id >= input.messageID) break + if (cutoff && MessageV2.compare(msg.info, cutoff) >= 0) break const newID = MessageID.ascending() idMap.set(msg.info.id, newID) @@ -811,7 +817,11 @@ export const layer: Layer.Layer) => ({ ...x, ...extra }) as any + +describe("TUI message ordering across an id wraparound", () => { + test("the premise: id order is inverted for these ids", () => { + expect(AUG_14.id < AUG_06.id).toBe(true) + expect(AUG_14.id < JUL_10.id).toBe(true) + }) + + test("compareMessages orders them chronologically", () => { + expect(compareMessages(JUL_10, AUG_06)).toBeLessThan(0) + expect(compareMessages(AUG_06, AUG_14)).toBeLessThan(0) + expect(compareMessages(AUG_14, JUL_10)).toBeGreaterThan(0) + expect([AUG_14, JUL_10, AUG_06].toSorted(compareMessages).map((x) => x.time.created)).toEqual([ + 1783687705445, 1786019107358, 1786713700254, + ]) + }) + + test("id still breaks ties inside one millisecond", () => { + const a = { id: "msg_000aaa", time: { created: 5 } } + const b = { id: "msg_000bbb", time: { created: 5 } } + expect(compareMessages(a, b)).toBeLessThan(0) + expect(compareMessages(b, a)).toBeGreaterThan(0) + expect(compareMessages(a, { ...a })).toBe(0) + }) + + // The bug that put new messages at the top of the transcript: the store keeps + // each bucket sorted and asks searchMessages where an incoming message goes. + describe("searchMessages picks the append index, not index 0", () => { + const history = [m(JUL_10), m(AUG_06)] + + test("a post-wrap message appends to the end", () => { + expect(searchMessages(history, AUG_14.id, AUG_14)).toEqual({ found: false, index: 2 }) + }) + + test("an existing message is found at its own index", () => { + expect(searchMessages(history, AUG_06.id, AUG_06)).toEqual({ found: true, index: 1 }) + expect(searchMessages(history, JUL_10.id, JUL_10)).toEqual({ found: true, index: 0 }) + }) + + test("an already-inserted post-wrap message is found, not duplicated", () => { + const withNew = [...history, m(AUG_14)] + expect(searchMessages(withNew, AUG_14.id, AUG_14)).toEqual({ found: true, index: 2 }) + }) + + test("id-only lookup (message.removed) finds by identity", () => { + const withNew = [...history, m(AUG_14)] + expect(searchMessages(withNew, AUG_14.id)).toEqual({ found: true, index: 2 }) + expect(searchMessages(withNew, "msg_absent")).toEqual({ found: false, index: 3 }) + }) + + // Regression guard for the data loss: the >100 trim drops list[0] and deletes + // its parts. Appending correctly is what keeps list[0] the genuinely oldest + // message; inserting at 0 made the trim delete the newest message instead. + test("repeated post-wrap appends keep the oldest message at index 0", () => { + const list = [m(JUL_10), m(AUG_06)] + for (let i = 0; i < 5; i++) { + const next = { id: `msg_0006f7687${i}`, time: { created: 1786713700254 + i } } + const at = searchMessages(list, next.id, next) + expect(at.found).toBe(false) + list.splice(at.index, 0, m(next)) + } + expect(list[0].id).toBe(JUL_10.id) + expect(list.at(-1)!.time.created).toBe(1786713700258) + expect(list.map((x) => x.time.created)).toEqual([...list].map((x) => x.time.created).toSorted((a, b) => a - b)) + }) + }) + + describe("compareToMarker resolves a session-held marker id", () => { + const list = [m(JUL_10), m(AUG_06), m(AUG_14)] + + test("orders messages around a post-wrap revert marker", () => { + expect(compareToMarker(list, m(JUL_10), AUG_14.id)).toBeLessThan(0) + expect(compareToMarker(list, m(AUG_14), AUG_14.id)).toBe(0) + }) + + test("orders messages around a pre-wrap revert marker", () => { + // A raw `id >= marker` compare calls the Aug-14 message "before" the Aug-06 + // marker, which is how /undo and the reverted-message strikethrough drifted. + expect(compareToMarker(list, m(AUG_14), AUG_06.id)).toBeGreaterThan(0) + expect(compareToMarker(list, m(JUL_10), AUG_06.id)).toBeLessThan(0) + }) + + test("returns undefined when the marker is outside the loaded window", () => { + expect(compareToMarker(list, m(AUG_14), "msg_notloaded")).toBeUndefined() + }) + }) + + test("selectMessages picks the newest bucket by time, not by id", () => { + // general-2's tail is post-wrap (smaller id, later time) — an id compare would + // pick general-1 and render the stale bucket. + const buckets = bucketMessages([m(AUG_06, { agentID: "general-1" }), m(AUG_14, { agentID: "general-2" })]) + expect(selectMessages(buckets, "main", "ses_actorhost").map((x: any) => x.id)).toEqual([AUG_14.id]) + }) +}) diff --git a/packages/opencode/test/cli/tui/select-messages.test.ts b/packages/opencode/test/cli/tui/select-messages.test.ts index 8e5f59f63..a648d793f 100644 --- a/packages/opencode/test/cli/tui/select-messages.test.ts +++ b/packages/opencode/test/cli/tui/select-messages.test.ts @@ -1,7 +1,12 @@ import { describe, test, expect } from "bun:test" import { bucketMessages, selectMessages } from "../../../src/cli/cmd/tui/context/sync" -const msg = (id: string, agentID?: string) => ({ id, agentID }) as any +// `time.created` mirrors the numeric suffix of `id`. selectMessages picks the +// newest bucket by its last message's TIME (ids wrap every ~2.18 years and invert +// across the boundary — see compareMessages), so a fixture without a time would +// leave every bucket tied and stop exercising the choice at all. +const msg = (id: string, agentID?: string) => + ({ id, agentID, time: { created: Number(id.replace(/^\D*/, "")) || 0 } }) as any describe("selectMessages", () => { test("renders the main bucket for a normal session", () => { diff --git a/packages/opencode/test/session/classify.test.ts b/packages/opencode/test/session/classify.test.ts index fd8207836..a3cfde24e 100644 --- a/packages/opencode/test/session/classify.test.ts +++ b/packages/opencode/test/session/classify.test.ts @@ -6,12 +6,22 @@ import { ProviderID, ModelID } from "../../src/provider/schema" const sessionID = SessionID.make("session") +// `created` mirrors the numeric suffix of `id` so fixtures order the same way +// under MessageV2.compare (time first, id only as a same-millisecond tie-break) +// as they read on the page. Leaving every fixture at created: 0 would collapse +// the suite onto the id tie-break and stop exercising the time ordering the +// staleness guards at classify.ts #3a/#4 actually use. +function seq(id: string) { + const n = Number(id.replace(/^\D*/, "")) + return Number.isNaN(n) ? 0 : n +} + function userInfo(id: string): MessageV2.User { return { id: MessageID.make(id), sessionID, role: "user", - time: { created: 0 }, + time: { created: seq(id) }, agent: "user", model: { providerID: ProviderID.make("test"), modelID: ModelID.make("test") }, tools: {}, @@ -27,7 +37,7 @@ function assistantInfo( id: MessageID.make(id), sessionID, role: "assistant", - time: { created: 0 }, + time: { created: seq(id) }, parentID: MessageID.make("m-parent"), modelID: "test", providerID: "test", @@ -61,7 +71,7 @@ function toolPart(messageID: string, opts?: { providerExecuted?: boolean }) { } as unknown as MessageV2.Part } -// User "m-1" precedes assistant "m-2" so the stale guard (lastUser.id < assistant.id) is satisfied. +// User "m-1" precedes assistant "m-2" so the stale guard (compare(lastUser, assistant) < 0) is satisfied. const lastUser = userInfo("m-1") describe("classifyAssistantStep", () => { @@ -274,7 +284,7 @@ describe("classifyAssistantStep", () => { ).toBe("invalid") }) - test("existing-assistant phase + stale assistant (lastUser.id >= assistant.id) => continue", () => { + test("existing-assistant phase + stale assistant (assistant predates current user) => continue", () => { // user "m-2" comes after assistant "m-1": assistant predates the current turn. expect( classifyAssistantStep({ @@ -409,4 +419,53 @@ describe("classifyAssistantStep", () => { expect(result.type).toBe("continue") }) }) + + // Message ids encode a 48-bit timestamp that wraps every ~2.18 years (last + // boundary 2026-08-14T11:19:55Z), so across a wrap a NEWER message carries a + // SMALLER id. Both staleness guards (#3a and #4) must follow time, not the id, + // or a live turn is judged stale and vice versa. Ids below are real ones from + // a session that straddled the boundary. + describe("staleness guards across an id wraparound", () => { + const preWrap = "msg_fd708d21e001JXYNUE1Jba3VEw" // 2026-08-06 + const postWrap = "msg_0006f768700114fd6bDaDwzOWs" // 2026-08-14 + const withTime = (info: T, created: number) => + ({ ...info, time: { created } }) as T + + test("id order is inverted for this pair — the premise of the cases below", () => { + expect(postWrap < preWrap).toBe(true) + }) + + test("fresh post-wrap assistant is not judged stale", () => { + const result = classifyAssistantStep({ + phase: "existing-assistant", + lastUser: withTime(userInfo(preWrap), 1786019107358), + assistant: withTime(assistantInfo(postWrap, { finish: "stop" }), 1786713700254), + parts: [textPart(postWrap, "fresh answer")], + }) + // A raw id compare reads postWrap < preWrap and returns "continue" here. + expect(result).toEqual({ type: "final" }) + }) + + test("genuinely stale pre-wrap assistant is still judged stale", () => { + const result = classifyAssistantStep({ + phase: "existing-assistant", + lastUser: withTime(userInfo(postWrap), 1786713700254), + assistant: withTime(assistantInfo(preWrap, { finish: "stop" }), 1786019107358), + parts: [textPart(preWrap, "old answer")], + }) + expect(result).toEqual({ type: "continue" }) + }) + + test("text-form tool call in a fresh post-wrap turn is still detected", () => { + const result = classifyAssistantStep({ + phase: "existing-assistant", + lastUser: withTime(userInfo(preWrap), 1786019107358), + assistant: withTime(assistantInfo(postWrap, { finish: "tool-calls" }), 1786713700254), + parts: [textPart(postWrap, 'ls')], + }) + // A raw id compare skips #3a and falls through to the unconditional + // tool-calls continue at #3, losing the text-tool-call retry. + expect(result.type).toBe("text-tool-call") + }) + }) }) diff --git a/packages/opencode/test/session/message-id-wrap-order.test.ts b/packages/opencode/test/session/message-id-wrap-order.test.ts new file mode 100644 index 000000000..926826ae4 --- /dev/null +++ b/packages/opencode/test/session/message-id-wrap-order.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, test } from "bun:test" +import { Identifier } from "@/id/id" +import { MessageV2 } from "../../src/session/message-v2" + +// The id encoder packs `Date.now() * 0x1000 + counter` into 6 bytes, so the +// sortable prefix wraps every 2^36 ms. The most recent boundary: +const WRAP = 26 * 2 ** 36 // 1786706395136 ms = 2026-08-14 11:19:55 UTC +const PERIOD = 2 ** 36 + +const msg = (id: string, created: number) => ({ id, time: { created } }) + +describe("message id wraparound", () => { + test("encoder really does wrap at the 2^36 ms boundary", () => { + // Not asserting the bug is fixed in the encoder — asserting it EXISTS, so + // that widening the encoding later fails this test loudly rather than + // leaving the ordering fix looking pointless. + const before = Identifier.create("msg", "ascending", WRAP - 1) + const after = Identifier.create("msg", "ascending", WRAP + 1) + expect(before.slice(4, 16) > after.slice(4, 16)).toBe(true) + expect(after.slice(4, 8)).toBe("0000") + }) + + test("wrap period is ~2.18 years, next boundary ~Oct 2028", () => { + expect(PERIOD).toBe(68719476736) + expect(new Date(WRAP).toISOString()).toBe("2026-08-14T11:19:55.136Z") + expect(new Date(WRAP + PERIOD).getUTCFullYear()).toBe(2028) + }) + + test("compare orders across a wrap where bare id compare inverts", () => { + // Real ids: pre-wrap tail and post-wrap prompt from the wedged session. + const pre = msg("msg_fd708d21e001JXYNUE1Jba3VEw", 1786019107358) // Aug 06 + const post = msg("msg_0006f768700114fd6bDaDwzOWs", 1786713700254) // Aug 14 + + // The bug: lexicographic id order claims the Aug-14 message is older. + expect(post.id < pre.id).toBe(true) + // The fix: chronological order is correct. + expect(MessageV2.compare(pre, post)).toBeLessThan(0) + expect(MessageV2.compare(post, pre)).toBeGreaterThan(0) + }) + + test("compare handles the upstream-reported id pair too", () => { + const pre = msg("msg_ffd4c8ecc001nKKJtMDUGfajRL", 1786700000000) + const post = msg("msg_000bcb6970013Jm92xkWhlZiOA", 1786718762656) + expect(post.id < pre.id).toBe(true) + expect(MessageV2.compare(pre, post)).toBeLessThan(0) + }) + + test("id breaks ties within the same millisecond", () => { + const a = msg("msg_000aaa", 1786713700254) + const b = msg("msg_000bbb", 1786713700254) + expect(MessageV2.compare(a, b)).toBeLessThan(0) + expect(MessageV2.compare(b, a)).toBeGreaterThan(0) + }) + + test("compare is 0 only for the same message", () => { + const a = msg("msg_000aaa", 1786713700254) + expect(MessageV2.compare(a, { ...a })).toBe(0) + }) + + test("sorting a straddling session yields chronological order", () => { + const msgs = [ + msg("msg_0006f7687001", 1786713700254), // Aug 14, post-wrap + msg("msg_f4c12734b001", 1783687705445), // Jul 10, pre-wrap + msg("msg_fd708d21e001", 1786019107358), // Aug 06, pre-wrap + ] + expect([...msgs].sort(MessageV2.compare).map((m) => m.time.created)).toEqual([ + 1783687705445, 1786019107358, 1786713700254, + ]) + // Bare id sort puts the newest message first — the wedging behaviour. + expect([...msgs].sort((a, b) => (a.id < b.id ? -1 : 1))[0]!.time.created).toBe(1786713700254) + }) +})