fix: address review feedback on conversation-viewed dedup#9424
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d723c5129e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ne(messages.id, excludeMessageId), | ||
| )) | ||
| .orderBy(asc(messages.createdAt)) | ||
| .orderBy(asc(messages.createdAt), asc(messages.id)) |
There was a problem hiding this comment.
Order same-timestamp messages by sequence, not UUID
getNextMessage now uses asc(messages.id) as the tie-breaker, but message IDs in this store are UUID v4 values (conversation-store.ts imports v4 as uuid), so the tie-break order is effectively random relative to actual message chronology. In legacy conversations where multiple rows share createdAt, this can return a non-adjacent row; handleMessageContentRequest depends on this helper returning the immediate next user message to merge tool results, so tool outputs can be skipped and message content becomes incomplete.
Useful? React with 👍 / 👎.
| @@ -592,14 +592,6 @@ async function hatchLocal(species: Species, name: string | null, daemonOnly: boo | |||
|
|
|||
| const baseDataDir = join(process.env.BASE_DATA_DIR?.trim() || (process.env.HOME ?? userInfo().homedir), ".vellum"); | |||
There was a problem hiding this comment.
Keep a guard against rehatching an active local data dir
Removing the local baseDataDir existence guard allows rehatching into an already-used ~/.vellum directory; in that case a second local hatch can proceed even while another local assistant is running, and because gateway startup is fire-and-forget, this path can still record a new assistant entry and overwrite PID metadata even if the new gateway dies on port conflict. That leaves lockfile/default-assistant state inconsistent with the actually running gateway.
Useful? React with 👍 / 👎.
Addresses review feedback from PR #9361 on conversation-viewed event deduplication.