Say which Slack link conflict happened, and stop caching the claim

Four things from review, and a dependency narrowing that was asked for.

The 409 from linking a Slack account said one sentence for two
opposite conflicts. The store already knew which key the insert lost
to: the Slack identity belonging to another OpenBot account, or the
caller's own account already linked to a different Slack user in the
same workspace. It threw the same string for both, so somebody
re-linking under a new Slack id was told their identity belonged to
another account -- a false claim about their own account, with no
action attached. The conflict now travels as a code, and the
confirmation page says the true one.

GET and POST on the link route did not send `Cache-Control: no-store`,
which every sibling route in the file does. The request URL carries the
token and the response is the identity claim decoded from it, so an
intermediary keying on that URL would hold a decoded claim beside the
credential that produced it.

The read-only Slack transcript had no rejection handler: a failed
`/messages` left the view on its restoring skeleton for as long as
somebody left it open, and rejected with nobody listening. The
`unreadable` counter it should have fed was unreachable -- the read is
all-or-nothing -- so it is a fact about the read now, and says the
conversation could not be read.

A Slack turn established its private execution context twice, and
protecting copied every time, so a turn had two executions: the run
wrote `agentId` to one and a computer tool reading the other would have
found none and refused. It only worked because someone else's agent
loop happens to invoke tool handlers after the run returns. Protecting
an already-protected execution now returns it unchanged, which holds
the invariant here rather than in a dependency, and there is a test for
it. The stable-threadId property the append-only binding rests on is
named at the binding site, because it is a property of managed delivery
rather than of Channels.

`@copilotkit/channels` was the umbrella package, so the Discord,
Telegram, Teams and WhatsApp adapters came with it to be used by
nothing. Narrowed to `channels-core` and `channels-ui`, with
`channels-slack` a devDependency for the one test that asserts rendered
Block Kit.

`waitForAssistance` was replaced by `waitForExactAssistance` and called
by nothing while keeping ninety lines of tests, and `pinnedFirst` was
superseded by `conversationRoster`. Both removed, and what their tests
uniquely covered -- the bounded wait expiring after the link is posted,
a turn cancelled mid-wait, and a title never moving a row -- is now
asserted on the paths that ship.

Author: jerelvelarde

app/src/components/app-sidebar/app-sidebar.tsx

@@ -106,20 +106,6 @@ function UserAvatar() {
  */
 const MAX_ANIMATED_ROWS = 60;
 
-/**
- * Pinned channels first, everything else after, newest activity first within each group.
- *
- * The mirror of a server rule, not the rule itself: the roster query orders pinned-first and its
- * cursor carries the pin, so a pinned channel arrives on page one however long ago it was last
- * spoken in. Sorting here as well is for what happens between refetches — the socket patches a pin
- * onto a loaded row without moving it, and re-sorts a page by recency alone — which is the same
- * reason `byRecency` in use-channel-events.ts mirrors the recency rule. A stable partition, so the
- * recency order inside each group is whatever arrived.
- */
-export function pinnedFirst(channels: ChannelSummary[]): ChannelSummary[] {
-  return [...channels].sort((a, b) => Number(b.pinned) - Number(a.pinned));
-}
-
 /**
  * Whether a Bot has said something this member has not had on screen yet.
  *

app/src/components/channels/external-thread-chat.tsx

@@ -13,16 +13,33 @@ export function ExternalThreadChat({
 }) {
   const [messages, setMessages] = useState<readonly Message[]>([]);
   const [restoring, setRestoring] = useState(true);
-  const [unreadable, setUnreadable] = useState(0);
+  const [unreadable, setUnreadable] = useState(false);
 
   useEffect(() => {
     let current = true;
-    void readExternalThreadMessages(target.threadId).then((stored) => {
-      if (!current) return;
-      setMessages(stored);
-      setUnreadable(0);
-      setRestoring(false);
-    });
+    setRestoring(true);
+    setUnreadable(false);
+    void readExternalThreadMessages(target.threadId)
+      .then((stored) => {
+        if (!current) return;
+        setMessages(stored);
+        setRestoring(false);
+      })
+      /*
+       * A read that fails has to stop the restoring state and say so.
+       *
+       * Without this the promise rejects with nobody listening and the view sits on its skeleton
+       * for as long as the person leaves it open — which reads as a conversation still loading
+       * rather than one that could not be read, and is the state a failed `/messages` used to leave
+       * behind. The transcript is all-or-nothing: the endpoint either answers with the turns or it
+       * does not, so this is a fact about the read and not a count of messages.
+       */
+      .catch(() => {
+        if (!current) return;
+        setMessages([]);
+        setUnreadable(true);
+        setRestoring(false);
+      });
     return () => {
       current = false;
     };
@@ -38,11 +55,10 @@ export function ExternalThreadChat({
             This is the canonical Slack conversation with {target.agentName}. It
             is read-only here for this demo; continue the conversation in Slack.
           </p>
-          {unreadable > 0 ? (
+          {unreadable ? (
             <p>
-              {unreadable === 1
-                ? "One earlier message could not be read."
-                : `${unreadable} earlier messages could not be read.`}
+              This conversation could not be read. It is still in Slack; reload
+              to try again.
             </p>
           ) : null}
         </div>

app/src/lib/channels/use-channel-events.ts

@@ -152,7 +152,7 @@ export function applyChannelEvent(
    *
    * The spread below would carry this event's null message onto the row and wipe the preview the
    * roster renders. No re-sort either: a pin is not activity, and pinned rows are lifted at render
-   * time by `pinnedFirst`, not by the order they sit in here.
+   * time by `conversationRoster`, not by the order they sit in here.
    */
   if (activity.pinned !== undefined) {
     if (previous.pinned === activity.pinned) return data;

app/src/routes/_authed/link/slack.tsx

@@ -41,7 +41,39 @@ export function slackLinkToken(search: Record<string, unknown>): string | null {
   return token === "" ? null : token;
 }
 
-export function slackLinkResult(status: number) {
+/**
+ * Which key the link lost to, as the server reports it.
+ *
+ * Only one of the two is about somebody else's account, and saying the wrong one is worse than
+ * saying nothing: a person re-linking under a new Slack id in the same workspace was told their
+ * identity belonged to another OpenBot account, which is a false claim about their own and one
+ * they cannot act on. An unrecognised or absent code falls back to the safe half of the pair.
+ */
+export type SlackLinkConflict =
+  | "provider_identity_linked"
+  | "openbot_user_linked";
+
+export function slackLinkConflict(value: unknown): SlackLinkConflict {
+  const conflict =
+    value && typeof value === "object" && !Array.isArray(value)
+      ? (value as { conflict?: unknown }).conflict
+      : undefined;
+  return conflict === "openbot_user_linked"
+    ? "openbot_user_linked"
+    : "provider_identity_linked";
+}
+
+const SLACK_LINK_CONFLICT_MESSAGES = {
+  provider_identity_linked:
+    "That Slack identity is already linked to another OpenBot account.",
+  openbot_user_linked:
+    "Your OpenBot account is already linked to a different Slack user in this workspace. Unlink it before linking this one.",
+} as const satisfies Record<SlackLinkConflict, string>;
+
+export function slackLinkResult(
+  status: number,
+  conflict: SlackLinkConflict = "provider_identity_linked",
+) {
   if (status === 200)
     return {
       kind: "linked",
@@ -50,8 +82,7 @@ export function slackLinkResult(status: number) {
   if (status === 409)
     return {
       kind: "conflict",
-      message:
-        "That Slack identity is already linked to another OpenBot account.",
+      message: SLACK_LINK_CONFLICT_MESSAGES[conflict],
     } as const;
   return {
     kind: "invalid",
@@ -68,10 +99,13 @@ export function slackLinkFailure(): SlackLinkFailure {
 }
 
 /** Only documented token refusals are terminal-invalid; unknown responses stay retryable. */
-export function slackLinkResponseOutcome(status?: number): SlackLinkResponse {
+export function slackLinkResponseOutcome(
+  status?: number,
+  conflict?: SlackLinkConflict,
+): SlackLinkResponse {
   if (status === 401) return { kind: "reauth" };
   if (status === 200 || status === 400 || status === 409)
-    return slackLinkResult(status);
+    return slackLinkResult(status, conflict);
   return slackLinkFailure();
 }
 
@@ -126,7 +160,12 @@ async function completeSlackLink(
     body: { token },
     signal,
   });
-  return slackLinkResponseOutcome(response.status);
+  // Read only on the conflict: it is the one status whose body decides what the page may say.
+  const conflict =
+    response.status === 409
+      ? slackLinkConflict(await response.json().catch(() => null))
+      : undefined;
+  return slackLinkResponseOutcome(response.status, conflict);
 }
 
 function SlackLinkPage() {

app/tests/channel-order.test.ts

@@ -22,6 +22,7 @@ function channel(
     agentIds: [`agent-${id}`],
     threadId: `thread-${id}`,
     active: true,
+    summary: null,
     lastMessage: null,
     lastMessageAt: null,
     lastMessageAgentId: null,
@@ -88,6 +89,29 @@ describe("sidebar conversation roster", () => {
     ]);
   });
 
+  /**
+   * Naming a conversation is not activity in it.
+   *
+   * Whatever order the roster was in, it is the same order once titles arrive, or rows would appear
+   * to jump for no reason anybody looking at them could account for. Held by construction — the
+   * sort reads activity and the row key, never the summary — and asserted because that is the kind
+   * of thing a later sort change breaks quietly.
+   */
+  test("a title changes nothing about where a row sits", () => {
+    const ids = (rows: ReturnType<typeof conversationRoster>) =>
+      rows.map((row) => rosterKey(row));
+    const untitled = [channel("a"), channel("b", { pinned: true })];
+    const titled = [
+      channel("a", { summary: "Expense categories" }),
+      channel("b", { pinned: true, summary: "Quarterly revenue" }),
+    ];
+    const threads = [slack("s1")];
+
+    expect(ids(conversationRoster(titled, threads))).toEqual(
+      ids(conversationRoster(untitled, threads)),
+    );
+  });
+
   test("matches visible names and last-message text across native and Slack rows", () => {
     const rows = conversationRoster(
       [

app/tests/sidebar-roster.test.ts

@@ -1,6 +1,7 @@
 import { expect, test } from "bun:test";
 import {
   slackLinkClaim,
+  slackLinkConflict,
   slackLinkFailure,
   slackLinkResponseOutcome,
   slackLinkResult,
@@ -17,6 +18,43 @@ test("requires a token and maps completion responses", () => {
   expect(slackLinkResult(409).kind).toBe("conflict");
 });
 
+/**
+ * The two 409s say opposite things, and only one of them is about somebody else.
+ *
+ * A person re-linking under a new Slack id in the same workspace used to be told their identity
+ * belonged to another OpenBot account: a false claim about their own account, and one with no
+ * action attached. The server distinguishes the two keys, so this page has to as well — and an
+ * unrecognised or absent code has to fall back to the claim that is safe to make.
+ */
+test("says which of the two conflicts happened, and falls back to the safe one", () => {
+  expect(slackLinkResult(409, "provider_identity_linked").message).toBe(
+    "That Slack identity is already linked to another OpenBot account.",
+  );
+  expect(slackLinkResult(409, "openbot_user_linked").message).toBe(
+    "Your OpenBot account is already linked to a different Slack user in this workspace. Unlink it before linking this one.",
+  );
+  expect(slackLinkResult(409).message).toBe(
+    slackLinkResult(409, "provider_identity_linked").message,
+  );
+
+  expect(slackLinkConflict({ conflict: "openbot_user_linked" })).toBe(
+    "openbot_user_linked",
+  );
+  expect(slackLinkConflict({ conflict: "provider_identity_linked" })).toBe(
+    "provider_identity_linked",
+  );
+  for (const body of [
+    null,
+    undefined,
+    {},
+    { conflict: 42 },
+    { conflict: "something_else" },
+    ["openbot_user_linked"],
+  ]) {
+    expect(slackLinkConflict(body)).toBe("provider_identity_linked");
+  }
+});
+
 test("rejects non-string, empty, and repeated token search inputs", () => {
   expect(slackLinkToken({ token: "" })).toBeNull();
   expect(slackLinkToken({ token: "   " })).toBeNull();
@@ -46,6 +84,9 @@ test("classifies documented token, authentication, and transient responses", ()
     expect(slackLinkResponseOutcome(status).kind).toBe("invalid");
   }
   expect(slackLinkResponseOutcome(409).kind).toBe("conflict");
+  expect(
+    slackLinkResponseOutcome(409, "openbot_user_linked").message,
+  ).toContain("a different Slack user in this workspace");
   expect(slackLinkResponseOutcome(401).kind).toBe("reauth");
 
   for (const status of [408, 418, 425, 429, 500, 502, 503]) {