fix(session): delete reverted messages boundary-last and tie-break ids by raw order

[NamedIdentity]

Issue for this PR

Closes #42816

Type of change

• Bug fix
• New feature
• Refactor / code improvement
• Documentation

What does this PR do?

Two fixes to the ID rollover ordering work.

1. Revert cleanup deletes the boundary message first.

SessionRevert.cleanup removes messages from the revert boundary onward. The boundary is remove[0] when no partID is set, and each removeMessage commits separately. If that loop is interrupted, the boundary is already gone but the rest are not. The next cleanup calls findIndex for the boundary, gets -1, so remove is empty and it deletes nothing. clearRevert() still runs at the end, discarding the marker that could have located those rows. They stay in the session permanently and get sent to the model on the next turn.

Using .toReversed() deletes the boundary last, so it works as a progress marker. An interrupted cleanup then leaves a state the next one can finish. The part loop below it has the same problem and the same fix.

2. The localeCompare tie-break does not match storage order.

Storage pages with ORDER BY time_created, id, and SQLite's default collation is BINARY. sync.tsx and Share.tsx tie-break the same data with localeCompare, which is locale collation. For two messages sharing a time.created these can return opposite results, so the client and the database can disagree about which comes first. Ids minted in the same millisecond by different processes share the timestamp prefix, so the random mixed case suffix decides, and that is exactly where the two collations differ.

localeCompare can also return 0 for two distinct strings that are canonically equivalent. MessageID only requires a msg prefix, so two different primary keys can compare equal and the sort becomes dependent on input order.

Raw < and > match what storage already does.

How did you verify your code works?

Read the affected paths and traced the interruption sequence above. Confirmed toReversed() is already used elsewhere in the repo, so there are no target or lib concerns.

Both fixes have been running in our fork for a day. We hit the same rollover in production on 2026-08-14, wrote the same core fix independently, then found yours had landed. These two turned up while auditing ours.

I have not run the full test suite against this branch.

Screenshots / recordings

Not a UI change.

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

[github-actions]

Hey! Your PR title fix(session): delete reverted messages boundary-last and tie-break ids by raw order doesn't follow conventional commit format.

Please update it to start with one of:

• feat: or feat(scope): new feature
• fix: or fix(scope): bug fix
• docs: or docs(scope): documentation changes
• chore: or chore(scope): maintenance tasks
• refactor: or refactor(scope): code refactoring
• test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

[github-actions]

Thanks for updating your PR! It now meets our contributing guidelines. 👍

[Enough1122]

AI code review — automated review for reference; please use your judgment.

Review of the revert-deletion ordering fix — the boundary-last strategy and its rationale comment are well reasoned (interrupted cleanups stay resumable because the surviving marker still locates the remaining rows):

1. Test coverage — the crash-resumption property motivating toReversed() ships without any test — add a regression case that fails removePart/removeMessage partway through the loop, then re-runs cleanup and asserts no rows silently rejoin the transcript — this is exactly the invariant a future refactor of the slice/reverse logic would break invisibly.
2. packages/tui/src/context/sync.tsx:55 and packages/web/src/components/Share.tsx (~L79) — the same comparator (and now the same subtle BINARY-collation rationale) is duplicated across two packages — the copies will drift when one gains a fix the other misses — extract a shared compareByCreatedThenId helper into a common package both frontends already import from.
3. packages/tui/src/context/sync.tsx:59 — raw </> orders by UTF-16 code units, while SQLite BINARY collation orders by UTF-8 bytes — these disagree for non-BMP characters (surrogates sort below U+E000–U+FFFF in UTF-16 but above in code-point order) — harmless while session/message ids stay ASCII, but the comment claims general equivalence it does not have — scope the claim ("ids are ASCII") or compare by code points explicitly.
4. packages/opencode/src/session/revert.ts:109 — nit: consider extracting the two commented loops' shared "delete backwards, boundary last" pattern into a tiny named helper (removeReversed) so the durability argument lives in exactly one place alongside the code enforcing it.

[github-actions]

Automated PR Cleanup

Thank you for contributing to opencode.

Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions.

This PR was closed because it matched the following cleanup criteria:

• The PR was created more than 1 month ago
• The PR had fewer than 2 positive reactions
• Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

PRs created within the last month are not affected by this cleanup.

If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate.

Thanks again for taking the time to contribute.