Repository navigation
fix(review-desk): Close Review items that no agent can act on - #2506
Merged
Merged
Conversation
Co-Authored-By: David Cramer <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
A recurring Automation refreshes its Review item by dedupe key, but the item keeps the Conversation of its first run. Purge of that Conversation closed the item while the Automation was still live. Purge now skips Automation items, and an Automation delete closes them. - Dismiss Automation items only after the delete returns a row, under the Automation lock. - Check the allowed domain of a subscribed reply only when an open item can match, so most replies need no Slack profile lookup. - Remove the plugin close path from the tool text and README. It does not exist yet. - Split the attachment retention tests into their own file, with shared seed helpers, to keep retention.test.ts under the length limit. Co-Authored-By: David Cramer <[email protected]>
dcramer
approved these changes
Nov 1, 2026
sentry-junior Bot
added a commit
that referenced
this pull request
Nov 1, 2026
…vate items A purge already leaves an open item of an Automation open, because a later run can still change it (#2506). The private item delete did not skip it, so a purge of the first run removed an item that the Automation still updates. Skip open Automation items in the delete and in the purge selection, so the next purge deletes the item after it closes. Co-Authored-By: David Cramer <[email protected]>
This was referenced Nov 1, 2026
dcramer
added a commit
that referenced
this pull request
Nov 2, 2026
…swers (#2525) A person answers one Review item in a thread, and every other open item for them in that thread closes too. `closeReviewItemsAnsweredByReply` closed every open action item that the replier could answer in the Conversation, and saved the same reply text as the answer to each. Since #2506, a plain reply in a subscribed thread runs this path too. So one typed reply cleared all open questions with the wrong answers. Now the decision model checks the reply before anything closes (`review-items/reply-match.ts`). One `completeDecision` call (the same decision model as the passive reply router, prompt `junior.review_item_reply_match`) asks one `answers` / `unrelated` question for each open item that the replier can answer. Only items with `answers` at p >= 0.7 close. If the decision fails or takes more than 10 seconds, all items stay open and the agent still gets the reply. In Slack, the match runs after the ingress lock is released, so a slow decision does not block the thread. The `addReviewItem` / update tool descriptions and `review-items/AGENTS.md` describe this. The matcher calls `completeDecision` directly. No Slack or dashboard service takes a review-item decision function. **Tests and evals.** Which items a reply answers is a model choice, so it is tested in evals, not with a fake: - New `classifiers` eval suite (`evals/classifiers/**`, hard pass/fail, own config, global setup, and `evals-classifiers.yml` workflow). `review-item-replies.eval.ts` checks the exact set of items that a reply answers with the real decision model: picks an option, acknowledgement, follow-up question, one of two similar questions, both questions, a text answer, and an unrelated message. The workflow runs when the suite, the harness, `reply-match.ts`, or `pi/client.ts` changes. - The Slack integration test has no model fake. It covers the rules that need no model choice: a non-assignee or a Slack author outside the allowed domains cannot close an item, and a retry of a reply that already closed an item delivers it again (card update and the other linked thread). Card clicks and desk submits were not affected: they close only their own item and do not use the reply path. Remaining risk: each thread reply makes one decision-model call while the replier has open items in that thread. <!-- junior-request-attribution:start --> via @dcramer. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3AC0B595QDZLL%3A1793593475.449169 --> -- [View Session](https://junior-prod.sentry.dev/conversations/slack%3AC0B595QDZLL%3A1793593475.449169) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC0B595QDZLL%3A1793593475.449169/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: David Cramer <[email protected]> Co-authored-by: David Cramer <[email protected]>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #2488
Some Review items stayed open forever, because no agent could act on them any more. Some questions also stayed open after a person answered in the thread.
automation_id. An Automation delete closes those.deleteAutomationonly soft-deletes the row, so theset nullforeign key never runs. It now dismisses the open items of the Automation in the same transaction, with the reason "Automation deleted". The dismiss runs only after the delete returns a row, so it runs under the Automation lock.updateReviewItemdescription said that an item never closes on its own. They now list every way an item closes on main today.A purge or an Automation delete sends no message and does not change the Slack cards. A click on an old card tells the person that the item is closed.
retention.test.tswent over the 1,000-line limit, so the attachment retention tests move toretention-attachments.test.ts, with shared seed helpers intests/fixtures/conversation-retention.ts. The tests do not change.via David Cramer.
--
View Session [Sentry]