CedarCopilot wants to merge 3 commits into staging from fix/sherlock-post-meeting-draft-thread-and-cleanup-20260922-084120
Live on prod, 0 of 6 surfaces working, 2 days left
internal_only
internal_only
internal_only
internal_only
single_user
internal_only
single_user
internal_only
Libra found 6 production surfaces on prod but could not judge any of them yet. 4 surfaces had under 20 requests, so Libra has not judged them; 2 surfaces had no requests at all. Libra checks hourly for 3 days after each deploy.
PR #2878 (merged) fixed the original post-meeting draft detach/delete bug and the first round of Greptile findings. Greptile's review continued after that merge and found 3 more real issues that were fixed on this same branch , but since #2878 had already merged by the time these landed, they were never actually included in an open PR. This PR carries just those remaining commits.
1. Closes the concurrent-independent-deletion race properly (apps/server/src/services/user-tasks/tasks.ts)
The shared-draftId guard added in #2878 (findLiveTaskSharingDraft) is only race-safe when the caller serializes access. The bulk functions (deleteTasksByDraftIds, deleteAllTasksOlderThan) serialize the tasks they discover together, but two fully independent deleteTask calls (e.g. a chat-driven single delete racing an unrelated cron sweep) had no such protection , both could see the other task as still todo and both defer, orphaning the Gmail draft with no task pointing to it.
Fixed with two changes, both provably correct without a lock or transaction:
deleteTask/deleteAllTasksOlderThan now mark a task's own status terminal before running the guard, not after , this makes the "both defer" interleaving logically impossible (see deleteTask's doc comment for the argument).getDraftOwnerTaskId, a deterministic tie-break (lowest task id among every task naming the draftId) that only runs once the guard has confirmed no task is live, at which point the set is stable and every caller agrees on the same owner.New test in shared-draft-cleanup.integration.test.ts runs two fully independent deleteTask calls through Promise.all against real SQL (PGlite) and asserts the Gmail draft is deleted exactly once , verified locally it fails (2 calls) with the reorder alone and passes once the tie-break is added.
2. Stops accepting a draft from an unrelated thread as a rewrite target (apps/server/src/services/drafting/drafting.ts)
checkAndHandleExistingDraft's event.newMessages fast path took the first draft it found with no check that its own threadId matched the requested thread. event.newMessages is scoped to the triggering event's own thread, which isn't necessarily the thread a rewrite targets (e.g. replying on an older thread while a meeting event, which has no thread of its own, triggered the run). The hydratedConversation fallback path already scopes correctly; this brings the fast path in line with it.
Consequence before this fix: the returned draft was reported back under the requested threadId regardless of which thread it actually came from, so a caller could rewrite an unrelated conversation's draft while also skipping the post-meeting new-thread rule for it.
New test (check-existing-draft-thread-scoping.test.ts) , no prior test exercised the real function at all; every caller-side test mocked it away. Verified locally it fails on both cases without the filter and passes with it.
3. Removes the two banned type casts flagged in review (test files only, no behavior change)
Both as any / as unknown as T casts were copied from pre-existing precedent elsewhere in the codebase, but the project rule against them is absolute, and both turned out removable once actually tried , see commit for the specifics of each.
All new/existing tests pass (360 across the touched directories, verified locally). The two race-closing tests were verified to fail without their respective fixes and pass with them, confirming they're not just accidentally green.
🤖 Generated with Claude Code
| Surface | Requests | Errors | p95 | Users | Verdict |
|---|---|---|---|---|---|
| model_chunk Next Steps and Tasks Manager | 6 → 11 | 0 → 0 (0%) | 1 ms → 1 ms | 0 | Insufficient traffic 11 requests, under the 20 Libra needs |
| model_step Next Steps and Tasks Manager | 0 → 3 | 0 → 0 (0%) | not measured → 17622 ms | 0 | Insufficient traffic 3 requests, under the 20 Libra needs |
| model_inference Next Steps and Tasks Manager | 0 → 3 | 0 → 0 (0%) | not measured → 17616 ms | 0 | Insufficient traffic 3 requests, under the 20 Libra needs |
| /api/trpc/userTasks.listUserTasks | 7 → 3 | 0 → 0 (0%) | 378 ms → 203 ms | 0 | Insufficient traffic 3 requests, under the 20 Libra needs |
| POST /api/trpc/userTasks.listUserTasks | 4 → 2 | 0 → 0 (0%) | 378 ms → 203 ms | 0 | Insufficient traffic 2 requests, under the 20 Libra needs |
| invoke_agent Next Steps and Tasks Manager | 0 → 1 | 0 → 0 (0%) | not measured → 29048 ms | 0 | Insufficient traffic 1 request, under the 20 Libra needs |
| /api/trpc/userTasks.listUserTasks | 0 → 0 | 0 → 0 (0%) | not measured | 0 | No traffic No requests recorded since this deploy. |
| POST /api/trpc/userTasks.listUserTasks | 0 → 0 | 0 → 0 (0%) | not measured | 0 | No traffic No requests recorded since this deploy. |
| model_chunk Next Steps and Tasks Manager | 8 → 0 | 0 → 0 (0%) | 1 ms → not measured | 0 | No traffic No requests recorded since this deploy. |
| model_step Next Steps and Tasks Manager | 3 → 0 | 0 → 0 (0%) | 71504 ms → not measured | 0 | No traffic No requests recorded since this deploy. |
| invoke_agent Next Steps and Tasks Manager | 0 → 0 | 0 → 0 (0%) | not measured | 0 | No traffic No requests recorded since this deploy. |
| model_inference Next Steps and Tasks Manager | 3 → 0 | 0 → 0 (0%) | 71504 ms → not measured | 0 | No traffic No requests recorded since this deploy. |