fix(engine): clear left_at when re-resolving a 1:1 DM - #302
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe DM participant conflict update clears ChangesDeterministic DM behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a745b4adc
ℹ️ 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".
| }).onConflictDoUpdate(rejoin); | ||
| await db.insert(dmParticipants).values({ | ||
| conversationId: deterministicConversationId, | ||
| agentId: toAgentId, | ||
| }).onConflictDoNothing(); | ||
| }).onConflictDoUpdate(rejoin); |
There was a problem hiding this comment.
This changes user-visible behavior when a 1:1 participant has left_at set: the next send now restores that participant and resumes routing instead of leaving the roster departed. Neither the root changelog nor packages/engine/CHANGELOG.md records the fix, and the package changelog still has an unqualified [Unreleased] heading; add concise patch entries and raise that heading to [Unreleased - Patch] as required.
AGENTS.md reference: AGENTS.md:L39-L45
Useful? React with 👍 / 👎.
|
Addressed the Codex P1 in 2a8f23a — good catch, the AGENTS.md release-level rule was a repo convention I missed rather than an oversight I'd argue with.
Patch is the right level: behaviour only changes in a state no current code path can reach ( CodeRabbit reported no actionable comments. |
A 1:1 DM conversation id is a pure function of (workspaceId, sorted agent pair), so it names the durable A<->B relationship rather than a conversation instance. `resolveConversation` finds an existing 1:1 by joining dm_participants with `left_at IS NULL`; if a participant is ever marked departed the SELECT misses, the create branch runs, recomputes the same deterministic id, and re-inserts with onConflictDoNothing() — which no-ops against the existing rows and leaves `left_at` set. The conversation then keeps resolving correctly while its roster says someone left. Nothing throws; the two just disagree from then on. Identifier stability is not what breaks — the id never consults `left_at` — what breaks is the claim that the identifier names a relationship with fixed membership. That claim is load-bearing outside this repo: Agent Relay has declared this lifecycle model to Identities AI as the basis on which Ratify delegation certificates are scoped to a DM (invariant DM-1 in the Agent Relay Resource Identifier Profile). So it is stated as an invariant with a test rather than left as a code comment. The create branch now clears `left_at` on conflict instead of no-opping. Reaching that branch for an id that already exists means the lookup missed, and for an existing 1:1 the only way it can miss is a departed participant — so re-resolution heals the roster. Resolution semantics are otherwise unchanged, and no current code path can set `left_at` for a 1:1 (it is written only by the group-DM code). Adds four tests covering the model: id derives from (workspace, sorted pair); either sender resolves the same conversation; re-resolution creates no second conversation; and DM-1 itself. The DM-1 test fails against the previous behaviour, which is the point. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Raises the engine changelog's pending heading to [Unreleased - Patch] per the AGENTS.md release-level rule — the fix changes user-visible behaviour, since a 1:1 whose participant was marked departed now restores that participant on the next send rather than resolving with a stale roster. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2a8f23a to
7a88f50
Compare
|
Rebased onto The changelog entries needed re-filing after the rebase:
Re-verified on the new base: |
1 participant
What
A 1:1 DM conversation id is a pure function of
(workspaceId, sorted agent pair), so it names the durable A↔B relationship rather than a conversation instance.resolveConversationfinds an existing 1:1 by joiningdm_participantswithleft_at IS NULL. If a participant is ever marked departed theSELECTmisses, the create branch runs, recomputes the same deterministic id, and re-inserts withonConflictDoNothing()— which no-ops against the existing conversation, channel, and participant rows, leavingleft_atset.The conversation then keeps resolving correctly while its roster says someone left. Nothing throws; the two just disagree from then on.
Identifier stability is not what breaks — the id never consults
left_at. What breaks is the claim that the identifier names a relationship with fixed membership.Why it matters outside this repo
That claim is load-bearing for the Ratify design partnership. Agent Relay has declared this lifecycle model to Identities AI as the basis on which Ratify delegation certificates are scoped to a DM — invariant DM-1 in the Agent Relay Resource Identifier Profile. It is stated as an invariant with a test rather than left as a code comment, so it cannot change silently later.
Change
The create branch clears
left_aton conflict instead of no-opping. Reaching that branch for an id that already exists means the lookup missed, and for an existing 1:1 the only way it can miss is a departed participant — so re-resolution heals the roster.Resolution semantics are otherwise unchanged. No current code path can set
left_atfor a 1:1: it is written only by the group-DM code (groupDm.ts). The test reaches that state directly so the invariant is guarded before some future path can reach it accidentally.Tests
Four tests covering the model:
(workspace, sorted pair)The DM-1 test fails against the previous behaviour, which is the point — it pins the invariant, not the current behaviour.
Verification
npx turbo build && npx vitest runinpackages/engine— 515/515npx turbo lint— cleanReview note
This touches production message-routing code (
onConflictDoNothing→onConflictDoUpdatein the 1:1 resolution path). It is one line and only fires in a branch unreachable today, but a reviewer who owns this file should confirm the conflict-target choice.🤖 Generated with Claude Code