Fixes To Buncha Things

.claude/commands/finalize.md

@@ -410,7 +410,14 @@ cd apps/ade-cli  && npm test
 
 The desktop workspace has 3 projects (`unit-main`, `unit-renderer`, `unit-shared`); sharding distributes across all three automatically.
 
-If a shard fails, re-run **only that shard** (or only the failing test file). Never re-run all 9 to verify a one-file fix.
+If a shard fails, **re-run ONLY the failing test file(s)** — not the whole shard, and never the full sharded run. Sharding is a wall-clock optimization for the *initial* gate; once you've isolated which file failed, the cheapest signal is `npx vitest run <path/to/file.test.tsx>`. A 90-second shard rerun to verify a 5-second one-file fix is wasted time and burns the prompt cache.
+
+Anti-pattern (do NOT do this):
+- "Shard 1 had failures, let me rerun shard 1" → wrong; rerun just the failing file.
+- "I fixed the failing file, let me rerun the whole shard to be sure" → wrong; rerun just that file. The other tests in the shard already passed and didn't change.
+- "Let me rerun all 8 shards after a one-file fix" → wrong; this is the worst option.
+
+Only run the full sharded suite once at the start of Phase 3e. After that, narrow scope to failing files until they pass, then move on.
 
 Workspace-project subsets exist for debugging only; they are NOT a substitute for the sharded run:
 
@@ -463,7 +470,7 @@ git diff --name-only | sort > /tmp/finalize-session-files.txt
 comm -13 /tmp/finalize-branch-files.txt /tmp/finalize-session-files.txt
 ```
 
-Revert the simplifier's edits to the offending files and re-run only the failed shard. Do NOT rewrite the test suite in Phase 3 — tests that drift because the feature branch refactored UI are a separate follow-up.
+Revert the simplifier's edits to the offending files and re-run **only the failing test file** (not the full shard). Do NOT rewrite the test suite in Phase 3 unless the user explicitly asks — tests that drift because the feature branch refactored UI are a separate follow-up by default.
 
 ### 3i. Cleanup lingering processes
 

apps/desktop/src/main/services/github/githubService.test.ts

@@ -39,8 +39,10 @@ vi.mock("node:fs", async (importOriginal) => {
   };
 });
 
+const runGitMock = vi.hoisted(() => vi.fn());
+
 vi.mock("../git/git", () => ({
-  runGit: vi.fn(),
+  runGit: runGitMock,
 }));
 
 // Replace global fetch
@@ -474,3 +476,98 @@ describe("githubService issue-domain helpers", () => {
     expect(url).toContain("/repos/scary%20owner/name%20with%20space/issues");
   });
 });
+
+// ---------------------------------------------------------------------------
+// getStatus — connection probing (regression for false-CONNECTED bug)
+// ---------------------------------------------------------------------------
+
+describe("githubService.getStatus", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    delete process.env.GITHUB_TOKEN;
+    delete process.env.ADE_GITHUB_TOKEN;
+  });
+
+  // Mocks `git remote get-url origin` so detectRepo returns acme/ade.
+  function stubOriginRemote() {
+    runGitMock.mockResolvedValue({
+      exitCode: 0,
+      stdout: "git@github.com:acme/ade.git\n",
+      stderr: "",
+    });
+  }
+
+  it("classic token with required scopes is connected (no repo probe needed)", async () => {
+    stubOriginRemote();
+    process.env.GITHUB_TOKEN = "ghp_classic";
+    mockFetch.mockResolvedValueOnce(
+      jsonResponse(200, { login: "alice" }, { "x-oauth-scopes": "repo, workflow" }),
+    );
+    // Repo probe still runs and we still mock it; the response just decorates the status.
+    mockFetch.mockResolvedValueOnce(jsonResponse(200, { full_name: "acme/ade" }));
+
+    const status = await makeService().getStatus();
+
+    expect(status.tokenStored).toBe(true);
+    expect(status.tokenType).toBe("classic");
+    expect(status.userLogin).toBe("alice");
+    expect(status.scopes).toEqual(["repo", "workflow"]);
+    expect(status.connected).toBe(true);
+  });
+
+  it("fine-grained token that authenticates but cannot read the repo is NOT connected", async () => {
+    // This is the original bug: /user works, so userLogin is set, but the
+    // active repo isn't included in the token's selected repositories. Every
+    // PR-tab call would 404. Status must reflect that with connected=false.
+    stubOriginRemote();
+    process.env.GITHUB_TOKEN = "github_pat_finegrained";
+    mockFetch.mockResolvedValueOnce(jsonResponse(200, { login: "alice" }));
+    mockFetch.mockResolvedValueOnce(jsonResponse(404, { message: "Not Found" }));
+
+    const status = await makeService().getStatus();
+
+    expect(status.tokenType).toBe("fine-grained");
+    expect(status.userLogin).toBe("alice");
+    expect(status.repoAccessOk).toBe(false);
+    expect(status.repoAccessError).toContain("404");
+    expect(status.connected).toBe(false);
+  });
+
+  it("fine-grained token with successful repo probe is connected", async () => {
+    stubOriginRemote();
+    process.env.GITHUB_TOKEN = "github_pat_finegrained";
+    mockFetch.mockResolvedValueOnce(jsonResponse(200, { login: "alice" }));
+    mockFetch.mockResolvedValueOnce(jsonResponse(200, { full_name: "acme/ade" }));
+
+    const status = await makeService().getStatus();
+
+    expect(status.tokenType).toBe("fine-grained");
+    expect(status.repoAccessOk).toBe(true);
+    expect(status.connected).toBe(true);
+  });
+
+  it("classic token without required scopes is NOT connected", async () => {
+    stubOriginRemote();
+    process.env.GITHUB_TOKEN = "ghp_classic";
+    // Token has only `read:user` (insufficient).
+    mockFetch.mockResolvedValueOnce(
+      jsonResponse(200, { login: "alice" }, { "x-oauth-scopes": "read:user" }),
+    );
+    mockFetch.mockResolvedValueOnce(jsonResponse(200, { full_name: "acme/ade" }));
+
+    const status = await makeService().getStatus();
+
+    expect(status.scopes).toEqual(["read:user"]);
+    expect(status.connected).toBe(false);
+  });
+
+  it("missing token returns not-connected status with all the new fields populated", async () => {
+    stubOriginRemote();
+    const status = await makeService().getStatus();
+
+    expect(status.tokenStored).toBe(false);
+    expect(status.connected).toBe(false);
+    expect(status.repoAccessOk).toBeNull();
+    expect(status.repoAccessError).toBeNull();
+  });
+});

apps/desktop/src/main/services/github/githubService.ts

@@ -5,7 +5,7 @@ import type { Logger } from "../logging/logger";
 import { runGit } from "../git/git";
 import type { GitHubRepoRef, GitHubStatus } from "../../../shared/types";
 import { resolveAdeLayout } from "../../../shared/adeLayout";
-import { parseGitHubScopeHeaders } from "../../../shared/githubScopes";
+import { getGitHubTokenAccessState, parseGitHubScopeHeaders } from "../../../shared/githubScopes";
 
 import { nowIso, asString } from "../shared/utils";
 
@@ -207,6 +207,36 @@ export function createGithubService({
     };
   };
 
+  // Verifies the active token actually has access to the active repo by hitting
+  // /repos/{owner}/{name} (cheapest endpoint that requires Metadata: Read on
+  // fine-grained tokens; classic tokens with `repo` scope also pass). This is
+  // the only reliable connectivity check for fine-grained tokens, which never
+  // return x-oauth-scopes and so cannot be introspected via headers.
+  const probeRepoAccess = async (
+    token: string,
+    repo: GitHubRepoRef,
+  ): Promise<{ ok: boolean; error: string | null }> => {
+    try {
+      const response = await fetch(
+        `https://api.github.com/repos/${encodeURIComponent(repo.owner)}/${encodeURIComponent(repo.name)}`,
+        {
+          method: "GET",
+          headers: {
+            accept: "application/vnd.github+json",
+            authorization: `Bearer ${token}`,
+            "user-agent": "ade-desktop",
+          },
+        },
+      );
+      if (response.ok) return { ok: true, error: null };
+      const payload = (await response.json().catch(() => ({}))) as Record<string, unknown>;
+      const message = asString(payload.message) || `HTTP ${response.status}`;
+      return { ok: false, error: `${response.status}: ${message}` };
+    } catch (error) {
+      return { ok: false, error: error instanceof Error ? error.message : String(error) };
+    }
+  };
+
   // ETag cache for conditional GET requests. Responses that return 304 Not Modified
   // don't count against GitHub's rate limit, so this dramatically reduces API usage.
   const etagCache = new Map<string, { etag: string; data: unknown; linkHeader: string | null }>();
@@ -337,7 +367,37 @@ export function createGithubService({
   let cachedStatus: GitHubStatus | null = null;
   let cachedAt = 0;
 
-  const getStatus = async (): Promise<GitHubStatus> => {
+  // Decides whether the saved token is actually usable for the project. Classic
+  // tokens require both required scopes; fine-grained tokens require a successful
+  // repo probe (since their permissions are not introspectable via headers).
+  const computeConnected = (args: {
+    tokenStored: boolean;
+    userLogin: string | null;
+    tokenType: GitHubStatus["tokenType"];
+    scopes: string[];
+    repo: GitHubRepoRef | null;
+    repoAccessOk: boolean | null;
+  }): boolean => {
+    if (!args.tokenStored || !args.userLogin) return false;
+    if (args.tokenType === "fine-grained") {
+      // No repo to probe (e.g. project without a GitHub remote): a fine-grained
+      // token that authenticates as a user is the best signal we have.
+      if (!args.repo) return true;
+      return args.repoAccessOk === true;
+    }
+    if (args.tokenType === "classic") {
+      const access = getGitHubTokenAccessState(args.scopes);
+      return access.hasRequiredAccess;
+    }
+    // Unknown prefix — fall back to "user lookup worked" (best-effort).
+    return Boolean(args.userLogin);
+  };
+
+  const getStatus = async (opts: { forceRefresh?: boolean } = {}): Promise<GitHubStatus> => {
+    if (opts.forceRefresh) {
+      cachedStatus = null;
+      cachedAt = 0;
+    }
     const token = readStoredToken();
     const repo = await detectRepo().catch(() => null);
     if (!token) {
@@ -349,20 +409,59 @@ export function createGithubService({
         repo,
         userLogin: null,
         scopes: [],
-        checkedAt: null
+        checkedAt: null,
+        repoAccessOk: null,
+        repoAccessError: null,
+        connected: false,
       };
       cachedAt = Date.now();
       return cachedStatus;
     }
 
     const now = Date.now();
     if (cachedStatus && now - cachedAt < 30_000 && cachedStatus.tokenStored) {
-      // Still re-detect repo, it is cheap and reflects changed remotes.
-      return { ...cachedStatus, repo };
+      // Still re-detect repo and re-evaluate `connected` so a remote change is reflected.
+      const repoChanged =
+        (cachedStatus.repo?.owner ?? null) !== (repo?.owner ?? null) ||
+        (cachedStatus.repo?.name ?? null) !== (repo?.name ?? null);
+      // If the repo just changed we can't trust the cached probe result.
+      const repoAccessOk = repoChanged ? null : cachedStatus.repoAccessOk;
+      const repoAccessError = repoChanged ? null : cachedStatus.repoAccessError;
+      const connected = computeConnected({
+        tokenStored: true,
+        userLogin: cachedStatus.userLogin,
+        tokenType: cachedStatus.tokenType,
+        scopes: cachedStatus.scopes,
+        repo,
+        repoAccessOk,
+      });
+      return { ...cachedStatus, repo, repoAccessOk, repoAccessError, connected };
     }
 
     try {
       const validated = await validateToken(token);
+      let repoAccessOk: boolean | null = null;
+      let repoAccessError: string | null = null;
+      if (repo) {
+        const probe = await probeRepoAccess(token, repo);
+        repoAccessOk = probe.ok;
+        repoAccessError = probe.error;
+        if (!probe.ok) {
+          logger.warn("github.repo_probe_failed", {
+            repo: `${repo.owner}/${repo.name}`,
+            tokenType: validated.tokenType,
+            error: probe.error,
+          });
+        }
+      }
+      const connected = computeConnected({
+        tokenStored: true,
+        userLogin: validated.userLogin,
+        tokenType: validated.tokenType,
+        scopes: validated.scopes,
+        repo,
+        repoAccessOk,
+      });
       cachedStatus = {
         tokenStored: true,
         tokenDecryptionFailed: false,
@@ -371,7 +470,10 @@ export function createGithubService({
         repo,
         userLogin: validated.userLogin,
         scopes: validated.scopes,
-        checkedAt: nowIso()
+        checkedAt: nowIso(),
+        repoAccessOk,
+        repoAccessError,
+        connected,
       };
       cachedAt = now;
       return cachedStatus;
@@ -385,7 +487,10 @@ export function createGithubService({
         repo,
         userLogin: null,
         scopes: [],
-        checkedAt: nowIso()
+        checkedAt: nowIso(),
+        repoAccessOk: null,
+        repoAccessError: null,
+        connected: false,
       };
       cachedAt = now;
       return cachedStatus;

apps/desktop/src/main/services/ipc/registerIpc.ts

@@ -5072,21 +5072,36 @@ export function registerIpc({
 
   ipcMain.handle(IPC.conflictsSuggestResolverTarget, async (_event, arg) => getCtx().conflictService.suggestResolverTarget(arg));
 
-  ipcMain.handle(IPC.githubGetStatus, async (): Promise<GitHubStatus> => {
+  const broadcastGithubStatus = (status: GitHubStatus): void => {
+    for (const win of BrowserWindow.getAllWindows()) {
+      if (win.isDestroyed()) continue;
+      try {
+        win.webContents.send(IPC.githubStatusChanged, status);
+      } catch {
+        // ignore broadcast failures
+      }
+    }
+  };
+
+  ipcMain.handle(IPC.githubGetStatus, async (_event, arg?: { forceRefresh?: boolean }): Promise<GitHubStatus> => {
     const ctx = getCtx();
-    return await ctx.githubService.getStatus();
+    return await ctx.githubService.getStatus({ forceRefresh: Boolean(arg?.forceRefresh) });
   });
 
   ipcMain.handle(IPC.githubSetToken, async (_event, arg: { token: string }): Promise<GitHubStatus> => {
     const ctx = getCtx();
     ctx.githubService.setToken(arg.token);
-    return await ctx.githubService.getStatus();
+    const status = await ctx.githubService.getStatus();
+    broadcastGithubStatus(status);
+    return status;
   });
 
   ipcMain.handle(IPC.githubClearToken, async (): Promise<GitHubStatus> => {
     const ctx = getCtx();
     ctx.githubService.clearToken();
-    return await ctx.githubService.getStatus();
+    const status = await ctx.githubService.getStatus();
+    broadcastGithubStatus(status);
+    return status;
   });
 
   const resolveGithubRepoRef = async (