ship: iter 1 — address greptile/codex review on PR #248

- githubService.publishCurrentProject: clean up local origin remote when
  remote-add or push fails after createRepository succeeded, so retry
  isn't blocked by remote_already_exists guard (greptile P1 #3184587072)
- projectScaffoldService: throw coded errors with friendly messages
  (target_exists, invalid_github_url) so renderer can match by .code
  instead of raw .message (greptile P1 #3184587147)
- projectScaffoldService.listMyGitHubRepos: cache by sha256(token).slice(0,16)
  instead of token.slice(0,8) — fine-grained PATs all share the github_p
  prefix and would collide (codex P1 #3184597051)
- CloneProjectForm: tighten URL validation to GitHub-only shapes so UI
  surfaces a friendly hint before the IPC call (codex P2 #3184597055)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: arul28

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

@@ -760,7 +760,8 @@ describe("githubService.publishCurrentProject", () => {
       .mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "" })
       .mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" })
       .mockResolvedValueOnce({ exitCode: 0, stdout: "abc\n", stderr: "" })
-      .mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "Authentication failed" });
+      .mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "Authentication failed" })
+      .mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" }); // best-effort remote remove
 
     mockFetch.mockResolvedValueOnce(
       jsonResponse(201, {
@@ -775,4 +776,52 @@ describe("githubService.publishCurrentProject", () => {
       makeService().publishCurrentProject({ name: "proj", isPrivate: true }),
     ).rejects.toThrow(/authentication failed/i);
   });
+
+  it("removes the local origin when push fails so retry isn't blocked by remote_already_exists", async () => {
+    runGitMock
+      .mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "" }) // get-url origin
+      .mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" }) // remote add origin
+      .mockResolvedValueOnce({ exitCode: 0, stdout: "abc\n", stderr: "" }) // rev-parse HEAD
+      .mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "fatal: auth" }) // push fails
+      .mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" }); // remote remove origin
+
+    mockFetch.mockResolvedValueOnce(
+      jsonResponse(201, {
+        clone_url: "https://github.com/alice/proj.git",
+        ssh_url: "git@github.com:alice/proj.git",
+        html_url: "https://github.com/alice/proj",
+        default_branch: "main",
+      }),
+    );
+
+    await expect(
+      makeService().publishCurrentProject({ name: "proj", isPrivate: true }),
+    ).rejects.toThrow();
+
+    const gitCalls = runGitMock.mock.calls.map((c) => c[0]);
+    expect(gitCalls).toContainEqual(["remote", "remove", "origin"]);
+  });
+
+  it("removes the local origin when remote add fails after createRepository succeeded", async () => {
+    runGitMock
+      .mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "" }) // get-url origin
+      .mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "fatal: cannot lock" }) // remote add fails
+      .mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" }); // remote remove origin
+
+    mockFetch.mockResolvedValueOnce(
+      jsonResponse(201, {
+        clone_url: "https://github.com/alice/proj.git",
+        ssh_url: "git@github.com:alice/proj.git",
+        html_url: "https://github.com/alice/proj",
+        default_branch: "main",
+      }),
+    );
+
+    await expect(
+      makeService().publishCurrentProject({ name: "proj", isPrivate: true }),
+    ).rejects.toThrow(/Failed to add origin remote/);
+
+    const gitCalls = runGitMock.mock.calls.map((c) => c[0]);
+    expect(gitCalls).toContainEqual(["remote", "remove", "origin"]);
+  });
 });

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

@@ -692,11 +692,24 @@ export function createGithubService({
       isPrivate: args.isPrivate,
     });
 
+    // After createRepository succeeds, the GitHub repo exists. If we then
+    // fail to add the local origin or push to it, we must remove the local
+    // origin we may have added so that a retry isn't blocked by the
+    // remote_already_exists guard at the top of this function.
+    const cleanupLocalOrigin = async (): Promise<void> => {
+      try {
+        await runGit(["remote", "remove", "origin"], { cwd: projectRoot, timeoutMs: 8_000 });
+      } catch {
+        // best-effort
+      }
+    };
+
     const remoteAddRes = await runGit(["remote", "add", "origin", created.cloneUrl], {
       cwd: projectRoot,
       timeoutMs: 8_000,
     });
     if (remoteAddRes.exitCode !== 0) {
+      await cleanupLocalOrigin();
       throw new Error(`Failed to add origin remote: ${remoteAddRes.stderr.trim() || `exit ${remoteAddRes.exitCode}`}`);
     }
 
@@ -705,6 +718,7 @@ export function createGithubService({
     if (headRes.exitCode === 0) {
       const pushRes = await runGit(["push", "-u", "origin", "HEAD"], { cwd: projectRoot, timeoutMs: 5 * 60_000 });
       if (pushRes.exitCode !== 0) {
+        await cleanupLocalOrigin();
         throw new Error(`Failed to push to origin: ${pushRes.stderr.trim() || `exit ${pushRes.exitCode}`}`);
       }
       resultState = "pushed";

apps/desktop/src/main/services/projects/projectScaffoldService.test.ts

@@ -206,9 +206,14 @@ describe("createLocalProject", () => {
       githubService: makeGithubServiceStub(),
     });
 
-    await expect(
-      service.createLocalProject({ name: "collision", parentDir }),
-    ).rejects.toThrow("target_exists");
+    let caught: any;
+    try {
+      await service.createLocalProject({ name: "collision", parentDir });
+    } catch (err) {
+      caught = err;
+    }
+    expect(caught).toBeInstanceOf(Error);
+    expect(caught.code).toBe("target_exists");
   });
 
   it("allows reusing an empty existing directory as the target", async () => {
@@ -242,9 +247,14 @@ describe("cloneRepository", () => {
       }),
     });
 
-    await expect(
-      service.cloneRepository({ url: "https://gitlab.com/foo/bar.git", parentDir }),
-    ).rejects.toThrow("invalid_github_url");
+    let caught: any;
+    try {
+      await service.cloneRepository({ url: "https://gitlab.com/foo/bar.git", parentDir });
+    } catch (err) {
+      caught = err;
+    }
+    expect(caught).toBeInstanceOf(Error);
+    expect(caught.code).toBe("invalid_github_url");
   });
 
   it("rejects empty URLs", async () => {
@@ -254,9 +264,14 @@ describe("cloneRepository", () => {
       githubService: makeGithubServiceStub(),
     });
 
-    await expect(
-      service.cloneRepository({ url: "", parentDir }),
-    ).rejects.toThrow("invalid_github_url");
+    let caught: any;
+    try {
+      await service.cloneRepository({ url: "", parentDir });
+    } catch (err) {
+      caught = err;
+    }
+    expect(caught).toBeInstanceOf(Error);
+    expect(caught.code).toBe("invalid_github_url");
   });
 
   it("invokes git clone with the URL and target path, deriving name from the URL", async () => {
@@ -305,9 +320,14 @@ describe("cloneRepository", () => {
       githubService: makeGithubServiceStub(),
     });
 
-    await expect(
-      service.cloneRepository({ url: "https://github.com/octocat/Hello-World", parentDir }),
-    ).rejects.toThrow("target_exists");
+    let caught: any;
+    try {
+      await service.cloneRepository({ url: "https://github.com/octocat/Hello-World", parentDir });
+    } catch (err) {
+      caught = err;
+    }
+    expect(caught).toBeInstanceOf(Error);
+    expect(caught.code).toBe("target_exists");
   });
 
   it("surfaces git clone failures with stderr", async () => {
@@ -436,6 +456,40 @@ describe("listMyGitHubRepos", () => {
     expect(apiRequest).toHaveBeenCalledTimes(1);
   });
 
+  it("invalidates the cache when the token changes (fine-grained PATs share the same prefix)", async () => {
+    const apiRequest = vi
+      .fn()
+      .mockResolvedValueOnce({
+        data: [
+          { owner: { login: "alice" }, name: "first", full_name: "alice/first", private: false, pushed_at: null, default_branch: "main", html_url: "", clone_url: "", ssh_url: "" },
+        ],
+        response: null,
+      })
+      .mockResolvedValueOnce({
+        data: [
+          { owner: { login: "bob" }, name: "second", full_name: "bob/second", private: false, pushed_at: null, default_branch: "main", html_url: "", clone_url: "", ssh_url: "" },
+        ],
+        response: null,
+      });
+
+    const getTokenOrThrow = vi
+      .fn<[], string>()
+      .mockReturnValueOnce("github_pat_AAAAAAAA_one")
+      .mockReturnValueOnce("github_pat_BBBBBBBB_two");
+
+    const service = createProjectScaffoldService({
+      logger: makeLogger(),
+      githubService: makeGithubServiceStub({ apiRequest, getTokenOrThrow }),
+    });
+
+    const first = await service.listMyGitHubRepos({});
+    const second = await service.listMyGitHubRepos({});
+
+    expect(apiRequest).toHaveBeenCalledTimes(2);
+    expect(first.repos.map((r) => r.fullName)).toEqual(["alice/first"]);
+    expect(second.repos.map((r) => r.fullName)).toEqual(["bob/second"]);
+  });
+
   it("stops paginating after 5 pages even if every page is full", async () => {
     const apiRequest = vi.fn();
     const fullPage = Array.from({ length: 100 }, (_, i) => ({

apps/desktop/src/main/services/projects/projectScaffoldService.ts

@@ -1,3 +1,4 @@
+import crypto from "node:crypto";
 import fs from "node:fs";
 import os from "node:os";
 import path from "node:path";
@@ -69,14 +70,22 @@ function isGitIdentityError(stderr: string): boolean {
   );
 }
 
+function codedError(message: string, code: string): Error & { code: string } {
+  return Object.assign(new Error(message), { code });
+}
+
+function hashToken(token: string): string {
+  return crypto.createHash("sha256").update(token).digest("hex").slice(0, 16);
+}
+
 export function createProjectScaffoldService({
   logger,
   githubService,
 }: {
   logger: Logger;
   githubService: GithubService;
 }) {
-  let cachedRepos: { tokenPrefix: string; expiresAt: number; repos: MyGitHubRepoSummary[] } | null = null;
+  let cachedRepos: { tokenHash: string; expiresAt: number; repos: MyGitHubRepoSummary[] } | null = null;
 
   const createLocalProject = async (input: CreateProjectInput): Promise<CreateProjectResult> => {
     const name = validateProjectName(input.name);
@@ -88,7 +97,7 @@ export function createProjectScaffoldService({
     const rootPath = path.join(parentDir, name);
 
     if (fs.existsSync(rootPath) && isDirectoryNonEmpty(rootPath)) {
-      throw new Error("target_exists");
+      throw codedError("A folder already exists at that path.", "target_exists");
     }
 
     fs.mkdirSync(rootPath, { recursive: true });
@@ -154,11 +163,11 @@ export function createProjectScaffoldService({
   const cloneRepository = async (input: CloneProjectInput): Promise<CloneProjectResult> => {
     const url = (input.url ?? "").trim();
     if (!url) {
-      throw new Error("invalid_github_url");
+      throw codedError("Enter a GitHub repository URL.", "invalid_github_url");
     }
     const repoRef = githubService.parseGitHubRepoFromRemoteUrl(url);
     if (!repoRef) {
-      throw new Error("invalid_github_url");
+      throw codedError("Only GitHub repository URLs are supported.", "invalid_github_url");
     }
 
     const parentDir = (input.parentDir ?? "").trim();
@@ -171,7 +180,7 @@ export function createProjectScaffoldService({
     const rootPath = path.join(parentDir, name);
 
     if (fs.existsSync(rootPath) && isDirectoryNonEmpty(rootPath)) {
-      throw new Error("target_exists");
+      throw codedError("A folder already exists at that path.", "target_exists");
     }
 
     fs.mkdirSync(parentDir, { recursive: true });
@@ -198,10 +207,10 @@ export function createProjectScaffoldService({
       throw wrapped;
     }
 
-    const tokenPrefix = token.slice(0, 8);
+    const tokenHash = hashToken(token);
     const now = Date.now();
     let repos: MyGitHubRepoSummary[];
-    if (cachedRepos && cachedRepos.tokenPrefix === tokenPrefix && cachedRepos.expiresAt > now) {
+    if (cachedRepos && cachedRepos.tokenHash === tokenHash && cachedRepos.expiresAt > now) {
       repos = cachedRepos.repos;
     } else {
       const collected: MyGitHubRepoSummary[] = [];
@@ -241,7 +250,7 @@ export function createProjectScaffoldService({
         if (items.length < perPage) break;
       }
       repos = collected;
-      cachedRepos = { tokenPrefix, expiresAt: now + REPO_LIST_CACHE_TTL_MS, repos };
+      cachedRepos = { tokenHash, expiresAt: now + REPO_LIST_CACHE_TTL_MS, repos };
     }
 
     const search = (input.search ?? "").trim().toLowerCase();

apps/desktop/src/renderer/components/projects/CloneProjectForm.tsx

@@ -37,9 +37,33 @@ export type CloneProjectFormProps = {
 
 type Tab = "url" | "my-repos";
 
-const URL_PATTERN = /^(https?:\/\/[^\s]+|git@[^\s:]+:[^\s]+|ssh:\/\/[^\s]+)$/i;
 const SLUG_PATTERN = /([^/:]+?)(?:\.git)?$/;
 
+// Mirrors the backend `parseGitHubRepoFromRemoteUrl` accepted shapes so the
+// UI surfaces a friendly hint before the IPC call would reject with
+// `invalid_github_url`.
+function isGitHubRepoUrl(raw: string): boolean {
+  const trimmed = raw.trim();
+  if (!trimmed) return false;
+  const sshScp = trimmed.match(/^git@github\.com:(.+)$/i);
+  if (sshScp) {
+    const slug = sshScp[1]!.replace(/\.git$/i, "").trim();
+    const [owner, name] = slug.split("/");
+    return Boolean(owner && name);
+  }
+  if (/^(https?|ssh):\/\//i.test(trimmed)) {
+    try {
+      const url = new URL(trimmed);
+      if (!/github\.com$/i.test(url.hostname)) return false;
+      const parts = url.pathname.replace(/^\/+/, "").replace(/\.git$/i, "").split("/");
+      return Boolean(parts[0]?.trim() && parts[1]?.trim());
+    } catch {
+      return false;
+    }
+  }
+  return false;
+}
+
 const inputStyle: CSSProperties = {
   height: 36,
   padding: "0 12px",
@@ -209,7 +233,7 @@ function UrlTab({
 
   const trimmedUrl = url.trim();
   const trimmedName = name.trim();
-  const urlValid = trimmedUrl.length > 0 && URL_PATTERN.test(trimmedUrl);
+  const urlValid = isGitHubRepoUrl(trimmedUrl);
   const previewPath = useMemo(
     () => (parentDir && trimmedName ? joinPath(parentDir, trimmedName) : ""),
     [parentDir, trimmedName],
@@ -293,7 +317,9 @@ function UrlTab({
           disabled={pending}
         />
         {url.length > 0 && !urlValid ? (
-          <InlineHint tone="danger">URL must look like https://, git@…:…, or ssh://…</InlineHint>
+          <InlineHint tone="danger">
+            Enter a GitHub URL like https://github.com/owner/repo or git@github.com:owner/repo.git
+          </InlineHint>
         ) : null}
       </Field>