ref(review-desk): Deliver every Review item change through one path

TELEMETRY.md

@@ -537,6 +537,33 @@ Pivot from create confirmation or Tasks page with `app.task.id`. From a fire
 conversation, use `app.dispatch.id` or `agent-dispatch:<dispatch_id>` as
 `gen_ai.conversation.id`.
 
+### Review items
+
+A Review item change did not reach a linked Conversation, or a Slack card did
+not change.
+
+Spans: `review_item.deliver` wraps the work after each change: the surfaces
+show the new state, then the linked Conversations get their messages. Parent
+spans are the agent tool call (`gen_ai.execute_tool` for `addReviewItem` or
+`updateReviewItem`), the review desk API request,
+`chat.slack_review_item_click` / `chat.slack_review_item_submit`, or the
+Slack event or dashboard message request of a reply in the thread.
+
+Events: `review_item.message.failed` (one linked Conversation did not get the
+message; the change call fails and a retry sends only missing messages),
+`review_item.slack_card.sync.failed` (cards did not change; the next
+delivery of the item shows its latest state),
+`review_item.slack_input.persist.failed` (the Slack thread of a card click
+got a message instead of Slack input)
+
+Attributes: `app.review_item.id`, `app.review_item.change` (`content` for an
+agent content change, else `taken`, `submitted`, `completed`, `dismissed`,
+`replied`, or `closed`), `app.review_item.kind`, `app.review_item.status`,
+`app.review_item.visibility`, `app.review_item.conversation_count`,
+`gen_ai.conversation.id` (the Conversation that missed a message)
+
+Pivot from a review desk item with `app.review_item.id`.
+
 ### Auth And Resume
 
 A turn parked for auth, resumed late, or failed after callback.

packages/junior/.dependency-cruiser.mjs

@@ -49,6 +49,18 @@ export default {
         pathNot: "^src/chat/slack/timestamp\\.ts$",
       },
     },
+    {
+      name: "no-review-items-to-slack",
+      comment:
+        "Review item code is provider-neutral. App setup injects Slack cards and mentions; see src/chat/review-items/README.md.",
+      severity: "error",
+      from: {
+        path: "^src/chat/(review-items/|tools/review-items\\.ts$)",
+      },
+      to: {
+        path: "^src/chat/slack/",
+      },
+    },
     {
       name: "no-slack-sdk-outside-provider-modules",
       comment:

packages/junior/src/api.ts

@@ -25,7 +25,9 @@ import {
   statsReportSchema,
 } from "./api/schema";
 import { readStatsReport } from "./api/stats";
+import { getProductionSlackAdapter } from "./chat/app/production";
 import { logException } from "./chat/logging";
+import { createSlackReviewItemShow } from "./chat/slack/review-item-card-sync";
 import type { ConversationWorkQueue } from "./chat/task-execution/queue";
 import { getVercelConversationWorkQueue } from "./chat/task-execution/vercel-queue";
 import {
@@ -89,7 +91,15 @@ export function createJuniorApi(
       queue,
     }),
   );
-  app.route("/api/review-items", createReviewItemRoutes({ queue }));
+  app.route(
+    "/api/review-items",
+    createReviewItemRoutes({
+      delivery: {
+        queue,
+        show: createSlackReviewItemShow(getProductionSlackAdapter),
+      },
+    }),
+  );
   app.route("/api/code", createCodeRoutes());
   app.route("/api/connections", createConnectionRoutes());
   app.route("/api/personal-tokens", createPersonalTokenRoutes());

packages/junior/src/api/conversations/create.ts

@@ -15,6 +15,7 @@ import { getConversationWorkState } from "@/chat/task-execution/store";
 import type { ConversationWorkQueue } from "@/chat/task-execution/queue";
 import { getProductionSlackAdapter } from "@/chat/app/production";
 import { closeReviewItemsAnsweredByReply } from "@/chat/review-items/actions";
+import { createSlackReviewItemShow } from "@/chat/slack/review-item-card-sync";
 import { getStateAdapter } from "@/chat/state/adapter";
 import { throwApiError } from "../http";
 import type {
@@ -125,8 +126,10 @@ export async function appendConversationMessageForViewer(
   const closeAnswered = () =>
     closeReviewItemsAnsweredByReply({
       conversationId,
-      getSlackAdapter: getProductionSlackAdapter,
-      queue: services.queue,
+      delivery: {
+        queue: services.queue,
+        show: createSlackReviewItemShow(getProductionSlackAdapter),
+      },
       resolveUser: async () => viewer,
       text: body.message,
     });

packages/junior/src/api/review-items/routes.ts

@@ -15,7 +15,6 @@ import {
 } from "@/api/schema/review-item";
 import { validateRequest } from "@/api/validation";
 import { requireViewer } from "@/api/viewer";
-import { getProductionSlackAdapter } from "@/chat/app/production";
 import { getDb } from "@/chat/db";
 import {
   canActOnReviewItem,
@@ -33,7 +32,7 @@ import {
   searchReviewItems,
   type ReviewItem,
 } from "@/chat/review-items/store";
-import type { ConversationWorkQueue } from "@/chat/task-execution/queue";
+import type { ReviewItemDelivery } from "@/chat/review-items/changes";
 
 const LIST_LIMIT = 100;
 
@@ -88,7 +87,7 @@ function actionErrorResponse(error: unknown): Response {
 
 /** Create the authenticated review desk routes. */
 export function createReviewItemRoutes(services: {
-  queue: ConversationWorkQueue;
+  delivery: ReviewItemDelivery;
 }): Hono<JuniorApiEnv> {
   const app = new Hono<JuniorApiEnv>();
   app.get(
@@ -163,9 +162,8 @@ export function createReviewItemRoutes(services: {
       try {
         const item = await actOnReviewItem({
           action: body.action,
-          getSlackAdapter: getProductionSlackAdapter,
+          delivery: services.delivery,
           id,
-          queue: services.queue,
           user: viewer,
           ...(body.action === "submit"
             ? { answer: { ...body.answer, requestId: id } }

packages/junior/src/app.ts

@@ -594,6 +594,7 @@ export async function createApp(
       bindMessageAgent(request, { queue: conversationWorkQueue }),
     bindSpawnAgent: (request) =>
       bindSpawnAgent(request, { queue: conversationWorkQueue }),
+    conversationWorkQueue,
     tracePropagation,
   });
   const runtimeServiceOverrides = {

packages/junior/src/chat/agent/tools.ts

@@ -476,6 +476,7 @@ export async function wireAgentTools(
     ...commonToolRuntimeContext,
     ...toolRoute,
     attachmentStorage: args.run.environment?.attachmentStorage,
+    conversationWorkQueue: args.run.environment?.conversationWorkQueue,
     workspaces: {
       activeWorkspaceId: () => agentSandbox.sandboxRef()?.workspaceId,
       switch: agentSandbox.switchWorkspace,

packages/junior/src/chat/agent/types.ts

@@ -33,6 +33,7 @@ import type {
 } from "@/chat/tools/types";
 import type { AssistantMessage } from "@earendil-works/pi-ai";
 import type { AttachmentStorage } from "@/chat/attachments/storage";
+import type { ConversationWorkQueue } from "@/chat/task-execution/queue";
 
 /** One attachment the model may see for the current instruction. */
 export type AgentAttachment = {
@@ -216,6 +217,8 @@ export type AgentEvent =
 export type AgentEnvironment = {
   attachmentStorage?: AttachmentStorage;
   configuration?: Record<string, unknown>;
+  /** Queue of the app, for tools that wake other Conversations. */
+  conversationWorkQueue?: ConversationWorkQueue;
   locationConfiguration?: LocationConfigurationService;
   skillDirs?: string[];
   /** Per-slice override for app-owned sandbox egress trace propagation. */

packages/junior/src/chat/ingress/slack-review-item.ts

@@ -14,9 +14,10 @@ import {
   type ReviewItemAction,
 } from "@/chat/review-items/actions";
 import {
-  renderReviewItemMessage,
-  sendReviewItemMessages,
-} from "@/chat/review-items/messages";
+  deliverReviewItemChange,
+  type ReviewItemDelivery,
+} from "@/chat/review-items/changes";
+import { renderReviewItemMessage } from "@/chat/review-items/messages";
 import type { ReviewItem } from "@/chat/review-items/store";
 import { parseSlackChannelId } from "@/chat/slack/ids";
 import {
@@ -29,6 +30,7 @@ import {
   type SlackReviewItemClick,
 } from "@/chat/slack/review-item-card";
 import {
+  createSlackReviewItemShow,
   readSlackReviewItemCardConversationId,
   syncReviewItemSlackCards,
 } from "@/chat/slack/review-item-card-sync";
@@ -151,6 +153,10 @@ async function act(args: {
       ...target,
       itemId: args.itemId,
     })) ?? `slack:${target.channelId}:${target.threadTs}`;
+  const delivery: ReviewItemDelivery = {
+    queue: services.queue,
+    show: createSlackReviewItemShow(() => services.adapter),
+  };
   let item: ReviewItem;
   try {
     const current = await readReviewItemForUser(actor.user, args.itemId);
@@ -160,9 +166,8 @@ async function act(args: {
       // Two clicks can both see an open item. Only the click that closes it
       // adds the input to this thread, so one action adds one Turn.
       firstCloseOnly: true,
-      getSlackAdapter: () => services.adapter,
+      delivery,
       id: current.id,
-      queue: services.queue,
       user: actor.user,
       ...(args.answer ? { answer: args.answer(current) } : undefined),
     });
@@ -194,11 +199,11 @@ async function act(args: {
     logException(error, "review_item.slack_input.persist.failed", {
       "app.review_item.id": item.id,
     });
-    await sendReviewItemMessages({
+    await deliverReviewItemChange({
       change,
+      delivery,
       item,
       nowMs: Date.now(),
-      queue: services.queue,
     });
   }
 }

packages/junior/src/chat/ingress/slack-webhook.ts

@@ -78,6 +78,7 @@ import { bindMessageAuthorIdentityId } from "@/chat/services/message-actor-ident
 import { createUserTokenStore } from "@/chat/capabilities/factory";
 import type { UserTokenStore } from "@/chat/credentials/user-token-store";
 import { publishAppHomeView } from "@/chat/slack/app-home";
+import { createSlackReviewItemShow } from "@/chat/slack/review-item-card-sync";
 import { presentSlackAutomationDetails } from "@/chat/slack/automation-details";
 import { getSlackClient } from "@/chat/slack/client";
 import { createSlackWebApiStatusSender } from "@/chat/slack/assistant-thread/status-send";
@@ -623,8 +624,10 @@ async function routeParsedMessage(args: {
     if (message.author.isBot === true) return;
     await closeReviewItemsAnsweredByReply({
       conversationId,
-      getSlackAdapter: () => args.adapter,
-      queue: args.queue,
+      delivery: {
+        queue: args.queue,
+        show: createSlackReviewItemShow(() => args.adapter),
+      },
       resolveUser: async () => {
         const teamId = args.installation.teamId;
         const userId = parseActorUserId(message.author.userId);

packages/junior/src/chat/review-items/README.md

@@ -120,9 +120,29 @@ message. A Slack person without a Junior User closes only items with no
 assignees. The message is the answer in its own Conversation. Each other
 linked Conversation gets an Event. The first close wins, so a reply after a
 click changes nothing. A retry of the reply, or a later reply from the same
-person, sends the Events that are missing for the items that the person
-closed by reply. Each Event has a key, so an Event that was sent does not
-go again.
+person, delivers again each item that the person closed by reply. The cards
+show the close again, and only the missing Events go. Each Event has a key,
+so an Event that was sent does not go again.
+
+## After a change
+
+Every change goes through one path: `deliverReviewItemChange` in
+`changes.ts`. The agent tools, the person actions, and a reply in the
+thread call it after they save the change.
+
+- First, every surface shows the new state. App setup gives the `show`
+  function. Today it is the Slack card sync. Review item code does not import
+  Slack code (`no-review-items-to-slack` in `.dependency-cruiser.mjs`).
+- Then a person action or an agent close goes to the other linked
+  Conversations as messages. An agent change to the content sends no
+  messages.
+- Delivery runs inline, after the change is saved. A failed message fails the
+  call that made the change. The caller can retry: a retry sends only the
+  messages that are missing. A failed card is logged. The next delivery of
+  the item shows its latest state.
+- App setup also gives the agent tools `findMentionedUserId`. The Slack
+  version (`chat/slack/review-item-tools.ts`) resolves a Slack mention to a
+  User through the Slack Identity in the workspace of the item.
 
 ## Messages to linked Conversations
 
@@ -160,8 +180,8 @@ each card, so Junior can change it in place.
   state. A closed card has no controls and shows who closed it.
 - A card in a thread that the item is no longer linked to loses its
   content.
-- Cards are optional UI. A Slack failure is logged and does not fail the
-  change.
+- Cards are optional UI. A Slack failure is logged
+  (`review_item.slack_card.sync.failed`) and does not fail the change.
 - Syncs of one item run one at a time under a state lock (`withLock` in
   `chat/state/locks.ts`). So two syncs cannot post two cards in one thread.
   Each sync reads the latest item, so a sync that waits shows the newest