Skip to content

Commit 7e80d3d

Browse files
arul28claude
andcommitted
ship: iter 2 — address greptile/codex re-review on PR #248
- projectScaffoldService.createLocalProject + cloneRepository: rollback the directory we created if any later step fails, so a partial scaffold doesn't permanently trip the target_exists guard on retry. Skip rollback when the dir pre-existed (greptile P1 #3184743711) - githubService.publishCurrentProject: when createRepository 422s with "name already exists" and the existing repo is empty (size=0), reuse its cloneUrl instead of dying. Throw a coded repo_name_taken with a friendly message when the existing repo has commits (codex P1 #3184743954) - useGithubProjectRemote + GitHubStatus: expose hasOrigin separately from repo (which only tracks GitHub origins). TopBar Publish pill now hides for ANY existing origin, including non-GitHub remotes that would otherwise dead-end at remote_already_exists (codex P2 #3184743960) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9985fe5 commit 7e80d3d

7 files changed

Lines changed: 371 additions & 70 deletions

File tree

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

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,46 @@ describe("githubService.getStatus", () => {
570570
expect(status.repoAccessOk).toBeNull();
571571
expect(status.repoAccessError).toBeNull();
572572
});
573+
574+
it("reports hasOrigin=true with repo=null when origin is non-GitHub (GitLab/Bitbucket)", async () => {
575+
runGitMock.mockResolvedValueOnce({
576+
exitCode: 0,
577+
stdout: "git@gitlab.com:acme/internal.git\n",
578+
stderr: "",
579+
});
580+
process.env.GITHUB_TOKEN = "ghp_classic";
581+
mockFetch.mockResolvedValueOnce(
582+
jsonResponse(200, { login: "alice" }, { "x-oauth-scopes": "repo, workflow" }),
583+
);
584+
585+
const status = await makeService().getStatus();
586+
587+
expect(status.repo).toBeNull();
588+
expect(status.hasOrigin).toBe(true);
589+
});
590+
591+
it("reports hasOrigin=false when no origin remote is configured", async () => {
592+
runGitMock.mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "fatal: no origin" });
593+
594+
const status = await makeService().getStatus();
595+
596+
expect(status.repo).toBeNull();
597+
expect(status.hasOrigin).toBe(false);
598+
});
599+
600+
it("reports hasOrigin=true with repo populated for GitHub origin", async () => {
601+
stubOriginRemote();
602+
process.env.GITHUB_TOKEN = "ghp_classic";
603+
mockFetch.mockResolvedValueOnce(
604+
jsonResponse(200, { login: "alice" }, { "x-oauth-scopes": "repo, workflow" }),
605+
);
606+
mockFetch.mockResolvedValueOnce(jsonResponse(200, { full_name: "acme/ade" }));
607+
608+
const status = await makeService().getStatus();
609+
610+
expect(status.repo).toEqual({ owner: "acme", name: "ade" });
611+
expect(status.hasOrigin).toBe(true);
612+
});
573613
});
574614

575615
// ---------------------------------------------------------------------------
@@ -824,4 +864,68 @@ describe("githubService.publishCurrentProject", () => {
824864
const gitCalls = runGitMock.mock.calls.map((c) => c[0]);
825865
expect(gitCalls).toContainEqual(["remote", "remove", "origin"]);
826866
});
867+
868+
it("recovers from 'name already exists' by reusing an empty existing repo", async () => {
869+
runGitMock
870+
.mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "" }) // get-url origin
871+
.mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" }) // remote add origin
872+
.mockResolvedValueOnce({ exitCode: 0, stdout: "abc\n", stderr: "" }) // rev-parse HEAD
873+
.mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" }); // push
874+
875+
// 1) createRepository → 422 already exists
876+
mockFetch.mockResolvedValueOnce(
877+
jsonResponse(422, {
878+
message: "Repository creation failed",
879+
errors: [{ message: "name already exists on this account" }],
880+
}),
881+
);
882+
// 2) validateToken /user → returns owner login
883+
mockFetch.mockResolvedValueOnce(jsonResponse(200, { login: "alice" }));
884+
// 3) getRepository /repos/alice/proj → empty repo
885+
mockFetch.mockResolvedValueOnce(
886+
jsonResponse(200, {
887+
clone_url: "https://github.com/alice/proj.git",
888+
ssh_url: "git@github.com:alice/proj.git",
889+
html_url: "https://github.com/alice/proj",
890+
default_branch: "main",
891+
size: 0,
892+
}),
893+
);
894+
895+
const result = await makeService().publishCurrentProject({
896+
name: "proj",
897+
isPrivate: true,
898+
});
899+
900+
expect(result).toEqual({ state: "pushed", htmlUrl: "https://github.com/alice/proj" });
901+
});
902+
903+
it("throws repo_name_taken when the existing repo has commits", async () => {
904+
runGitMock.mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "" }); // get-url origin
905+
906+
mockFetch.mockResolvedValueOnce(
907+
jsonResponse(422, {
908+
message: "Repository creation failed",
909+
errors: [{ message: "name already exists on this account" }],
910+
}),
911+
);
912+
mockFetch.mockResolvedValueOnce(jsonResponse(200, { login: "alice" }));
913+
mockFetch.mockResolvedValueOnce(
914+
jsonResponse(200, {
915+
clone_url: "https://github.com/alice/proj.git",
916+
html_url: "https://github.com/alice/proj",
917+
default_branch: "main",
918+
size: 1234,
919+
}),
920+
);
921+
922+
let caught: any;
923+
try {
924+
await makeService().publishCurrentProject({ name: "proj", isPrivate: true });
925+
} catch (err) {
926+
caught = err;
927+
}
928+
expect(caught).toBeInstanceOf(Error);
929+
expect(caught.code).toBe("repo_name_taken");
930+
});
827931
});

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

Lines changed: 79 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,18 @@ export function createGithubService({
182182
return parseGitHubRepoFromRemoteUrl(res.stdout);
183183
};
184184

185+
// Returns `{ repo, hasOrigin }` in one git invocation. `hasOrigin` is true
186+
// for ANY `origin` remote (GitLab/Bitbucket/etc.), while `repo` is populated
187+
// only when origin parses as GitHub. Callers use `hasOrigin` to hide the
188+
// Publish-to-GitHub CTA on projects with non-GitHub remotes.
189+
const detectOrigin = async (): Promise<{ repo: GitHubRepoRef | null; hasOrigin: boolean }> => {
190+
const res = await runGit(["remote", "get-url", "origin"], { cwd: projectRoot, timeoutMs: 8000 });
191+
if (res.exitCode !== 0) return { repo: null, hasOrigin: false };
192+
const url = res.stdout.trim();
193+
if (!url) return { repo: null, hasOrigin: false };
194+
return { repo: parseGitHubRepoFromRemoteUrl(url), hasOrigin: true };
195+
};
196+
185197
const validateToken = async (token: string): Promise<{ userLogin: string | null; scopes: string[]; tokenType: GitHubStatus["tokenType"] }> => {
186198
const response = await fetch("https://api.github.com/user", {
187199
method: "GET",
@@ -399,14 +411,15 @@ export function createGithubService({
399411
cachedAt = 0;
400412
}
401413
const token = readStoredToken();
402-
const repo = await detectRepo().catch(() => null);
414+
const { repo, hasOrigin } = await detectOrigin().catch(() => ({ repo: null, hasOrigin: false }));
403415
if (!token) {
404416
cachedStatus = {
405417
tokenStored: false,
406418
tokenDecryptionFailed,
407419
storageScope: "app",
408420
tokenType: "unknown",
409421
repo,
422+
hasOrigin,
410423
userLogin: null,
411424
scopes: [],
412425
checkedAt: null,
@@ -435,7 +448,7 @@ export function createGithubService({
435448
repo,
436449
repoAccessOk,
437450
});
438-
return { ...cachedStatus, repo, repoAccessOk, repoAccessError, connected };
451+
return { ...cachedStatus, repo, hasOrigin, repoAccessOk, repoAccessError, connected };
439452
}
440453

441454
try {
@@ -468,6 +481,7 @@ export function createGithubService({
468481
storageScope: "app",
469482
tokenType: validated.tokenType,
470483
repo,
484+
hasOrigin,
471485
userLogin: validated.userLogin,
472486
scopes: validated.scopes,
473487
checkedAt: nowIso(),
@@ -485,6 +499,7 @@ export function createGithubService({
485499
storageScope: "app",
486500
tokenType: detectGitHubTokenType(token),
487501
repo,
502+
hasOrigin,
488503
userLogin: null,
489504
scopes: [],
490505
checkedAt: nowIso(),
@@ -670,10 +685,34 @@ export function createGithubService({
670685
};
671686
};
672687

688+
const getRepository = async (
689+
owner: string,
690+
name: string,
691+
): Promise<{
692+
cloneUrl: string;
693+
sshUrl: string;
694+
htmlUrl: string;
695+
defaultBranch: string;
696+
size: number;
697+
}> => {
698+
const { data } = await apiRequest<Record<string, unknown>>({
699+
method: "GET",
700+
path: `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}`,
701+
});
702+
return {
703+
cloneUrl: asString(data.clone_url),
704+
sshUrl: asString(data.ssh_url),
705+
htmlUrl: asString(data.html_url),
706+
defaultBranch: asString(data.default_branch) || "main",
707+
size: typeof data.size === "number" ? data.size : 0,
708+
};
709+
};
710+
673711
const publishCurrentProject = async (
674712
args: { name: string; description?: string; isPrivate: boolean },
675713
): Promise<{ state: "pushed" | "remote_added"; htmlUrl: string }> => {
676-
if (!readStoredToken()) {
714+
const token = readStoredToken();
715+
if (!token) {
677716
const err = new Error("GitHub is not connected. Add a token in Settings.") as Error & { code?: string };
678717
err.code = "github_not_connected";
679718
throw err;
@@ -686,11 +725,42 @@ export function createGithubService({
686725
throw err;
687726
}
688727

689-
const created = await createRepository({
690-
name: args.name,
691-
description: args.description,
692-
isPrivate: args.isPrivate,
693-
});
728+
// If a previous publish attempt created the GitHub repo but then failed
729+
// before push (auth/network), `createRepository` will 422 with "name
730+
// already exists". Recover by GETting the existing repo and reusing its
731+
// cloneUrl — but only when it's empty (size=0), so we never overwrite
732+
// an unrelated user repo with the same name.
733+
let created: { cloneUrl: string; sshUrl: string; htmlUrl: string; defaultBranch: string };
734+
try {
735+
created = await createRepository({
736+
name: args.name,
737+
description: args.description,
738+
isPrivate: args.isPrivate,
739+
});
740+
} catch (createErr) {
741+
const message = createErr instanceof Error ? createErr.message : String(createErr);
742+
const isNameTaken = /already exists/i.test(message);
743+
if (!isNameTaken) throw createErr;
744+
745+
const validated = await validateToken(token).catch(() => ({ userLogin: null as string | null }));
746+
const owner = validated.userLogin;
747+
if (!owner) throw createErr;
748+
749+
const existing = await getRepository(owner, args.name);
750+
if (existing.size > 0) {
751+
const taken = new Error(
752+
`A GitHub repo named '${args.name}' already exists on your account and contains commits. Pick a different name.`,
753+
) as Error & { code?: string };
754+
taken.code = "repo_name_taken";
755+
throw taken;
756+
}
757+
created = {
758+
cloneUrl: existing.cloneUrl,
759+
sshUrl: existing.sshUrl,
760+
htmlUrl: existing.htmlUrl,
761+
defaultBranch: existing.defaultBranch,
762+
};
763+
}
694764

695765
// After createRepository succeeds, the GitHub repo exists. If we then
696766
// fail to add the local origin or push to it, we must remove the local
@@ -768,6 +838,7 @@ export function createGithubService({
768838

769839
// Repo creation + publish
770840
createRepository,
841+
getRepository,
771842
publishCurrentProject,
772843

773844
// Polling/picker read helpers

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,48 @@ describe("createLocalProject", () => {
231231
service.createLocalProject({ name: "empty-target", parentDir }),
232232
).resolves.toEqual({ rootPath: empty });
233233
});
234+
235+
it("rolls back the created directory when a step after mkdir fails", async () => {
236+
// init ok, then `git add .` fails after README/.gitignore are written.
237+
runGitMock
238+
.mockResolvedValueOnce(gitOk())
239+
.mockResolvedValueOnce(gitFail("fatal: not a git repository", 128));
240+
241+
const parentDir = makeTempDir("ade-scaffold-rollback-");
242+
const service = createProjectScaffoldService({
243+
logger: makeLogger(),
244+
githubService: makeGithubServiceStub(),
245+
});
246+
247+
const rootPath = path.join(parentDir, "doomed");
248+
await expect(
249+
service.createLocalProject({ name: "doomed", parentDir }),
250+
).rejects.toThrow();
251+
252+
// The directory we created must be gone so a retry isn't blocked by target_exists.
253+
expect(fs.existsSync(rootPath)).toBe(false);
254+
});
255+
256+
it("does not roll back a pre-existing empty directory on failure", async () => {
257+
runGitMock
258+
.mockResolvedValueOnce(gitOk())
259+
.mockResolvedValueOnce(gitFail("boom", 128));
260+
261+
const parentDir = makeTempDir("ade-scaffold-rollback-preexist-");
262+
const rootPath = path.join(parentDir, "user-made");
263+
fs.mkdirSync(rootPath);
264+
265+
const service = createProjectScaffoldService({
266+
logger: makeLogger(),
267+
githubService: makeGithubServiceStub(),
268+
});
269+
270+
await expect(
271+
service.createLocalProject({ name: "user-made", parentDir }),
272+
).rejects.toThrow();
273+
274+
expect(fs.existsSync(rootPath)).toBe(true);
275+
});
234276
});
235277

236278
describe("cloneRepository", () => {
@@ -342,6 +384,29 @@ describe("cloneRepository", () => {
342384
service.cloneRepository({ url: "https://github.com/octocat/Hello-World", parentDir }),
343385
).rejects.toThrow(/repository not found/i);
344386
});
387+
388+
it("rolls back a partial clone directory so retry isn't blocked by target_exists", async () => {
389+
const parentDir = makeTempDir("ade-scaffold-clone-rollback-");
390+
const rootPath = path.join(parentDir, "Hello-World");
391+
392+
// Simulate `git clone` partially populating the dir then failing.
393+
runGitMock.mockImplementationOnce(async () => {
394+
fs.mkdirSync(rootPath);
395+
fs.writeFileSync(path.join(rootPath, "partial"), "x");
396+
return gitFail("fatal: authentication failed");
397+
});
398+
399+
const service = createProjectScaffoldService({
400+
logger: makeLogger(),
401+
githubService: makeGithubServiceStub(),
402+
});
403+
404+
await expect(
405+
service.cloneRepository({ url: "https://github.com/octocat/Hello-World", parentDir }),
406+
).rejects.toThrow();
407+
408+
expect(fs.existsSync(rootPath)).toBe(false);
409+
});
345410
});
346411

347412
describe("listMyGitHubRepos", () => {

0 commit comments

Comments
 (0)