fix(productivity-review): regenerate description when trigger flips (BLO-22105)

server/src/__tests__/productivity-review-service.test.ts

@@ -542,6 +542,243 @@ describeEmbeddedPostgres("productivity review service", () => {
     expect(await listRefreshComments(review!.id)).toHaveLength(DEFAULT_PRODUCTIVITY_REVIEW_MAX_REFRESH_COMMENTS);
   });
 
+  describe("BLO-22105: refresh regenerates the description on a trigger flip", () => {
+    it("regenerates the Manager Decision block when a no_comment_streak review flips to runtime_failure_streak", async () => {
+      const now = new Date("2026-04-28T12:00:00.000Z");
+      const seeded = await seedAssignedIssue();
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now,
+      });
+
+      const service = productivityReviewService(db);
+      await service.reconcileProductivityReviews({ now, companyId: seeded.companyId });
+      const [review] = await listProductivityReviews(seeded.companyId);
+      expect(review?.description).toContain("Primary trigger: `no_comment_streak`");
+      expect(review?.description).toContain("Request decomposition");
+
+      const refreshAt = new Date(now.getTime() + DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS);
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now: refreshAt,
+        status: "failed",
+        livenessState: "failed",
+        usageJson: { inputTokens: 0, outputTokens: 0 },
+      });
+
+      const refresh = await service.reconcileProductivityReviews({ now: refreshAt, companyId: seeded.companyId });
+      expect(refresh.updated).toBe(1);
+
+      const [refreshedReview] = await listProductivityReviews(seeded.companyId);
+      expect(refreshedReview?.id).toBe(review!.id);
+      expect(refreshedReview?.description).toContain("Primary trigger: `runtime_failure_streak`");
+      expect(refreshedReview?.description).toContain("do not decompose, block, or cancel");
+      expect(refreshedReview?.description).not.toContain("Request decomposition");
+    });
+
+    it("regenerates the Manager Decision block when a runtime_failure_streak review flips to no_comment_streak", async () => {
+      const now = new Date("2026-04-28T12:00:00.000Z");
+      const seeded = await seedAssignedIssue();
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now,
+        status: "failed",
+        livenessState: "failed",
+        usageJson: { inputTokens: 0, outputTokens: 0 },
+      });
+
+      const service = productivityReviewService(db);
+      await service.reconcileProductivityReviews({ now, companyId: seeded.companyId });
+      const [review] = await listProductivityReviews(seeded.companyId);
+      expect(review?.description).toContain("Primary trigger: `runtime_failure_streak`");
+      expect(review?.description).toContain("do not decompose, block, or cancel");
+
+      const refreshAt = new Date(now.getTime() + DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS);
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now: refreshAt,
+      });
+
+      const refresh = await service.reconcileProductivityReviews({ now: refreshAt, companyId: seeded.companyId });
+      expect(refresh.updated).toBe(1);
+
+      const [refreshedReview] = await listProductivityReviews(seeded.companyId);
+      expect(refreshedReview?.id).toBe(review!.id);
+      expect(refreshedReview?.description).toContain("Primary trigger: `no_comment_streak`");
+      expect(refreshedReview?.description).toContain("Request decomposition");
+      expect(refreshedReview?.description).not.toContain("do not decompose, block, or cancel");
+    });
+
+    it("does not rewrite the description when the refresh observes the same trigger", async () => {
+      const now = new Date("2026-04-28T12:00:00.000Z");
+      const seeded = await seedAssignedIssue();
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now,
+      });
+
+      const service = productivityReviewService(db);
+      await service.reconcileProductivityReviews({ now, companyId: seeded.companyId });
+      const [review] = await listProductivityReviews(seeded.companyId);
+      const originalDescription = review!.description;
+
+      const refreshAt = new Date(now.getTime() + DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS);
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now: refreshAt,
+      });
+
+      const refresh = await service.reconcileProductivityReviews({ now: refreshAt, companyId: seeded.companyId });
+      expect(refresh.updated).toBe(1);
+
+      const [refreshedReview] = await listProductivityReviews(seeded.companyId);
+      expect(refreshedReview?.description).toBe(originalDescription);
+    });
+
+    it("still regenerates a stale description after the refresh-comment cap is reached, without posting another comment", async () => {
+      const now = new Date("2026-04-28T12:00:00.000Z");
+      const seeded = await seedAssignedIssue();
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now,
+      });
+
+      const service = productivityReviewService(db);
+      await service.reconcileProductivityReviews({ now, companyId: seeded.companyId });
+      const [review] = await listProductivityReviews(seeded.companyId);
+
+      // Exhaust the DEFAULT_PRODUCTIVITY_REVIEW_MAX_REFRESH_COMMENTS cap (3)
+      // with unchanged-trigger refreshes, exactly like the existing
+      // "caps refresh comments" coverage above.
+      let cursor = now;
+      for (let i = 0; i < DEFAULT_PRODUCTIVITY_REVIEW_MAX_REFRESH_COMMENTS; i += 1) {
+        cursor = new Date(cursor.getTime() + DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS);
+        await insertRuns({
+          companyId: seeded.companyId,
+          agentId: seeded.coderId,
+          issueId: seeded.issueId,
+          count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+          now: cursor,
+        });
+        await service.reconcileProductivityReviews({ now: cursor, companyId: seeded.companyId });
+      }
+      expect(await listRefreshComments(review!.id)).toHaveLength(DEFAULT_PRODUCTIVITY_REVIEW_MAX_REFRESH_COMMENTS);
+
+      // The cap is now hit. A further refresh that only repeats the same
+      // trigger should stay throttled...
+      const stillNoOpAt = new Date(cursor.getTime() + DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS);
+      const noOpRefresh = await service.reconcileProductivityReviews({
+        now: stillNoOpAt,
+        companyId: seeded.companyId,
+      });
+      expect(noOpRefresh.updated).toBe(0);
+      expect(noOpRefresh.existing).toBe(1);
+
+      // ...but a trigger flip must still correct the Manager Decision block —
+      // the comment cap bounds comment churn, not correctness of the guidance.
+      const flipAt = new Date(stillNoOpAt.getTime() + DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS);
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now: flipAt,
+        status: "failed",
+        livenessState: "failed",
+        usageJson: { inputTokens: 0, outputTokens: 0 },
+      });
+      const flipRefresh = await service.reconcileProductivityReviews({ now: flipAt, companyId: seeded.companyId });
+      expect(flipRefresh.updated).toBe(1);
+
+      const [afterFlip] = await listProductivityReviews(seeded.companyId);
+      expect(afterFlip?.description).toContain("Primary trigger: `runtime_failure_streak`");
+      expect(afterFlip?.description).toContain("do not decompose, block, or cancel");
+      // No new comment: the cap still bounds comment churn.
+      expect(await listRefreshComments(review!.id)).toHaveLength(DEFAULT_PRODUCTIVITY_REVIEW_MAX_REFRESH_COMMENTS);
+    });
+
+    it("preserves a description edited concurrently with a trigger-flip refresh instead of clobbering it", async () => {
+      const now = new Date("2026-04-28T12:00:00.000Z");
+      const seeded = await seedAssignedIssue();
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now,
+      });
+
+      const service = productivityReviewService(db);
+      await service.reconcileProductivityReviews({ now, companyId: seeded.companyId });
+      const [review] = await listProductivityReviews(seeded.companyId);
+      expect(review?.description).toContain("Primary trigger: `no_comment_streak`");
+
+      const refreshAt = new Date(now.getTime() + DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS);
+      await insertRuns({
+        companyId: seeded.companyId,
+        agentId: seeded.coderId,
+        issueId: seeded.issueId,
+        count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
+        now: refreshAt,
+        status: "failed",
+        livenessState: "failed",
+        usageJson: { inputTokens: 0, outputTokens: 0 },
+      });
+
+      const concurrentlyEditedDescription = `${review!.description}\n\nManager note: escalating directly, do not overwrite.`;
+
+      // Simulate a human editing the review issue's description directly
+      // (e.g. via the issues API) in the window between this refresh's outer
+      // read of `existing` and the transaction's guarded UPDATE. The
+      // advisory lock only serializes this refresh path against itself; it
+      // says nothing about a plain issue edit landing concurrently.
+      const originalTransaction = db.transaction.bind(db);
+      const transactionSpy = vi.spyOn(db, "transaction").mockImplementation(
+        (async (...args: Parameters<typeof db.transaction>) => {
+          await db
+            .update(issues)
+            .set({ description: concurrentlyEditedDescription })
+            .where(eq(issues.id, review!.id));
+          return originalTransaction(...args);
+        }) as typeof db.transaction,
+      );
+
+      try {
+        const refresh = await service.reconcileProductivityReviews({ now: refreshAt, companyId: seeded.companyId });
+        // The refresh comment still gets appended — only the description
+        // overwrite lost the race.
+        expect(refresh.updated).toBe(1);
+      } finally {
+        transactionSpy.mockRestore();
+      }
+
+      const [afterRefresh] = await listProductivityReviews(seeded.companyId);
+      expect(afterRefresh?.description).toBe(concurrentlyEditedDescription);
+      expect(afterRefresh?.description).not.toContain("Primary trigger: `runtime_failure_streak`");
+    });
+  });
+
   it("allows only one productivity review per source issue in 24 hours", async () => {
     const now = new Date("2026-04-28T12:00:00.000Z");
     const seeded = await seedAssignedIssue();

server/src/services/productivity-review.ts

@@ -722,6 +722,25 @@ function formatTrigger(trigger: ProductivityReviewTrigger) {
   return "Long active duration";
 }
 
+const PRODUCTIVITY_REVIEW_TRIGGERS: readonly ProductivityReviewTrigger[] = [
+  "no_comment_streak",
+  "long_active_duration",
+  "high_churn",
+  "runtime_failure_streak",
+];
+
+// BLO-22105: `buildReviewMarkdown` bakes the trigger that produced it into the
+// `- Primary trigger:` line. Reading it back out of the persisted description
+// (rather than, say, the last activity-log entry) means the comparison is
+// against exactly what a reader currently sees, so a refresh regenerates
+// precisely when the visible Manager Decision guidance is actually stale.
+function extractReviewTriggerFromDescription(description: string | null): ProductivityReviewTrigger | null {
+  if (!description) return null;
+  const match = description.match(/^- Primary trigger: `([a-z_]+)`/m);
+  const candidate = match?.[1];
+  return PRODUCTIVITY_REVIEW_TRIGGERS.find((trigger) => trigger === candidate) ?? null;
+}
+
 // True when a run's most recent classification is `failed` liveness AND it
 // burned zero input+output tokens. That combination means the agent never
 // got a model turn — the runtime crashed, the process was killed, or every
@@ -2567,20 +2586,59 @@ export function productivityReviewService(db: Db, deps?: ProductivityReviewServi
 
         const refreshState = await getRefreshCommentState(evidence.sourceIssue.companyId, existing.id, tx);
         const lastRefreshAt = refreshState.latestCreatedAt ?? existing.createdAt;
-        if (
-          refreshState.count >= opts.thresholds.maxRefreshComments ||
-          evidence.generatedAt.getTime() - lastRefreshAt.getTime() < effectiveRefreshIntervalMs
-        ) {
+        // The hard-floor interval gates everything below, including the
+        // description rewrite — a trigger flip must not be usable to force
+        // more writes than a normal refresh already allows.
+        if (evidence.generatedAt.getTime() - lastRefreshAt.getTime() < effectiveRefreshIntervalMs) {
           return { throttled: true as const, lastRefreshAt };
         }
 
-        await addRefreshComment(
-          existing.id,
-          buildRefreshComment(evidence, opts.prefix),
-          evidence.generatedAt,
-          tx,
-        );
-        return { throttled: false as const, lastRefreshAt };
+        // BLO-22105: the Manager Decision block is trigger-conditional (see
+        // buildReviewMarkdown), so a review whose live trigger has flipped since
+        // it was created/last regenerated is showing stale — potentially
+        // under-enforcing — remedy guidance. Regenerate only on an actual flip
+        // (never on unparseable/legacy descriptions) and only inside this same
+        // throttle-gated branch, so a trigger flip cannot be used to force a
+        // description write more often than the hard-floor interval allows.
+        const previousTrigger = extractReviewTriggerFromDescription(existing.description);
+        const descriptionStale = previousTrigger !== null && previousTrigger !== evidence.trigger;
+        let descriptionRegenerated = false;
+        if (descriptionStale) {
+          // `existing.description` was read outside this transaction. The
+          // advisory lock only serializes this refresh path against itself —
+          // it says nothing about a human editing the review issue's
+          // description directly in between. Guard the overwrite with the
+          // description we actually read so a concurrent edit loses the race
+          // cleanly (0 rows matched, nothing clobbered) instead of being
+          // silently discarded.
+          const [updatedRow] = await tx
+            .update(issues)
+            .set({ description: buildReviewMarkdown(evidence, opts.prefix), updatedAt: evidence.generatedAt })
+            .where(and(eq(issues.id, existing.id), eq(issues.description, existing.description as string)))
+            .returning({ id: issues.id });
+          descriptionRegenerated = updatedRow !== undefined;
+        }
+
+        // `maxRefreshComments` bounds refresh-comment churn, not the
+        // correctness of the durable Manager Decision guidance. Gating the
+        // description rewrite on it too would mean a review that outlives the
+        // cap could never self-correct after a trigger flip — exactly the
+        // staleness this fix exists to close. Only the comment emission is
+        // capped; the interval check above still applies to both.
+        const commentCapped = refreshState.count >= opts.thresholds.maxRefreshComments;
+        if (!commentCapped) {
+          await addRefreshComment(
+            existing.id,
+            buildRefreshComment(evidence, opts.prefix),
+            evidence.generatedAt,
+            tx,
+          );
+        }
+
+        if (commentCapped && !descriptionRegenerated) {
+          return { throttled: true as const, lastRefreshAt };
+        }
+        return { throttled: false as const, lastRefreshAt, descriptionRegenerated };
       });
 
       if (refreshOutcome.throttled) {
@@ -2591,7 +2649,7 @@ export function productivityReviewService(db: Db, deps?: ProductivityReviewServi
             lastRefreshAt: refreshOutcome.lastRefreshAt.toISOString(),
             minIntervalMs: effectiveRefreshIntervalMs,
           },
-          "productivity review refresh throttled: previous refresh within hard-floor window",
+          "productivity review refresh throttled: within hard-floor window or comment cap reached with no stale description to fix",
         );
         return { kind: "existing" as const, reviewIssueId: existing.id };
       }
@@ -2610,6 +2668,7 @@ export function productivityReviewService(db: Db, deps?: ProductivityReviewServi
           noCommentStreak: evidence.noCommentStreak,
           runCountLastHour: evidence.runCountLastHour,
           commentCountLastHour: evidence.commentCountLastHour,
+          descriptionRegenerated: refreshOutcome.descriptionRegenerated,
         },
       });
       return { kind: "updated" as const, reviewIssueId: existing.id };