Skip to content

Commit db581e4

Browse files
fix(opencode): order legacy message loop by time (#40990)
Co-authored-by: Dax <[email protected]>
1 parent a54a693 commit db581e4

4 files changed

Lines changed: 120 additions & 10 deletions

File tree

‎packages/opencode/src/session/message-v2.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -577,29 +577,32 @@ export const filterCompactedEffect = Effect.fnUntraced(function* (sessionID: Ses
577577

578578
// filterCompacted reorders messages for model consumption
579579
// ([compaction-user, summary, ...retained tail..., continue-user]), so array
580-
// position is not chronological. Derive each binding by max id (MessageID
581-
// is monotonic via MessageID.ascending) so a pre-compaction overflowing tail
582-
// assistant doesn't get mistaken for the most recent turn. tasks are
583-
// compaction/subtask parts attached to user messages newer than the latest
584-
// finished assistant — i.e. unprocessed work.
580+
// position is not chronological. IDs are only a deterministic tie-breaker
581+
// because imported messages do not necessarily have monotonic IDs.
585582
export function latest(msgs: WithParts[]) {
586583
let user: User | undefined
587584
let assistant: Assistant | undefined
588585
let finished: Assistant | undefined
589586
for (const msg of msgs) {
590587
const info = msg.info
591-
if (info.role === "user" && (!user || info.id > user.id)) user = info
592-
if (info.role === "assistant" && (!assistant || info.id > assistant.id)) assistant = info
593-
if (info.role === "assistant" && info.finish && (!finished || info.id > finished.id)) finished = info
588+
if (info.role === "user" && isAfter(info, user)) user = info
589+
if (info.role === "assistant" && isAfter(info, assistant)) assistant = info
590+
if (info.role === "assistant" && info.finish && isAfter(info, finished)) finished = info
594591
}
595592
const tasks = msgs.flatMap((m) =>
596-
finished && m.info.id <= finished.id
593+
finished && !isAfter(m.info, finished)
597594
? []
598595
: m.parts.filter((p): p is CompactionPart | SubtaskPart => p.type === "compaction" || p.type === "subtask"),
599596
)
600597
return { user, assistant, finished, tasks }
601598
}
602599

600+
function isAfter(info: Info, other?: Info) {
601+
if (!other) return true
602+
if (info.time.created !== other.time.created) return info.time.created > other.time.created
603+
return info.id > other.id
604+
}
605+
603606
export function fromError(
604607
e: unknown,
605608
ctx: { providerID: ProviderV2.ID; aborted?: boolean },

‎packages/opencode/src/session/prompt.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1112,7 +1112,7 @@ const layer = Layer.effect(
11121112
lastAssistant?.finish &&
11131113
!["tool-calls"].includes(lastAssistant.finish) &&
11141114
!hasToolCalls &&
1115-
lastUser.id < lastAssistant.id
1115+
lastAssistant.parentID === lastUser.id
11161116
) {
11171117
const orphan = lastAssistantMsg?.parts.find(
11181118
(part): part is SessionV1.ToolPart => part.type === "tool" && isOrphanedInterruptedTool(part),

‎packages/opencode/test/session/message-v2.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,6 +1611,44 @@ describe("session.message-v2.latest", () => {
16111611
] as SessionV1.Part[],
16121612
}
16131613

1614+
test("selects latest messages by creation time when IDs are nonmonotonic", () => {
1615+
const oldUser = { ...userInfo("msg_z_user"), time: { created: 100 } }
1616+
const newUser = { ...userInfo("msg_a_user"), time: { created: 200 } }
1617+
const oldAssistant = {
1618+
...assistantInfo("msg_z_assistant", oldUser.id),
1619+
time: { created: 300 },
1620+
finish: "stop",
1621+
} as SessionV1.Assistant
1622+
const newAssistant = {
1623+
...assistantInfo("msg_a_assistant", newUser.id),
1624+
time: { created: 400 },
1625+
finish: "stop",
1626+
} as SessionV1.Assistant
1627+
1628+
const state = MessageV2.latest([
1629+
{ info: newAssistant, parts: [] },
1630+
{ info: oldUser, parts: [] },
1631+
{ info: oldAssistant, parts: [] },
1632+
{ info: newUser, parts: [] },
1633+
])
1634+
1635+
expect(state.user?.id).toBe(newUser.id)
1636+
expect(state.assistant?.id).toBe(newAssistant.id)
1637+
expect(state.finished?.id).toBe(newAssistant.id)
1638+
})
1639+
1640+
test("uses ID as a deterministic tie-breaker for equal creation times", () => {
1641+
const lower = { ...userInfo("msg_a_user"), time: { created: 100 } }
1642+
const higher = { ...userInfo("msg_z_user"), time: { created: 100 } }
1643+
1644+
const state = MessageV2.latest([
1645+
{ info: higher, parts: [] },
1646+
{ info: lower, parts: [] },
1647+
])
1648+
1649+
expect(state.user?.id).toBe(higher.id)
1650+
})
1651+
16141652
// Regression for double auto-compaction. The reorder in filterCompacted
16151653
// (#27145) returns [compaction-user, summary, ...tail..., continue-user],
16161654
// so picking lastFinished by array position landed on the pre-compaction
@@ -1659,4 +1697,33 @@ describe("session.message-v2.latest", () => {
16591697
expect(state.tasks).toHaveLength(1)
16601698
expect(state.tasks[0]).toMatchObject({ type: "compaction", auto: true })
16611699
})
1700+
1701+
test("selects compaction and subtask work after the finished boundary by creation time", () => {
1702+
const finished = {
1703+
...assistantInfo("msg_z_finished", "msg_parent"),
1704+
time: { created: 200 },
1705+
finish: "stop",
1706+
} as SessionV1.Assistant
1707+
const oldTask: SessionV1.WithParts = {
1708+
info: { ...userInfo("msg_z_old"), time: { created: 100 } },
1709+
parts: [{ ...basePart("msg_z_old", "old"), type: "compaction", auto: true }] as SessionV1.Part[],
1710+
}
1711+
const newTask: SessionV1.WithParts = {
1712+
info: { ...userInfo("msg_a_new"), time: { created: 300 } },
1713+
parts: [
1714+
{
1715+
...basePart("msg_a_new", "new"),
1716+
type: "subtask",
1717+
prompt: "inspect",
1718+
description: "inspect ordering",
1719+
agent: "general",
1720+
},
1721+
] as SessionV1.Part[],
1722+
}
1723+
1724+
const state = MessageV2.latest([newTask, { info: finished, parts: [] }, oldTask])
1725+
1726+
expect(state.tasks).toHaveLength(1)
1727+
expect(state.tasks[0]).toMatchObject({ type: "subtask", prompt: "inspect" })
1728+
})
16621729
})

‎packages/opencode/test/session/prompt.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,46 @@ noLLMServer.instance(
460460
{ config: cfg },
461461
)
462462

463+
noLLMServer.instance(
464+
"loop exits for a completed parent turn with nonmonotonic message IDs",
465+
() =>
466+
Effect.gen(function* () {
467+
const prompt = yield* SessionPrompt.Service
468+
const sessions = yield* Session.Service
469+
const chat = yield* sessions.create({ title: "Pinned" })
470+
const userID = MessageID.make("msg_z_user")
471+
const assistantID = MessageID.make("msg_a_assistant")
472+
yield* sessions.updateMessage({
473+
id: userID,
474+
role: "user",
475+
sessionID: chat.id,
476+
agent: "build",
477+
model: ref,
478+
time: { created: 100 },
479+
})
480+
yield* sessions.updateMessage({
481+
id: assistantID,
482+
role: "assistant",
483+
parentID: userID,
484+
sessionID: chat.id,
485+
mode: "build",
486+
agent: "build",
487+
cost: 0,
488+
path: { cwd: "/tmp", root: "/tmp" },
489+
tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } },
490+
modelID: ref.modelID,
491+
providerID: ref.providerID,
492+
time: { created: 200, completed: 201 },
493+
finish: "stop",
494+
})
495+
496+
const result = yield* prompt.loop({ sessionID: chat.id })
497+
498+
expect(result.info.id).toBe(assistantID)
499+
}),
500+
{ config: cfg },
501+
)
502+
463503
it.instance("loop exits without an LLM request for interrupted orphan tool calls", () =>
464504
Effect.gen(function* () {
465505
const { llm } = yield* useServerConfig(providerCfg)

0 commit comments

Comments
 (0)