DottaandClaude
4baff83225
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Chat channels let a linked user talk to an agent from GitHub, Slack and other platforms. Each external thread is a chat conversation. Many conversations can share one task. > - A reviewed chat run attests exactly one conversation. #17054 keeps each conversation in its own wake batch, but it compares only the conversation that a wake records. > - The work-intent outbox saves unanswered user messages as a plain comment wake. That wake records no conversation, even when its messages are chat messages. A wake with no recorded conversation matched every conversation, so a message from another conversation merged into it. > - The promoted run held messages of two conversations. The binding check correctly refused it (`reviewed_chat_execution_binding_not_authorized`, `binding_denied`), and the task needed Retry. > - This pull request compares the conversations of the queued messages themselves. Wakes of different conversations never merge. Each conversation gets its own run, in order. > - The benefit is that replies in several threads of one task each get an attested run, without a failed run or a Retry. ## Linked Issues or Issue Description Refs #17054 (the earlier per-conversation batching fix). Refs #20689 (a different shape of the same error). **What happened?** On staging, a GitHub pull-request review task had two runs fail with `setup_failed` / `reviewed_chat_execution_binding_not_authorized` (`reviewedChatBinding.denialReason: binding_denied`). Each failed run held the same two older saved messages plus one new message. The two runs recorded two different `chatConversationId` values. The work-intent outbox had saved the older messages as plain deferred wakes with no conversation. A new message from each conversation then merged into one of these saved wakes. **Steps to reproduce** 1. Bind two chat conversations to one `in_review` task. 2. Let the outbox save an unanswered message of conversation B as a deferred wake while a run is active. 3. As the linked user, send a message in conversation A while the run is still active. 4. See that the message of A merges into the saved wake of B. The promoted run fails the binding check. **Expected behavior** The saved message of B runs as its own wake. The message of A runs after it, in its own attested run. ## What Changed - New `server/src/modules/wake-queue/adapters/chat-conversation-batch.ts`: reads the inbound chat links (`chat_message_links`, direction `inbound`) of queued messages and finds messages of a conversation other than the expected one. - Deferred-wake lookup (`findExistingDeferredWake`): when the incoming wake has a conversation, a saved wake qualifies only if none of its queued messages is an inbound message of another conversation. This is one more predicate in the same query (no new round trip). It uses the existing `chat_message_links_company_comment_idx` index. - Coalescing into the active run (`coalesceIntoActiveExecutionRun`): returns no merge when the merged batch would hold messages of a conversation other than the one it records. The wake is then saved behind the run, which is the existing fallback. - Release drain (`findNextDeferredWake`), upgrade repair: a saved chat wake that older code already merged across two conversations is split. The other conversation's messages move to a plain saved wake (the outbox shape), ordered just before it. The original wake keeps its conversation and its own messages. The drain then reads the queue again. - `doc/execution-semantics.md`: describe the comparison and the upgrade repair. - New test `chat-wake-conversation-batch.test.ts` (embedded Postgres, real admission and release code, real `authorizeChatConversationForBoundRun`). ## Verification - Red on `origin/master` (test file only): `pnpm --dir server exec vitest run src/modules/wake-queue/adapters/chat-wake-conversation-batch.test.ts` gives 4 failed, 6 passed. The 4 failures are the bug: the new message merges into the other conversation's saved wake (`[[earlier, reply]]`), two conversations cross-merge, a message coalesces into a queued run of another conversation, and the old merged shape stays merged. The 6 passing cases pin unchanged batching. - Green with the fix: 10 of 10 pass. Each separated wake passes `authorizeChatConversationForBoundRun` for its own conversation. The old merged shape fails it with `paperclip_runner_chat_attachment_binding_denied` before the split. - Batching that stays the same (pinned, pass before and after): comment wakes with no conversation merge; messages of one conversation merge; an outbox-saved wake of the same conversation still absorbs the reply; a chat message and plain comments merge in both orders; same-conversation messages still coalesce into a queued run; two recorded conversations stay apart. - Related suites pass locally: `src/modules/wake-queue`, `src/modules/run-dispatch`, `chat-run-milestone-work`, `durable-chat-wakeup`, `heartbeat-comment-wake-batching`, `heartbeat-reviewed-chat-binding.integration`, `slack-conversation-lifecycle`, `queued-comment-consumption`, `issue-queued-comments-routes` (23 files, 1037 tests). `heartbeat-process-recovery`: see the comment below. - `tsc --noEmit -p server/tsconfig.json` passes. ## Risks - Low risk. No schema change and no migration. - Behavior change only for chat messages: a message no longer joins a saved wake or a run that holds another conversation's inbound messages. It gets its own wake behind them, and it runs after them. Before this change, that batch always failed the binding check. - Board comments have no inbound chat link, so they belong to no conversation. Wakes without a conversation, wakes of one conversation, and mixed board-comment and chat wakes batch exactly as before. - Ordering: the deferred queue still promotes one wake per release, oldest first. The second conversation waits for the issue execution lock. Single-assignee and atomic checkout are unchanged. - Upgrade: a saved wake that older code merged across conversations is split at promotion. No message is dropped or delivered twice: each message id moves to exactly one wake. A run that older code already promoted in the merged shape is not changed; it fails the binding check as before and keeps its existing Retry path. - No wait is added before a turn starts. Admission adds one predicate to an existing query. Coalescing adds one indexed read only when the merged batch has a chat conversation. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), in Claude Code with tool use (shell, file edits, GitHub CLI). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>
Feature modules
A feature module under server/src/modules/<name>/ uses three layers. Each
layer has one rule: which layer below it, it may import from.
adapters → application → domain
domain/holds pure business rules. A domain file takes plain data in and returns plain data out. A domain file must not importdrizzle-orm,@paperclipai/db, a service underserver/src/services/, a route underserver/src/routes/, or a Node.js I/O module (node:child_process,node:fs,node:net). A domain function must not read the system clock; the caller passesnowas an explicitDatevalue.application/holds use cases and the ports they need. A use case takes its ports as constructor arguments and calls domain functions for policy decisions. An application file must not importdrizzle-orm, a SQL client, a concrete adapter, or the server's HTTP error helpers. The outer service or route translates application errors into transport responses.adapters/holds the concrete implementations of the ports: Postgres queries, transactions, and process control. An adapter file may importdrizzle-orm,@paperclipai/db, and Node.js I/O modules.
A module exposes its commands through index.ts, which composes the adapters
and the use cases behind a factory function. Code outside the module imports
that entry point, never a file inside domain/, application/, or
adapters/ directly.
pnpm check:module-boundaries enforces these rules for production source
files. It also checks access to the company deletion entry point below.
Company deletion
The existing company deletion service owns the database transaction and deletion
order. A module can expose a separate company-deletion.ts entry point.
Only services/company-deletion.ts can import this entry point.
The entry point exports an implementation of CompanyDeletionParticipant from
lib/company-deletion.ts. Do not export it through the normal module index.
The service must complete external cleanup before it starts the transaction. It must lock the company before it deletes dependent records or calls a module. Creation commands must use the same company lock. The service removes dependent records before it calls the module. The module checks its deletion conditions and deletes the records it owns. Use the supplied transaction. Do not start another transaction or call external systems. Throw an error if deletion cannot proceed. An error must roll back all database changes in the deletion transaction.
Call each module explicitly in dependency order. No module registry is required.
The agent lifecycle module also has a restricted configuration.ts entry point.
It saves configuration and invalidates verification in the caller's transaction.
The lifecycle guide lists its permitted callers.
Ordinary configuration updates use the root lifecycle command.
The agent lifecycle module owns agent creation, pause, resume, and termination. Use its commands for these changes.
The execution admission module owns project task-execution policy and the
durable admission records that charge project capacity. Today it exposes only
internal policy commands and the deletion guards that project, agent, and
company deletion call. Settlement releases active charges but keeps each
admission's complete project set for later retained-session resolution.
Execution claims still belong to services/heartbeat/queue.ts and native
review dispatch. See execution admission accounting.