fix(opencode): order legacy message loop by time - #149
Conversation
Co-authored-by: Dax <mail@thdxr.com>
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/test/session/prompt.test.ts">
<violation number="1" location="packages/opencode/test/session/prompt.test.ts:498">
P3: This test does not discriminate the fix: with a single user message and a single assistant message, `MessageV2.latest`/`isAfter` returns the same result whether ordered by ID or by `time.created`, so the test would pass on the pre-fix code that sorts only by ID. To actually guard the rollover regression described in the PR, add a second (newer-by-time) user message whose ID sorts below the complete assistant, matching the cross-rollover layout where ID order and time order diverge.</violation>
</file>
<file name="packages/opencode/src/session/prompt.ts">
<violation number="1" location="packages/opencode/src/session/prompt.ts:1115">
P2: The new exit condition `lastAssistant.parentID === lastUser.id` never holds when the final completed assistant message has no resolvable `parentID`, which can happen for the restored/imported sessions this PR targets (session fork/import in session.ts only preserves parentID when the parent id resolves in `idMap`). In that case the loop does not recognize the completed turn and issues an unwanted extra LLM call before self-healing. Consider treating a missing/unmatched parentID (e.g. no later user than the assistant's parent by time) as a completed turn so restored legacy sessions exit without a redundant request.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| !["tool-calls"].includes(lastAssistant.finish) && | ||
| !hasToolCalls && | ||
| lastUser.id < lastAssistant.id | ||
| lastAssistant.parentID === lastUser.id |
There was a problem hiding this comment.
P2: The new exit condition lastAssistant.parentID === lastUser.id never holds when the final completed assistant message has no resolvable parentID, which can happen for the restored/imported sessions this PR targets (session fork/import in session.ts only preserves parentID when the parent id resolves in idMap). In that case the loop does not recognize the completed turn and issues an unwanted extra LLM call before self-healing. Consider treating a missing/unmatched parentID (e.g. no later user than the assistant's parent by time) as a completed turn so restored legacy sessions exit without a redundant request.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/session/prompt.ts, line 1115:
<comment>The new exit condition `lastAssistant.parentID === lastUser.id` never holds when the final completed assistant message has no resolvable `parentID`, which can happen for the restored/imported sessions this PR targets (session fork/import in session.ts only preserves parentID when the parent id resolves in `idMap`). In that case the loop does not recognize the completed turn and issues an unwanted extra LLM call before self-healing. Consider treating a missing/unmatched parentID (e.g. no later user than the assistant's parent by time) as a completed turn so restored legacy sessions exit without a redundant request.</comment>
<file context>
@@ -1112,7 +1112,7 @@ const layer = Layer.effect(
!["tool-calls"].includes(lastAssistant.finish) &&
!hasToolCalls &&
- lastUser.id < lastAssistant.id
+ lastAssistant.parentID === lastUser.id
) {
const orphan = lastAssistantMsg?.parts.find(
</file context>
|
|
||
| const result = yield* prompt.loop({ sessionID: chat.id }) | ||
|
|
||
| expect(result.info.id).toBe(assistantID) |
There was a problem hiding this comment.
P3: This test does not discriminate the fix: with a single user message and a single assistant message, MessageV2.latest/isAfter returns the same result whether ordered by ID or by time.created, so the test would pass on the pre-fix code that sorts only by ID. To actually guard the rollover regression described in the PR, add a second (newer-by-time) user message whose ID sorts below the complete assistant, matching the cross-rollover layout where ID order and time order diverge.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/test/session/prompt.test.ts, line 498:
<comment>This test does not discriminate the fix: with a single user message and a single assistant message, `MessageV2.latest`/`isAfter` returns the same result whether ordered by ID or by `time.created`, so the test would pass on the pre-fix code that sorts only by ID. To actually guard the rollover regression described in the PR, add a second (newer-by-time) user message whose ID sorts below the complete assistant, matching the cross-rollover layout where ID order and time order diverge.</comment>
<file context>
@@ -460,6 +460,46 @@ noLLMServer.instance(
+
+ const result = yield* prompt.loop({ sessionID: chat.id })
+
+ expect(result.info.id).toBe(assistantID)
+ }),
+ { config: cfg },
</file context>
Summary
Cherry-picks upstream OpenCode commit
db581e47a3a6f4900a6289ad7fddec60fec44e1cunchanged.Messages are now ordered by
time.created, with IDs used only as a deterministic tie-breaker. The legacy prompt loop now considers a turn complete only when the latest assistant message has the latest user message as its parent.Root cause
Ascending message IDs encode a 36-bit millisecond timestamp plus a counter. The timestamp portion rolled over at
2026-08-14T11:19:55.136Z, causing new IDs to sort below pre-rollover IDs. Restored sessions therefore selected an old completed assistant turn and exited without making an LLM call.Impact
Cross-rollover sessions recover on their next turn after upgrading BrowserCode. No data migration or transcript rewrite is required.
Verification
bun test test/session/message-v2.test.ts: 39 passedbun typecheckinpackages/opencode: passedbun run typecheck: 17/17 packages passedprompt.test.tsremains unhealthy in this fork locally: 28 passed, 29 failed, 1 skipped; the new nonmonotonic-ID regression passes and none of the failures are in the changed ordering coverage.Summary by cubic
Orders legacy message selection by time.created with IDs as a tie-breaker, fixing nonmonotonic ID issues. The prompt loop now marks a turn complete only when the latest assistant’s parentID matches the latest user; previously it compared IDs and could exit early after the ID timestamp rollover.
packages/opencode/src/session/message-v2.tsand the loop exit condition inpackages/opencode/src/session/prompt.ts.Written for commit de29ad3. Summary will update on new commits.