CL-6463: Fix connect-github card never flipping to connected on its own submit

packages/chat-ui/src/blocks/connect-github-block-container.tsx

@@ -1,11 +1,20 @@
 // Wires the presentational `ConnectGithubBlockView` to a live
 // `ConnectGithubActions` port (CL-6345) — mirroring `PollBlockView`'s
 // own container shape: an initial `getConnectState` read on mount, plus
-// a live `subscribeConnectState` fold for every update after, never a
-// second fetch once mounted. With no port at all, the card renders the
-// same fixed-disabled disconnected framing every other block's "no
-// port, no feature" fallback uses.
-import { useEffect, useState } from "react";
+// a live `subscribeConnectState` fold for every update after. With no
+// port at all, the card renders the same fixed-disabled disconnected
+// framing every other block's "no port, no feature" fallback uses.
+//
+// CL-6463: a card's own successful PAT submit is the one change this
+// container never waits on a fold for. `subscribeConnectState` folds
+// whatever a host chooses to publish, and the room's `chat.settings`
+// event (the only thing `connect-github-stream.ts` can fold) is written
+// by the later, unrelated repo-review PATCH — never by the credential
+// save itself. So `submitAccessToken` gets its own explicit
+// `getConnectState` refetch here, run once as the direct consequence of
+// that one submit — not a poll, and not a second source of truth
+// alongside the fold; the fold keeps handling every other update.
+import { useCallback, useEffect, useRef, useState } from "react";
 import type { ConnectGithubBlockData } from "@corbits/chat/blocks";
 
 import type {
@@ -24,36 +33,53 @@ export function ConnectGithubBlockContainer({
 }) {
   const [query, setQuery] = useState<ConnectGithubQuery>({ kind: "loading" });
   const [selectedRepoIds, setSelectedRepoIds] = useState<readonly string[]>([]);
+  const mountedRef = useRef(true);
 
-  useEffect(() => {
-    if (actions === undefined) return;
-    let cancelled = false;
+  const applyQuery = useCallback((result: ConnectGithubQuery) => {
+    if (!mountedRef.current) return;
+    setQuery(result);
+    if (result.kind === "connected") setSelectedRepoIds(result.selectedRepoIds);
+  }, []);
 
-    function applyQuery(result: ConnectGithubQuery) {
-      if (cancelled) return;
-      setQuery(result);
-      if (result.kind === "connected")
-        setSelectedRepoIds(result.selectedRepoIds);
-    }
+  useEffect(() => {
+    mountedRef.current = true;
+    return () => {
+      mountedRef.current = false;
+    };
+  }, []);
 
+  useEffect(() => {
+    if (actions === undefined) return;
     actions.getConnectState(messageId).then(applyQuery);
     const unsubscribe = actions.subscribeConnectState(messageId, applyQuery);
-    return () => {
-      cancelled = true;
-      unsubscribe();
-    };
-  }, [actions, messageId]);
+    return unsubscribe;
+  }, [actions, messageId, applyQuery]);
+
+  // The one refetch this container ever runs outside its mount effect:
+  // a submit this card itself just made succeeded, so re-reading the
+  // card's own state is a direct consequence of that submit — never a
+  // poll, and it runs whether or not the host's `subscribeConnectState`
+  // happens to fan the change out on its own.
+  const submitAccessTokenAndRefresh = useCallback(
+    async (token: string) => {
+      if (actions === undefined) {
+        return { ok: false as const, message: "Not available." };
+      }
+      const result = await actions.submitAccessToken(token);
+      if (result.ok) {
+        applyQuery(await actions.getConnectState(messageId));
+      }
+      return result;
+    },
+    [actions, messageId, applyQuery],
+  );
 
   if (actions === undefined || query.kind !== "connected") {
     return (
       <ConnectGithubBlockView
         kind="disconnected"
         onConnect={() => actions?.requestConnect()}
-        onSubmitAccessToken={(token) =>
-          actions !== undefined
-            ? actions.submitAccessToken(token)
-            : Promise.resolve({ ok: false, message: "Not available." })
-        }
+        onSubmitAccessToken={submitAccessTokenAndRefresh}
       />
     );
   }

packages/chat-ui/test/connect-github-block-container.test.tsx

@@ -0,0 +1,164 @@
+// CL-6463: a successful PAT submit must flip the connect-github card to
+// connected on its own — never leaning on a host that happens to fan the
+// change out through `subscribeConnectState` (real hosts vary, and
+// `chat.settings` never carries the credential-save path at all; see
+// `connect-github-stream.ts`'s own header). These fakes deliberately never
+// call the subscriber from `submitAccessToken`, so a pass here proves the
+// container drove its own state from the submit's own result — not from a
+// side channel a differently-wired host might forget.
+import { afterEach, describe, expect, test } from "bun:test";
+import { act } from "react";
+import { createRoot } from "react-dom/client";
+import type { Root } from "react-dom/client";
+
+import type { ConnectGithubBlockData } from "@corbits/chat/blocks";
+
+import type {
+  ConnectGithubActions,
+  ConnectGithubQuery,
+  ConnectGithubRepo,
+} from "../src/blocks/connect-github-actions";
+import { ConnectGithubBlockContainer } from "../src/blocks/connect-github-block-container";
+
+const DATA: ConnectGithubBlockData = {
+  requiredForTemplate: "github",
+  state: "disconnected",
+};
+
+const REPOS: readonly ConnectGithubRepo[] = [
+  { id: "1", name: "acme/widgets", openPullRequestCount: 2 },
+];
+
+let container: HTMLDivElement | null = null;
+let root: Root | null = null;
+
+afterEach(() => {
+  if (root !== null) act(() => root?.unmount());
+  container?.remove();
+  container = null;
+  root = null;
+});
+
+async function mount(actions: ConnectGithubActions) {
+  container = document.createElement("div");
+  document.body.appendChild(container);
+  root = createRoot(container);
+  await act(async () => {
+    root?.render(
+      <ConnectGithubBlockContainer
+        data={DATA}
+        messageId="m1"
+        actions={actions}
+      />,
+    );
+  });
+  return container;
+}
+
+function typeInto(element: HTMLInputElement, text: string) {
+  const setter = Object.getOwnPropertyDescriptor(
+    globalThis.HTMLInputElement.prototype,
+    "value",
+  )?.set;
+  setter?.call(element, text);
+  element.dispatchEvent(new Event("input", { bubbles: true }));
+}
+
+/** A host that never notifies `subscribeConnectState` from
+ * `submitAccessToken` — the exact shape of the real gap this ticket fixes:
+ * the credential save succeeds, but nothing about it ever reaches the
+ * fold. `getConnectState` is the only thing that reports the new
+ * connected fact, mirroring the real `/github/state` route reading the
+ * just-written credential. */
+function buildNeverNotifiesHarness(options?: {
+  readonly submitResult?:
+    { readonly ok: true } | { readonly ok: false; readonly message: string };
+}) {
+  let connected = false;
+  return {
+    actions: {
+      getConnectState: () =>
+        Promise.resolve<ConnectGithubQuery>(
+          connected
+            ? {
+                kind: "connected",
+                orgName: "octocat",
+                repos: REPOS,
+                selectedRepoIds: [],
+              }
+            : { kind: "disconnected" },
+        ),
+      subscribeConnectState: () => () => {},
+      requestConnect: () => {},
+      submitAccessToken: async (_token: string) => {
+        const result = options?.submitResult ?? { ok: true as const };
+        if (result.ok) connected = true;
+        return result;
+      },
+      startReviewing: async () => ({ startedTriggerCount: 0 }),
+      skip: async () => {},
+    } satisfies ConnectGithubActions,
+  };
+}
+
+async function openFieldAndSubmit(el: HTMLElement, token: string) {
+  const connectButton = [...el.querySelectorAll("button")].find(
+    (button) => button.textContent === "Connect GitHub",
+  ) as HTMLButtonElement;
+  await act(async () => {
+    connectButton.click();
+  });
+  const tokenField = el.querySelector(
+    "#connect-github-token",
+  ) as HTMLInputElement;
+  await act(async () => {
+    typeInto(tokenField, token);
+  });
+  const submitButton = [...el.querySelectorAll("button")].find(
+    (button) => button.textContent === "Connect",
+  ) as HTMLButtonElement;
+  await act(async () => {
+    submitButton.click();
+  });
+  await act(async () => {
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+}
+
+describe("ConnectGithubBlockContainer post-submit refresh (CL-6463)", () => {
+  test("a successful PAT submit flips the card to connected on its own, even when the host never fans the change out through subscribeConnectState", async () => {
+    const harness = buildNeverNotifiesHarness();
+    const el = await mount(harness.actions);
+
+    expect(el.textContent).toContain("Connect GitHub");
+    await openFieldAndSubmit(el, "ghp_test123");
+
+    expect(el.textContent).toContain("Connected to GitHub as octocat");
+    expect(el.querySelectorAll(".chat-block-connect-repo-row")).toHaveLength(
+      REPOS.length,
+    );
+  });
+
+  test("a rejected token shows what went wrong and leaves a working submit button, never a dead card", async () => {
+    const harness = buildNeverNotifiesHarness({
+      submitResult: { ok: false, message: "That token looks expired." },
+    });
+    const el = await mount(harness.actions);
+
+    await openFieldAndSubmit(el, "ghp_bad");
+
+    expect(el.textContent).toContain("That token looks expired.");
+    expect(el.textContent).not.toContain("Connected to GitHub as");
+
+    const submitButton = [...el.querySelectorAll("button")].find(
+      (button) => button.textContent === "Connect",
+    ) as HTMLButtonElement;
+    expect(submitButton.disabled).toBe(false);
+
+    const tokenField = el.querySelector(
+      "#connect-github-token",
+    ) as HTMLInputElement;
+    expect(tokenField.disabled).toBe(false);
+  });
+});