fix(plannotator): recover orphaned reviews and report already-active refusals

packages/bb-plugin-plannotator/README.md

@@ -21,6 +21,8 @@ For development, offline use, or an independently updated upstream build, set **
 
 Start a new agent session after installing or reloading. When the agent explicitly calls `plannotator_review_plan`, BB opens the upstream UI as a persistent right-panel tab. The tool returns a review ID promptly, and the agent ends its turn. Approve or annotate in the tab. BB then closes it and sends the decision as a new message in the same thread, which wakes the agent. The message is queued if the agent is still working. There is no BB review deadline or hidden interaction countdown: the review remains open until Plannotator returns a decision or you use **Cancel review** in the tab. Plannotator is optional, and its approval is not a permission grant. The child receives the current BB provider identity explicitly, rather than inferring it from unrelated host environment variables.
 
+Only one review can be pending per thread. If you close its tab, open the **Plannotator** thread panel action to bring the pending review back. A second `plannotator_review_plan` call while a review is pending returns `status: "already_active"` with the pending review ID; it does not start a new review and is not a cancellation.
+
 BB stores the review state until it delivers the decision. If the plugin restarts during a review, it sends a cancellation message after restarting. If delivery fails temporarily, it retries. The agent can call `plannotator_review_status` with the review ID to recover a missing result; it does not need to poll during a normal review.
 
 Plannotator's plan history and configuration are stored under the plugin's BB data directory. Local child sessions use BB's session-scoped same-origin relay, and BB acknowledges the current upstream look-and-feel announcement before mounting the iframe, so the standalone setup wizard does not interrupt each review. The upstream plan UI remains otherwise unmodified. **Open externally** is available as a fallback, but external browser tabs may have a separate cookie jar from the BB panel.

packages/bb-plugin-plannotator/app.tsx

@@ -72,6 +72,67 @@ function parseReviewOpened(value: unknown): PlannotatorPayload | null {
   return parsePayload(value.payload);
 }
 
+function parseReviewClosed(
+  value: unknown,
+): { threadId: string; sessionId: string } | null {
+  if (
+    !isRecord(value) ||
+    value.kind !== "review-closed" ||
+    typeof value.threadId !== "string" ||
+    typeof value.sessionId !== "string"
+  ) {
+    return null;
+  }
+  return { threadId: value.threadId, sessionId: value.sessionId };
+}
+
+/**
+ * A panel opened from the thread action carries no params. Recover the
+ * thread's pending review from the server so a review whose tab was closed
+ * can still be approved, annotated, or cancelled.
+ */
+function useActiveReviewFallback(
+  threadId: string,
+  enabled: boolean,
+): PlannotatorPayload | null {
+  const rpc = useRpc<typeof rpcContract>();
+  const realtimeConnectionState = useRealtimeConnectionState();
+  const [active, setActive] = useState<PlannotatorPayload | null>(null);
+
+  useRealtime(PLANNOTATOR_REALTIME_CHANNEL, (value) => {
+    if (!enabled) return;
+    const opened = parseReviewOpened(value);
+    if (opened && opened.threadId === threadId) {
+      setActive(opened);
+      return;
+    }
+    const closed = parseReviewClosed(value);
+    if (closed && closed.threadId === threadId) {
+      setActive((current) =>
+        current?.sessionId === closed.sessionId ? null : current,
+      );
+    }
+  });
+
+  useEffect(() => {
+    if (!enabled || realtimeConnectionState !== "connected") return;
+    let cancelled = false;
+    void rpc
+      .call("getActiveReview", { threadId })
+      .then((value) => {
+        if (cancelled) return;
+        const payload = parsePayload(value);
+        setActive(payload && payload.threadId === threadId ? payload : null);
+      })
+      .catch(() => {});
+    return () => {
+      cancelled = true;
+    };
+  }, [enabled, realtimeConnectionState, rpc, threadId]);
+
+  return enabled ? active : null;
+}
+
 /**
  * Server-side tool calls can persist a tab, but only the app navigation API
  * can focus/open the side panel. This deliberately renders no control: it is
@@ -139,7 +200,9 @@ function EmptyPanel() {
 }
 
 function PlannotatorPanel({ threadId, params }: PluginThreadPanelProps) {
-  const payload = parsePayload(params);
+  const paramsPayload = parsePayload(params);
+  const fallbackPayload = useActiveReviewFallback(threadId, !paramsPayload);
+  const payload = paramsPayload ?? fallbackPayload;
   const rpc = useRpc<typeof rpcContract>();
   const [reloadKey, setReloadKey] = useState(0);
   const [loaded, setLoaded] = useState(false);
@@ -165,6 +228,7 @@ function PlannotatorPanel({ threadId, params }: PluginThreadPanelProps) {
   // browser-facing hostname. This keeps the real upstream UI while removing
   // standalone-browser onboarding from the embedded workflow.
   useLayoutEffect(() => {
+    setCanceling(false);
     if (!payload) {
       setEmbeddedUrl(null);
       return;
@@ -224,6 +288,9 @@ function PlannotatorPanel({ threadId, params }: PluginThreadPanelProps) {
               setCanceling(true);
               void rpc
                 .call("cancelReview", { threadId, sessionId: payload.sessionId })
+                .then((result) => {
+                  if (!result.cancelled) setCanceling(false);
+                })
                 .catch(() => setCanceling(false));
             }}
           >

packages/bb-plugin-plannotator/package.json

@@ -1,6 +1,6 @@
 {
   "name": "bb-plugin-plannotator",
-  "version": "0.3.0",
+  "version": "0.3.1",
   "type": "module",
   "engines": {
     "bb": ">=0.39",

packages/bb-plugin-plannotator/server.ts

@@ -336,6 +336,24 @@ function errorResponse(message: string) {
   };
 }
 
+// A refusal is not a review outcome. Report it with a distinct status so an
+// agent cannot mistake it for the user cancelling the pending review.
+function alreadyActiveResponse(reviewId: string | undefined) {
+  return {
+    content: [{
+      type: "text" as const,
+      text: JSON.stringify({
+        status: "already_active",
+        source: "plannotator",
+        ...(reviewId ? { reviewId } : {}),
+        error: "A Plannotator review is already pending in this thread. No new review was started.",
+        message: "Ask the user to finish or cancel the pending review. They can reopen it from the Plannotator thread panel action. Its decision will arrive as a new message in this thread.",
+      }),
+    }],
+    isError: true,
+  };
+}
+
 async function getConfiguredPath(settings: { get(): Promise<{ binaryPath: string }> }): Promise<string> {
   const configured = await settings.get();
   return configured.binaryPath.trim() || BUNDLED_BINARY;
@@ -494,11 +512,21 @@ export default async function plugin(bb: BbPluginApi) {
     }
     relaySessions.delete(active.sessionId);
     await closeReviewPanel(bb, threadId, active.sessionId);
+    // Panels opened manually from the thread action carry no session params,
+    // so closeReviewPanel cannot find them. Tell them the review ended.
+    bb.realtime.publish(PLANNOTATOR_REALTIME_CHANNEL, {
+      kind: "review-closed",
+      threadId,
+      sessionId: active.sessionId,
+    });
     try {
       await active.review.stop();
     } catch (error) {
       bb.log.warn(`Could not stop Plannotator review ${active.sessionId}: ${String(error)}`);
     }
+    // The review is over once the upstream process ends. Free the thread now
+    // so a slow or retried decision delivery does not block the next review.
+    if (activeReviews.get(threadId) === active) activeReviews.delete(threadId);
     if (disposing || active.deleted) return;
     const record: StoredReview = {
       id: active.sessionId,
@@ -513,7 +541,6 @@ export default async function plugin(bb: BbPluginApi) {
       bb.log.warn(`Could not save Plannotator decision ${record.id}: ${String(error)}`);
     }
     await deliverReview(record);
-    if (activeReviews.get(threadId) === active) activeReviews.delete(threadId);
   }
 
   registerPlannotatorRelayRoutes(bb, relaySessions);
@@ -558,7 +585,7 @@ export default async function plugin(bb: BbPluginApi) {
     parameters: reviewToolParametersSchema,
     async execute(params, context) {
       if (activeReviews.has(context.threadId) || startingReviews.has(context.threadId)) {
-        return errorResponse("A Plannotator review is already active in this thread.");
+        return alreadyActiveResponse(activeReviews.get(context.threadId)?.sessionId);
       }
       startingReviews.add(context.threadId);
 

packages/bb-plugin-plannotator/skills/plan-review/SKILL.md

@@ -31,6 +31,10 @@ The decision message has one of these results:
 - `decision=cancelled`: report the cancellation, then continue normally unless
   the user specifically asked to stop.
 
+If the tool returns `status=already_active`, no new review started and nothing
+was cancelled. Tell the user a review is still pending (give its `reviewId`)
+and that they can reopen it from the Plannotator thread panel action.
+
 Do not claim a review decision from the presence of a panel or the pending
 tool result alone. Only the later decision message describes what happened in
 Plannotator; it does not authorize or prohibit file changes.

packages/bb-plugin-plannotator/src/app.test.tsx

@@ -144,6 +144,63 @@ describe("Plannotator BB shell", () => {
     );
   });
 
+  it("recovers the pending review when the panel action opens without params", async () => {
+    const slot = renderSlot(
+      app.threadPanelActions[0]!,
+      { threadId: "thread-1", params: {} },
+      {
+        rpc: {
+          getActiveReview: () => payload,
+          cancelReview: () => ({ cancelled: true }),
+        },
+      },
+    );
+
+    const iframe = await slot.findByTitle("Plannotator review");
+    expect(iframe.getAttribute("src")).toContain("sessionId=session-1");
+    expect(slot.rpcCalls).toContainEqual({
+      method: "getActiveReview",
+      input: { threadId: "thread-1" },
+    });
+    expect(slot.queryByText(/Start a plan review from the agent/)).toBeNull();
+
+    fireEvent.click(slot.getByRole("button", { name: "Cancel review" }));
+    await waitFor(() =>
+      expect(slot.rpcCalls).toContainEqual({
+        method: "cancelReview",
+        input: { threadId: "thread-1", sessionId: "session-1" },
+      }),
+    );
+  });
+
+  it("follows review open and close signals in a param-less panel", async () => {
+    const slot = renderSlot(
+      app.threadPanelActions[0]!,
+      { threadId: "thread-1", params: {} },
+      { rpc: { getActiveReview: () => null } },
+    );
+
+    await slot.findByText(/Start a plan review from the agent/);
+    await slot.behavior.emitRealtime(PLANNOTATOR_REALTIME_CHANNEL, {
+      kind: "review-opened",
+      payload: { ...payload, threadId: "thread-2" },
+    });
+    expect(slot.queryByTitle("Plannotator review")).toBeNull();
+
+    await slot.behavior.emitRealtime(PLANNOTATOR_REALTIME_CHANNEL, {
+      kind: "review-opened",
+      payload,
+    });
+    await slot.findByTitle("Plannotator review");
+
+    await slot.behavior.emitRealtime(PLANNOTATOR_REALTIME_CHANNEL, {
+      kind: "review-closed",
+      threadId: "thread-1",
+      sessionId: "session-1",
+    });
+    await slot.findByText(/Start a plan review from the agent/);
+  });
+
   it("exposes explicit cancellation without a host interaction deadline", async () => {
     const slot = renderSlot(
       app.threadPanelActions[0]!,

packages/bb-plugin-plannotator/src/server.test.ts

@@ -304,6 +304,108 @@ describe("BB upstream Plannotator bridge", () => {
     });
   });
 
+  it("refuses a second review with a distinct status instead of cancelled", async () => {
+    const binary = await fakePlannotatorBinary({ approveAfterMs: null });
+    const host = createFakePluginHost({
+      pluginId: "plannotator",
+      settings: { binaryPath: binary },
+    });
+    hosts.push(host);
+    await plugin(host.bb);
+    wireInteractionSdk(host);
+
+    const first = JSON.parse(String(await host.harness.behavior.callAgentTool(
+      "plannotator_review_plan",
+      { planMarkdown: "# Plan\n\n- First" },
+      { threadId: "thread-1", projectId: "project-1" },
+    ))) as { status: string; reviewId: string };
+    expect(first.status).toBe("pending");
+
+    const refusal = (await host.harness.behavior.callAgentTool(
+      "plannotator_review_plan",
+      { planMarkdown: "# Plan\n\n- Second" },
+      { threadId: "thread-1", projectId: "project-1" },
+    )) as { isError: boolean; content: Array<{ text: string }> };
+    expect(refusal.isError).toBe(true);
+    const second = JSON.parse(refusal.content[0]!.text) as Record<string, unknown>;
+    expect(second).toMatchObject({
+      status: "already_active",
+      source: "plannotator",
+      reviewId: first.reviewId,
+    });
+    expect(second).not.toHaveProperty("decision");
+
+    // The pending review is still recoverable and cancellable from the panel.
+    const active = (await host.harness.callRpc("getActiveReview", {
+      threadId: "thread-1",
+    })) as { sessionId: string };
+    expect(active.sessionId).toBe(first.reviewId);
+    await expect(
+      host.harness.callRpc("cancelReview", {
+        threadId: "thread-1",
+        sessionId: active.sessionId,
+      }),
+    ).resolves.toEqual({ cancelled: true });
+    await sentReview(host);
+    await expect.poll(() => host.harness.realtimeSignals).toContainEqual({
+      channel: PLANNOTATOR_REALTIME_CHANNEL,
+      payload: {
+        kind: "review-closed",
+        threadId: "thread-1",
+        sessionId: first.reviewId,
+      },
+    });
+  });
+
+  it("frees the thread for a new review while a decision is still being delivered", async () => {
+    const binary = await fakePlannotatorBinary({ approveAfterMs: null });
+    const host = createFakePluginHost({
+      pluginId: "plannotator",
+      settings: { binaryPath: binary },
+    });
+    hosts.push(host);
+    await plugin(host.bb);
+    wireInteractionSdk(host);
+    let releaseSend!: () => void;
+    const sendBlocked = new Promise<void>((resolve) => {
+      releaseSend = resolve;
+    });
+    host.harness.sdk.stub("threads.send", async () => {
+      await sendBlocked;
+      return { ok: true, delivery: "queued" };
+    });
+
+    const first = JSON.parse(String(await host.harness.behavior.callAgentTool(
+      "plannotator_review_plan",
+      { planMarkdown: "# Plan\n\n- First" },
+      { threadId: "thread-1", projectId: "project-1" },
+    ))) as { reviewId: string };
+    await host.harness.callRpc("cancelReview", {
+      threadId: "thread-1",
+      sessionId: first.reviewId,
+    });
+    // The decision send is now in flight and has not returned.
+    await sentReview(host);
+
+    const second = JSON.parse(String(await host.harness.behavior.callAgentTool(
+      "plannotator_review_plan",
+      { planMarkdown: "# Plan\n\n- Second" },
+      { threadId: "thread-1", projectId: "project-1" },
+    ))) as { status: string; reviewId: string };
+    expect(second.status).toBe("pending");
+    expect(second.reviewId).not.toBe(first.reviewId);
+
+    releaseSend();
+    const active = (await host.harness.callRpc("getActiveReview", {
+      threadId: "thread-1",
+    })) as { sessionId: string };
+    expect(active.sessionId).toBe(second.reviewId);
+    await host.harness.callRpc("cancelReview", {
+      threadId: "thread-1",
+      sessionId: second.reviewId,
+    });
+  });
+
   it("rejects cancellation from another thread or session", async () => {
     const binary = await fakePlannotatorBinary();
     const host = createFakePluginHost({