Skip to content

fix(opencode): order legacy message loop by time - #149

Merged
MagMueller merged 1 commit into
mainfrom
message-ordering
Aug 15, 2026
Merged

fix(opencode): order legacy message loop by time#149
MagMueller merged 1 commit into
mainfrom
message-ordering

Conversation

@MagMueller

@MagMueller MagMueller commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Cherry-picks upstream OpenCode commit db581e47a3a6f4900a6289ad7fddec60fec44e1c unchanged.

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 passed
  • Focused prompt regression: 1 passed
  • bun typecheck in packages/opencode: passed
  • Root filtered bun run typecheck: 17/17 packages passed
  • Full prompt.test.ts remains 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.

  • No migration required; cross-rollover sessions recover on the next turn.
  • Changes are limited to selection logic in packages/opencode/src/session/message-v2.ts and the loop exit condition in packages/opencode/src/session/prompt.ts.
  • Adds tests for nonmonotonic IDs and post-finish task selection.

Written for commit de29ad3. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai 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.

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

@cubic-dev-ai cubic-dev-ai Bot Aug 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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>
Fix with cubic


const result = yield* prompt.loop({ sessionID: chat.id })

expect(result.info.id).toBe(assistantID)

@cubic-dev-ai cubic-dev-ai Bot Aug 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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>
Fix with cubic

@MagMueller
MagMueller merged commit dd8d70c into main Aug 15, 2026
3 checks passed
@MagMueller
MagMueller deleted the message-ordering branch August 15, 2026 17:20
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.

1 participant