From ad46feaa5d4087386ae9ee0cdd00835d4c63250c Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 08:27:04 -0500 Subject: [PATCH 01/12] feat(sessions): ensure personal repo on POST /api/sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a caller hits POST /api/sessions without a cloneUrl and without an org bound to their auth context, the handler now provisions (or reuses) their personal Recoupable workspace repo at recoupable/- before returning the session row. This unblocks chat.recoupable.com's Path C cutover (recoupable/chat#1748) — previously the chat-side bootstrap had to construct the personal cloneUrl from a client-side display name (e.g. "sweetman.eth" → "sweetman-eth"), which diverged from open-agents' canonical name source (account_info.name with email-local-part fallback) and 502'd at the clone step. Ported from open-agents: - buildPersonalRepoIdentifier, buildPersonalRepoUrl, githubOwner - repositoryExists, createRepository (plain fetch, no Octokit, to match recoup-api's existing lib/github/* style) - ensurePersonalRepo (idempotent check-then-create) - toKebabCase New session-side helper: - resolveSessionCloneUrl picks bodyCloneUrl > org-no-op > ensurePersonalRepo - buildSessionInsertRow takes the resolved cloneUrl as input - createSessionHandler returns 502 if cloneUrl resolution fails Co-Authored-By: Claude Opus 4.7 --- lib/github/__tests__/createRepository.test.ts | 98 ++++++++++++ lib/github/__tests__/repositoryExists.test.ts | 67 +++++++++ lib/github/createRepository.ts | 104 +++++++++++++ lib/github/repositoryExists.ts | 42 ++++++ .../buildPersonalRepoIdentifier.test.ts | 37 +++++ .../__tests__/ensurePersonalRepo.test.ts | 113 ++++++++++++++ lib/recoupable/buildPersonalRepoIdentifier.ts | 24 +++ lib/recoupable/buildPersonalRepoUrl.ts | 18 +++ lib/recoupable/ensurePersonalRepo.ts | 89 +++++++++++ lib/recoupable/githubOwner.ts | 13 ++ .../__tests__/buildSessionInsertRow.test.ts | 16 +- .../createSessionHandler.persistence.test.ts | 32 ++++ .../__tests__/createSessionHandler.test.ts | 3 + .../__tests__/resolveSessionCloneUrl.test.ts | 139 ++++++++++++++++++ lib/sessions/buildSessionInsertRow.ts | 11 +- lib/sessions/createSessionHandler.ts | 19 ++- lib/sessions/resolveSessionCloneUrl.ts | 93 ++++++++++++ lib/string/__tests__/toKebabCase.test.ts | 28 ++++ lib/string/toKebabCase.ts | 17 +++ 19 files changed, 954 insertions(+), 9 deletions(-) create mode 100644 lib/github/__tests__/createRepository.test.ts create mode 100644 lib/github/__tests__/repositoryExists.test.ts create mode 100644 lib/github/createRepository.ts create mode 100644 lib/github/repositoryExists.ts create mode 100644 lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts create mode 100644 lib/recoupable/__tests__/ensurePersonalRepo.test.ts create mode 100644 lib/recoupable/buildPersonalRepoIdentifier.ts create mode 100644 lib/recoupable/buildPersonalRepoUrl.ts create mode 100644 lib/recoupable/ensurePersonalRepo.ts create mode 100644 lib/recoupable/githubOwner.ts create mode 100644 lib/sessions/__tests__/resolveSessionCloneUrl.test.ts create mode 100644 lib/sessions/resolveSessionCloneUrl.ts create mode 100644 lib/string/__tests__/toKebabCase.test.ts create mode 100644 lib/string/toKebabCase.ts diff --git a/lib/github/__tests__/createRepository.test.ts b/lib/github/__tests__/createRepository.test.ts new file mode 100644 index 000000000..1b24c2b6b --- /dev/null +++ b/lib/github/__tests__/createRepository.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { createRepository } from "@/lib/github/createRepository"; + +describe("createRepository", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("rejects invalid names without hitting the network", async () => { + const fetchSpy = vi.spyOn(globalThis, "fetch"); + + const result = await createRepository({ + owner: "recoupable", + name: "bad name with spaces", + token: "tok", + }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/invalid/i); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("creates an org repo on 201, returning clone url + html url", async () => { + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response( + JSON.stringify({ + html_url: "https://github.com/recoupable/sweetman-id-1", + clone_url: "https://github.com/recoupable/sweetman-id-1.git", + owner: { login: "recoupable" }, + name: "sweetman-id-1", + }), + { status: 201, headers: { "content-type": "application/json" } }, + ), + ); + + const result = await createRepository({ + owner: "recoupable", + name: "sweetman-id-1", + token: "tok", + }); + + expect(result).toEqual({ + success: true, + repoUrl: "https://github.com/recoupable/sweetman-id-1", + cloneUrl: "https://github.com/recoupable/sweetman-id-1.git", + owner: "recoupable", + repoName: "sweetman-id-1", + }); + const [url, init] = fetchSpy.mock.calls[0] as [string, RequestInit]; + expect(url).toBe("https://api.github.com/orgs/recoupable/repos"); + expect(init.method).toBe("POST"); + const body = JSON.parse(init.body as string); + expect(body).toMatchObject({ + name: "sweetman-id-1", + private: true, + auto_init: true, + }); + }); + + it("returns name-conflict error on 422", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 422 })); + + const result = await createRepository({ + owner: "recoupable", + name: "sweetman-id-1", + token: "tok", + }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/already exists/i); + }); + + it("returns permission-denied error on 403", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 403 })); + + const result = await createRepository({ + owner: "recoupable", + name: "sweetman-id-1", + token: "tok", + }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/permission/i); + }); + + it("returns network-error on fetch rejection", async () => { + vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("ECONNRESET")); + + const result = await createRepository({ + owner: "recoupable", + name: "sweetman-id-1", + token: "tok", + }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/network/i); + }); +}); diff --git a/lib/github/__tests__/repositoryExists.test.ts b/lib/github/__tests__/repositoryExists.test.ts new file mode 100644 index 000000000..81e5a34c8 --- /dev/null +++ b/lib/github/__tests__/repositoryExists.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { repositoryExists } from "@/lib/github/repositoryExists"; + +describe("repositoryExists", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("returns true on 200", async () => { + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response(null, { status: 200 })); + + const result = await repositoryExists({ + owner: "recoupable", + repo: "sweetman-id-1", + token: "tok", + }); + + expect(result).toBe(true); + expect(fetchSpy).toHaveBeenCalledWith( + "https://api.github.com/repos/recoupable/sweetman-id-1", + expect.objectContaining({ + method: "GET", + headers: expect.objectContaining({ + Authorization: "Bearer tok", + }), + }), + ); + }); + + it("returns false on 404", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 404 })); + + expect( + await repositoryExists({ + owner: "recoupable", + repo: "missing", + token: "tok", + }), + ).toBe(false); + }); + + it("returns null on other statuses (auth, rate limit, etc.)", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 403 })); + + expect( + await repositoryExists({ + owner: "recoupable", + repo: "rate-limited", + token: "tok", + }), + ).toBeNull(); + }); + + it("returns null on network failure", async () => { + vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("ECONNRESET")); + + expect( + await repositoryExists({ + owner: "recoupable", + repo: "anything", + token: "tok", + }), + ).toBeNull(); + }); +}); diff --git a/lib/github/createRepository.ts b/lib/github/createRepository.ts new file mode 100644 index 000000000..fa3b69cec --- /dev/null +++ b/lib/github/createRepository.ts @@ -0,0 +1,104 @@ +export interface CreateRepositoryResult { + success: boolean; + /** GitHub UI URL (`html_url`) — `https://github.com//`. */ + repoUrl?: string; + /** Git clone URL (`clone_url`) — same shape, used by sandboxes to `git clone`. */ + cloneUrl?: string; + owner?: string; + repoName?: string; + /** Human-readable error message; only set when `success` is false. */ + error?: string; +} + +/** + * Create a GitHub repository under a Recoupable-owned organization. + * + * Ported from open-agents `apps/web/lib/github/client.ts#createRepository` + * with two intentional reductions: + * - Plain `fetch` (no Octokit) to match recoup-api's existing + * `lib/github/*` style. + * - Org-only creation: open-agents allowed a `User` `accountType` + * fallback, but Recoupable workspace repos are always created + * under the `recoupable` org (see `RECOUPABLE_GITHUB_OWNER`), so + * the `createForAuthenticatedUser` branch is unreachable in this + * codebase and has been dropped. + * + * `auto_init: true` so the repo has an initial `main` branch the + * sandbox can `git clone`. Without it, a clone of a 0-commit repo + * fails. + */ +export async function createRepository(params: { + owner: string; + name: string; + description?: string; + isPrivate?: boolean; + token: string; +}): Promise { + const { owner, name, description = "", isPrivate = true, token } = params; + + if (!/^[\w.-]+$/.test(name)) { + return { + success: false, + error: + "Invalid repository name. Use only letters, numbers, hyphens, underscores, and periods.", + }; + } + + try { + const response = await fetch(`https://api.github.com/orgs/${owner}/repos`, { + method: "POST", + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + "X-GitHub-Api-Version": "2022-11-28", + }, + body: JSON.stringify({ + name, + description, + private: isPrivate, + auto_init: true, + }), + }); + + if (response.status === 201) { + const data = (await response.json()) as { + html_url: string; + clone_url: string; + owner: { login: string }; + name: string; + }; + return { + success: true, + repoUrl: data.html_url, + cloneUrl: data.clone_url, + owner: data.owner.login, + repoName: data.name, + }; + } + + if (response.status === 422) { + return { + success: false, + error: "Repository name already exists or is invalid", + }; + } + if (response.status === 403) { + return { success: false, error: "Permission denied" }; + } + + let body = ""; + try { + body = await response.text(); + } catch { + body = ""; + } + console.error( + `[createRepository] unexpected status ${response.status} for ${owner}/${name}: ${body}`, + ); + return { success: false, error: `GitHub returned ${response.status}` }; + } catch (error) { + console.error("[createRepository] network error:", error); + return { success: false, error: "Network error talking to GitHub" }; + } +} diff --git a/lib/github/repositoryExists.ts b/lib/github/repositoryExists.ts new file mode 100644 index 000000000..bf7f6e42a --- /dev/null +++ b/lib/github/repositoryExists.ts @@ -0,0 +1,42 @@ +/** + * Returns `true` if `/` exists, `false` if 404, `null` on + * any other failure (auth, rate limit, network). Lets callers + * distinguish "doesn't exist yet" from "couldn't reach GitHub" before + * attempting destructive ops like create. + * + * Ported from open-agents `apps/web/lib/github/repository-exists.ts` — + * recoup-api uses plain `fetch` rather than Octokit to match the + * existing `lib/github/*` style. + * + * @param owner - GitHub owner (org or user login). + * @param repo - Repository name (the part after the slash). + * @param token - GitHub PAT or service token. Required — public + * `GET /repos/{owner}/{repo}` is rate-limited and private repos 404 + * when unauthenticated, so an unauthenticated probe is ambiguous. + */ +export async function repositoryExists(params: { + owner: string; + repo: string; + token: string; +}): Promise { + const { owner, repo, token } = params; + try { + const response = await fetch(`https://api.github.com/repos/${owner}/${repo}`, { + method: "GET", + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "X-GitHub-Api-Version": "2022-11-28", + }, + }); + + if (response.status === 200) return true; + if (response.status === 404) return false; + + console.error(`[repositoryExists] unexpected status ${response.status} for ${owner}/${repo}`); + return null; + } catch (error) { + console.error("[repositoryExists] network error:", error); + return null; + } +} diff --git a/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts b/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts new file mode 100644 index 000000000..fe7e15c6f --- /dev/null +++ b/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts @@ -0,0 +1,37 @@ +import { describe, it, expect } from "vitest"; +import { buildPersonalRepoIdentifier } from "@/lib/recoupable/buildPersonalRepoIdentifier"; +import { buildPersonalRepoUrl } from "@/lib/recoupable/buildPersonalRepoUrl"; + +describe("buildPersonalRepoIdentifier", () => { + it("produces recoupable owner + - repo", () => { + expect( + buildPersonalRepoIdentifier({ + accountName: "Sweetman", + accountId: "fb678396-a68f-4294-ae50-b8cacf9ce77b", + }), + ).toEqual({ + owner: "recoupable", + repo: "sweetman-fb678396-a68f-4294-ae50-b8cacf9ce77b", + }); + }); + + it("kebabs dotted display names", () => { + expect( + buildPersonalRepoIdentifier({ + accountName: "sweetman.eth", + accountId: "id-1", + }), + ).toEqual({ owner: "recoupable", repo: "sweetman-eth-id-1" }); + }); +}); + +describe("buildPersonalRepoUrl", () => { + it("composes the GitHub URL from the identifier", () => { + expect( + buildPersonalRepoUrl({ + accountName: "Sweetman", + accountId: "fb678396-a68f-4294-ae50-b8cacf9ce77b", + }), + ).toBe("https://github.com/recoupable/sweetman-fb678396-a68f-4294-ae50-b8cacf9ce77b"); + }); +}); diff --git a/lib/recoupable/__tests__/ensurePersonalRepo.test.ts b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts new file mode 100644 index 000000000..f1869f9c5 --- /dev/null +++ b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { ensurePersonalRepo } from "@/lib/recoupable/ensurePersonalRepo"; +import { repositoryExists } from "@/lib/github/repositoryExists"; +import { createRepository } from "@/lib/github/createRepository"; +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; + +vi.mock("@/lib/github/getServiceGithubToken", () => ({ + getServiceGithubToken: vi.fn(), +})); +vi.mock("@/lib/github/repositoryExists", () => ({ + repositoryExists: vi.fn(), +})); +vi.mock("@/lib/github/createRepository", () => ({ + createRepository: vi.fn(), +})); + +describe("ensurePersonalRepo", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns null when GITHUB_TOKEN is missing", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue(undefined); + + const result = await ensurePersonalRepo({ + accountName: "Sweetman", + accountId: "id-1", + }); + + expect(result).toBeNull(); + expect(repositoryExists).not.toHaveBeenCalled(); + expect(createRepository).not.toHaveBeenCalled(); + }); + + it("returns existing repo URL without creating when repository already exists", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + vi.mocked(repositoryExists).mockResolvedValue(true); + + const result = await ensurePersonalRepo({ + accountName: "Sweetman", + accountId: "id-1", + }); + + expect(result).toEqual({ + cloneUrl: "https://github.com/recoupable/sweetman-id-1", + repoUrl: "https://github.com/recoupable/sweetman-id-1", + owner: "recoupable", + repoName: "sweetman-id-1", + }); + expect(createRepository).not.toHaveBeenCalled(); + }); + + it("returns null when the existence check fails for non-404 reasons", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + vi.mocked(repositoryExists).mockResolvedValue(null); + + const result = await ensurePersonalRepo({ + accountName: "Sweetman", + accountId: "id-1", + }); + + expect(result).toBeNull(); + expect(createRepository).not.toHaveBeenCalled(); + }); + + it("creates the repo when it doesn't exist and returns the new URL", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + vi.mocked(repositoryExists).mockResolvedValue(false); + vi.mocked(createRepository).mockResolvedValue({ + success: true, + cloneUrl: "https://github.com/recoupable/sweetman-id-1.git", + repoUrl: "https://github.com/recoupable/sweetman-id-1", + owner: "recoupable", + repoName: "sweetman-id-1", + }); + + const result = await ensurePersonalRepo({ + accountName: "Sweetman", + accountId: "id-1", + }); + + expect(result).toEqual({ + cloneUrl: "https://github.com/recoupable/sweetman-id-1.git", + repoUrl: "https://github.com/recoupable/sweetman-id-1", + owner: "recoupable", + repoName: "sweetman-id-1", + }); + expect(createRepository).toHaveBeenCalledWith( + expect.objectContaining({ + owner: "recoupable", + name: "sweetman-id-1", + isPrivate: true, + token: "tok", + }), + ); + }); + + it("returns null when repo creation fails", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + vi.mocked(repositoryExists).mockResolvedValue(false); + vi.mocked(createRepository).mockResolvedValue({ + success: false, + error: "Permission denied", + }); + + const result = await ensurePersonalRepo({ + accountName: "Sweetman", + accountId: "id-1", + }); + + expect(result).toBeNull(); + }); +}); diff --git a/lib/recoupable/buildPersonalRepoIdentifier.ts b/lib/recoupable/buildPersonalRepoIdentifier.ts new file mode 100644 index 000000000..c1340a0cc --- /dev/null +++ b/lib/recoupable/buildPersonalRepoIdentifier.ts @@ -0,0 +1,24 @@ +import { toKebabCase } from "@/lib/string/toKebabCase"; +import { RECOUPABLE_GITHUB_OWNER } from "./githubOwner"; + +/** + * Returns the `` pair for an account's personal Recoupable + * workspace, mirroring `buildPersonalRepoUrl` for callers that talk to + * the GitHub API directly without re-parsing the URL. + * + * Convention: owner = `recoupable`, repo = `-`. + * + * Ported from open-agents + * `apps/web/lib/recoupable/build-personal-repo-identifier.ts` — keep in + * lockstep with `buildPersonalRepoUrl` and the chat-side helper. + */ +export function buildPersonalRepoIdentifier(params: { accountName: string; accountId: string }): { + owner: string; + repo: string; +} { + const slug = toKebabCase(params.accountName); + return { + owner: RECOUPABLE_GITHUB_OWNER, + repo: `${slug}-${params.accountId}`, + }; +} diff --git a/lib/recoupable/buildPersonalRepoUrl.ts b/lib/recoupable/buildPersonalRepoUrl.ts new file mode 100644 index 000000000..86a1283b9 --- /dev/null +++ b/lib/recoupable/buildPersonalRepoUrl.ts @@ -0,0 +1,18 @@ +import { buildPersonalRepoIdentifier } from "./buildPersonalRepoIdentifier"; + +/** + * Builds the GitHub URL for an account's per-account ("personal") + * repository, used as the fallback when the user has no Recoupable + * organization selected. + * Convention: `https://github.com/recoupable/-`. + * + * Example: `recoupable/sweetman-fb678396-a68f-4294-ae50-b8cacf9ce77b`. + * + * Ported from open-agents `apps/web/lib/recoupable/build-personal-repo-url.ts` + * — keep in lockstep with the chat-side helper and + * `buildPersonalRepoIdentifier`. + */ +export function buildPersonalRepoUrl(params: { accountName: string; accountId: string }): string { + const { owner, repo } = buildPersonalRepoIdentifier(params); + return `https://github.com/${owner}/${repo}`; +} diff --git a/lib/recoupable/ensurePersonalRepo.ts b/lib/recoupable/ensurePersonalRepo.ts new file mode 100644 index 000000000..882fbaf26 --- /dev/null +++ b/lib/recoupable/ensurePersonalRepo.ts @@ -0,0 +1,89 @@ +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; +import { createRepository } from "@/lib/github/createRepository"; +import { repositoryExists } from "@/lib/github/repositoryExists"; +import { buildPersonalRepoIdentifier } from "./buildPersonalRepoIdentifier"; +import { buildPersonalRepoUrl } from "./buildPersonalRepoUrl"; +import { RECOUPABLE_GITHUB_OWNER } from "./githubOwner"; + +export interface EnsurePersonalRepoResult { + cloneUrl: string; + repoUrl: string; + owner: string; + repoName: string; +} + +/** + * Idempotently ensures a personal repo exists for the given account. + * + * Naming follows `recoupable/-` (see + * `buildPersonalRepoUrl`). We check existence with `GET /repos/...` + * first so a pre-existing repo is a clean no-op; only when the repo + * is genuinely absent (404) do we attempt creation. Avoids the 422 + * ambiguity where the GitHub API returns the same status for "name + * taken" and "name invalid". + * + * Returns `null` when the service token is missing, the existence + * check fails for non-404 reasons, or creation fails — all treated + * as fatal by callers. The caller is responsible for surfacing a + * user-visible error (`createSessionHandler` returns a 502). + * + * Ported from open-agents + * `apps/web/lib/recoupable/ensure-personal-repo.ts` — keep behavior + * in lockstep so chat.recoupable.com and sandbox.recoupable.com + * converge on the same repo for the same account. + */ +export async function ensurePersonalRepo(params: { + accountName: string; + accountId: string; +}): Promise { + const token = getServiceGithubToken(); + if (!token) { + console.error("[ensurePersonalRepo] GITHUB_TOKEN missing; cannot ensure repo"); + return null; + } + + const { repo: repoName } = buildPersonalRepoIdentifier({ + accountName: params.accountName, + accountId: params.accountId, + }); + + const existing = await repositoryExists({ + owner: RECOUPABLE_GITHUB_OWNER, + repo: repoName, + token, + }); + + if (existing === null) { + console.error(`[ensurePersonalRepo] failed to check ${RECOUPABLE_GITHUB_OWNER}/${repoName}`); + return null; + } + + if (existing) { + return { + cloneUrl: buildPersonalRepoUrl(params), + repoUrl: `https://github.com/${RECOUPABLE_GITHUB_OWNER}/${repoName}`, + owner: RECOUPABLE_GITHUB_OWNER, + repoName, + }; + } + + const created = await createRepository({ + owner: RECOUPABLE_GITHUB_OWNER, + name: repoName, + description: `Personal Recoupable workspace for account ${params.accountId}`, + isPrivate: true, + token, + }); + + if (!created.success || !created.cloneUrl || !created.repoUrl) { + console.error(`[ensurePersonalRepo] createRepository failed: ${created.error ?? "unknown"}`); + return null; + } + + return { + cloneUrl: created.cloneUrl, + repoUrl: created.repoUrl, + owner: created.owner ?? RECOUPABLE_GITHUB_OWNER, + repoName: created.repoName ?? repoName, + }; +} diff --git a/lib/recoupable/githubOwner.ts b/lib/recoupable/githubOwner.ts new file mode 100644 index 000000000..a87ee71a2 --- /dev/null +++ b/lib/recoupable/githubOwner.ts @@ -0,0 +1,13 @@ +/** + * The GitHub organization that owns every Recoupable workspace repo, + * both per-Recoupable-org repos (`recoupable/org--`) and + * per-account personal repos (`recoupable/-`). + * + * Single source of truth so renames or alternate-environment overrides + * stay in lockstep across builders, parsers, and GitHub API callers. + * Mirrors open-agents `apps/web/lib/recoupable/github-owner.ts` and + * chat `lib/recoupable/githubOwner.ts` — all three surfaces must + * agree on this value, otherwise sandboxes try to clone from + * different orgs depending on which surface created them. + */ +export const RECOUPABLE_GITHUB_OWNER = "recoupable"; diff --git a/lib/sessions/__tests__/buildSessionInsertRow.test.ts b/lib/sessions/__tests__/buildSessionInsertRow.test.ts index a786871f5..6d54d0e3c 100644 --- a/lib/sessions/__tests__/buildSessionInsertRow.test.ts +++ b/lib/sessions/__tests__/buildSessionInsertRow.test.ts @@ -3,7 +3,12 @@ import { buildSessionInsertRow } from "@/lib/sessions/buildSessionInsertRow"; describe("buildSessionInsertRow", () => { it("returns sane defaults for an empty body", () => { - const row = buildSessionInsertRow({ body: {}, accountId: "acc-1", title: "Berlin" }); + const row = buildSessionInsertRow({ + body: {}, + accountId: "acc-1", + title: "Berlin", + cloneUrl: null, + }); expect(row.account_id).toBe("acc-1"); expect(row.title).toBe("Berlin"); expect(row.status).toBe("running"); @@ -15,14 +20,12 @@ describe("buildSessionInsertRow", () => { expect(row.id).toMatch(/^[0-9a-f-]{36}$/i); }); - it("forwards branch + clone fields verbatim", () => { + it("writes the resolved cloneUrl onto clone_url", () => { const row = buildSessionInsertRow({ - body: { - branch: "main", - cloneUrl: "https://github.com/recoupable/ai.git", - }, + body: { branch: "main" }, accountId: "acc-1", title: "Berlin", + cloneUrl: "https://github.com/recoupable/ai.git", }); expect(row.branch).toBe("main"); expect(row.clone_url).toBe("https://github.com/recoupable/ai.git"); @@ -33,6 +36,7 @@ describe("buildSessionInsertRow", () => { body: { sandboxType: "vercel" }, accountId: "acc-1", title: "Berlin", + cloneUrl: null, }); expect(row.sandbox_state).toEqual({ type: "vercel" }); }); diff --git a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts index 038889a3d..66e52963d 100644 --- a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts @@ -5,6 +5,7 @@ import { insertSession } from "@/lib/supabase/sessions/insertSession"; import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; import { insertChat } from "@/lib/supabase/chats/insertChat"; import { resolveSessionTitle } from "@/lib/sessions/resolveSessionTitle"; +import { resolveSessionCloneUrl } from "@/lib/sessions/resolveSessionCloneUrl"; import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow"; import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; @@ -22,6 +23,9 @@ vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); vi.mock("@/lib/sessions/resolveSessionTitle", () => ({ resolveSessionTitle: vi.fn(async () => "Anchorage"), })); +vi.mock("@/lib/sessions/resolveSessionCloneUrl", () => ({ + resolveSessionCloneUrl: vi.fn(async () => ({ ok: true, cloneUrl: null })), +})); const okValidated = (overrides: { body?: object; accountId?: string } = {}) => ({ body: overrides.body ?? {}, @@ -95,6 +99,34 @@ describe("createSessionHandler — persistence", () => { expect(deleteSessionById).toHaveBeenCalledWith("sess_rollback"); }); + it("returns 502 when resolveSessionCloneUrl fails (e.g. personal repo provisioning blew up)", async () => { + vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); + vi.mocked(resolveSessionCloneUrl).mockResolvedValueOnce({ + ok: false, + error: "Failed to provision personal repository", + }); + + const res = await createSessionHandler(makeCreateSessionReq({})); + expect(res.status).toBe(502); + expect(insertSession).not.toHaveBeenCalled(); + }); + + it("forwards the resolved cloneUrl onto the inserted session row", async () => { + vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); + vi.mocked(resolveSessionCloneUrl).mockResolvedValueOnce({ + ok: true, + cloneUrl: "https://github.com/recoupable/sweetman-acc-uuid-1", + }); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow()); + vi.mocked(insertChat).mockResolvedValue(baseChatRow()); + + await createSessionHandler(makeCreateSessionReq({})); + + expect(vi.mocked(insertSession).mock.calls[0][0].clone_url).toBe( + "https://github.com/recoupable/sweetman-acc-uuid-1", + ); + }); + it("logs an orphan-session error when rollback also fails", async () => { vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ id: "sess_orphan" })); diff --git a/lib/sessions/__tests__/createSessionHandler.test.ts b/lib/sessions/__tests__/createSessionHandler.test.ts index a239ccd82..cada68e71 100644 --- a/lib/sessions/__tests__/createSessionHandler.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.test.ts @@ -18,6 +18,9 @@ vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); vi.mock("@/lib/sessions/resolveSessionTitle", () => ({ resolveSessionTitle: vi.fn(async () => "Anchorage"), })); +vi.mock("@/lib/sessions/resolveSessionCloneUrl", () => ({ + resolveSessionCloneUrl: vi.fn(async () => ({ ok: true, cloneUrl: null })), +})); describe("createSessionHandler — short-circuits on validation failure", () => { beforeEach(() => vi.clearAllMocks()); diff --git a/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts b/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts new file mode 100644 index 000000000..6ceaad153 --- /dev/null +++ b/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts @@ -0,0 +1,139 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { resolveSessionCloneUrl } from "@/lib/sessions/resolveSessionCloneUrl"; +import { getAccountWithDetails } from "@/lib/supabase/accounts/getAccountWithDetails"; +import { ensurePersonalRepo } from "@/lib/recoupable/ensurePersonalRepo"; +import type { AuthContext } from "@/lib/auth/validateAuthContext"; + +vi.mock("@/lib/supabase/accounts/getAccountWithDetails", () => ({ + getAccountWithDetails: vi.fn(), +})); +vi.mock("@/lib/recoupable/ensurePersonalRepo", () => ({ + ensurePersonalRepo: vi.fn(), +})); + +const baseAuth: AuthContext = { + accountId: "acc-1", + orgId: null, + authToken: "tok", +}; + +describe("resolveSessionCloneUrl", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("uses the body cloneUrl when provided, regardless of org state", async () => { + const result = await resolveSessionCloneUrl({ + bodyCloneUrl: "https://github.com/recoupable/org-foo-id-1", + auth: { ...baseAuth, orgId: "org-1" }, + }); + + expect(result).toEqual({ + ok: true, + cloneUrl: "https://github.com/recoupable/org-foo-id-1", + }); + expect(getAccountWithDetails).not.toHaveBeenCalled(); + expect(ensurePersonalRepo).not.toHaveBeenCalled(); + }); + + it("returns cloneUrl=null when no body cloneUrl and an org is bound (current narrow scope)", async () => { + const result = await resolveSessionCloneUrl({ + bodyCloneUrl: undefined, + auth: { ...baseAuth, orgId: "org-1" }, + }); + + expect(result).toEqual({ ok: true, cloneUrl: null }); + expect(ensurePersonalRepo).not.toHaveBeenCalled(); + }); + + it("provisions a personal repo when no body cloneUrl and no org", async () => { + vi.mocked(getAccountWithDetails).mockResolvedValue({ + name: "Sweetman", + email: "sweetman@example.com", + } as never); + vi.mocked(ensurePersonalRepo).mockResolvedValue({ + cloneUrl: "https://github.com/recoupable/sweetman-acc-1", + repoUrl: "https://github.com/recoupable/sweetman-acc-1", + owner: "recoupable", + repoName: "sweetman-acc-1", + }); + + const result = await resolveSessionCloneUrl({ + bodyCloneUrl: undefined, + auth: baseAuth, + }); + + expect(result).toEqual({ + ok: true, + cloneUrl: "https://github.com/recoupable/sweetman-acc-1", + }); + expect(ensurePersonalRepo).toHaveBeenCalledWith({ + accountName: "Sweetman", + accountId: "acc-1", + }); + }); + + it("falls back to email local-part when name is missing", async () => { + vi.mocked(getAccountWithDetails).mockResolvedValue({ + name: null, + email: "sweetmantech@gmail.com", + } as never); + vi.mocked(ensurePersonalRepo).mockResolvedValue({ + cloneUrl: "url", + repoUrl: "url", + owner: "recoupable", + repoName: "sweetmantech-acc-1", + }); + + await resolveSessionCloneUrl({ + bodyCloneUrl: undefined, + auth: baseAuth, + }); + + expect(ensurePersonalRepo).toHaveBeenCalledWith({ + accountName: "sweetmantech", + accountId: "acc-1", + }); + }); + + it("returns an error when account has no name and no email", async () => { + vi.mocked(getAccountWithDetails).mockResolvedValue({ + name: null, + email: null, + } as never); + + const result = await resolveSessionCloneUrl({ + bodyCloneUrl: undefined, + auth: baseAuth, + }); + + expect(result.ok).toBe(false); + expect(ensurePersonalRepo).not.toHaveBeenCalled(); + }); + + it("returns an error when the account row is missing", async () => { + vi.mocked(getAccountWithDetails).mockResolvedValue(null); + + const result = await resolveSessionCloneUrl({ + bodyCloneUrl: undefined, + auth: baseAuth, + }); + + expect(result.ok).toBe(false); + expect(ensurePersonalRepo).not.toHaveBeenCalled(); + }); + + it("returns an error when ensurePersonalRepo fails", async () => { + vi.mocked(getAccountWithDetails).mockResolvedValue({ + name: "Sweetman", + } as never); + vi.mocked(ensurePersonalRepo).mockResolvedValue(null); + + const result = await resolveSessionCloneUrl({ + bodyCloneUrl: undefined, + auth: baseAuth, + }); + + expect(result.ok).toBe(false); + }); +}); diff --git a/lib/sessions/buildSessionInsertRow.ts b/lib/sessions/buildSessionInsertRow.ts index 8c718f57f..de6dfbce3 100644 --- a/lib/sessions/buildSessionInsertRow.ts +++ b/lib/sessions/buildSessionInsertRow.ts @@ -6,6 +6,13 @@ interface BuildSessionInsertRowInput { body: CreateSessionBody; accountId: string; title: string; + /** + * Final clone URL resolved by `resolveSessionCloneUrl`. When `null`, + * the session row stores no `clone_url` — matches the prior + * `body.cloneUrl ?? null` behavior for callers that don't (yet) + * trigger personal-repo provisioning. + */ + cloneUrl: string | null; } /** @@ -21,14 +28,14 @@ interface BuildSessionInsertRowInput { * @returns A row ready to pass to `insertSession`. */ export function buildSessionInsertRow(input: BuildSessionInsertRowInput): TablesInsert<"sessions"> { - const { body, accountId, title } = input; + const { body, accountId, title, cloneUrl } = input; return { id: generateUUID(), account_id: accountId, title, status: "running", branch: body.branch ?? null, - clone_url: body.cloneUrl ?? null, + clone_url: cloneUrl, global_skill_refs: [], sandbox_state: { type: body.sandboxType ?? "vercel" }, lifecycle_state: "provisioning", diff --git a/lib/sessions/createSessionHandler.ts b/lib/sessions/createSessionHandler.ts index d27b0b9e1..0dc416634 100644 --- a/lib/sessions/createSessionHandler.ts +++ b/lib/sessions/createSessionHandler.ts @@ -3,6 +3,7 @@ import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { generateUUID } from "@/lib/uuid/generateUUID"; import { validateCreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; import { resolveSessionTitle } from "@/lib/sessions/resolveSessionTitle"; +import { resolveSessionCloneUrl } from "@/lib/sessions/resolveSessionCloneUrl"; import { buildSessionInsertRow } from "@/lib/sessions/buildSessionInsertRow"; import { failedToCreateSession } from "@/lib/sessions/failedToCreateSession"; import { insertSession } from "@/lib/supabase/sessions/insertSession"; @@ -37,8 +38,24 @@ export async function createSessionHandler(request: NextRequest): Promise-` repo provisioned before + * `POST /api/sandbox` tries to clone it (Path C cutover for + * chat.recoupable.com — recoupable/chat#1747). + * 3. Otherwise (no `cloneUrl`, but an org is bound), return `null` + * so the session row stores no `clone_url` and the sandbox falls + * back to whatever existing behavior handles that case. This + * branch is intentionally narrow — org-repo URL derivation + * stays a client-side concern for now to keep this PR focused + * on personal-repo provisioning. + */ +export async function resolveSessionCloneUrl(params: { + bodyCloneUrl: string | undefined; + auth: AuthContext; +}): Promise { + const { bodyCloneUrl, auth } = params; + + if (bodyCloneUrl) { + return { ok: true, cloneUrl: bodyCloneUrl }; + } + + if (auth.orgId) { + return { ok: true, cloneUrl: null }; + } + + const account = await getAccountWithDetails(auth.accountId); + if (!account) { + return { + ok: false, + error: "Account not found for personal-session provisioning", + }; + } + + const accountName = resolvePersonalRepoAccountName(account); + if (!accountName) { + return { + ok: false, + error: "Account has no name or email to derive a personal repo identifier from", + }; + } + + const ensured = await ensurePersonalRepo({ + accountName, + accountId: auth.accountId, + }); + + if (!ensured) { + return { ok: false, error: "Failed to provision personal repository" }; + } + + return { ok: true, cloneUrl: ensured.cloneUrl }; +} + +interface PersonalRepoAccountFields { + name?: string | null; + email?: string | null; +} + +/** + * Pick the most stable string to kebab into the personal repo slug. + * Prefers `account_info.name`; falls back to the local-part of the + * account's email (matches open-agents `fetchOrCreateAccount` + * fallback). Returns `null` when neither is usable so callers can + * surface a clean error rather than building a slug like `-`. + */ +function resolvePersonalRepoAccountName(account: PersonalRepoAccountFields): string | null { + const trimmedName = account.name?.trim(); + if (trimmedName) return trimmedName; + + const trimmedEmail = account.email?.trim(); + if (trimmedEmail) { + const localPart = trimmedEmail.split("@")[0]; + if (localPart && localPart.length > 0) return localPart; + } + + return null; +} diff --git a/lib/string/__tests__/toKebabCase.test.ts b/lib/string/__tests__/toKebabCase.test.ts new file mode 100644 index 000000000..3329f0e47 --- /dev/null +++ b/lib/string/__tests__/toKebabCase.test.ts @@ -0,0 +1,28 @@ +import { describe, it, expect } from "vitest"; +import { toKebabCase } from "@/lib/string/toKebabCase"; + +describe("toKebabCase", () => { + it("lowercases input", () => { + expect(toKebabCase("Hello")).toBe("hello"); + }); + + it("replaces dots with single hyphen — guards the sweetman.eth -> sweetman-eth bug from chat", () => { + expect(toKebabCase("sweetman.eth")).toBe("sweetman-eth"); + }); + + it("collapses runs of non-alphanumerics into a single hyphen", () => { + expect(toKebabCase("Foo Bar___Baz")).toBe("foo-bar-baz"); + }); + + it("trims leading and trailing hyphens", () => { + expect(toKebabCase("---foo bar---")).toBe("foo-bar"); + }); + + it("preserves digits", () => { + expect(toKebabCase("Artist 42")).toBe("artist-42"); + }); + + it("returns empty string for all-symbol input", () => { + expect(toKebabCase("@@@")).toBe(""); + }); +}); diff --git a/lib/string/toKebabCase.ts b/lib/string/toKebabCase.ts new file mode 100644 index 000000000..18dd4383b --- /dev/null +++ b/lib/string/toKebabCase.ts @@ -0,0 +1,17 @@ +/** + * Converts an arbitrary string into kebab-case slug form. + * - lowercases + * - collapses any non-alphanumeric runs to a single `-` + * - trims leading / trailing `-` + * + * Used to build per-org and per-account repo identifiers from + * human-entered names. Ported from open-agents + * `apps/web/lib/string/to-kebab-case.ts` so chat.recoupable.com, + * sandbox.recoupable.com, and recoup-api all kebab the same way. + */ +export function toKebabCase(input: string): string { + return input + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, ""); +} From a54c39d94a8521098232986a2ac1594990024e70 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 10:41:29 -0500 Subject: [PATCH 02/12] fix(sessions): flatten ResolveSessionCloneUrlResult to single interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Next.js 16's next build doesn't narrow the discriminated union { ok: true; cloneUrl } | { ok: false; error } through `if (!result.ok)`, breaking the Vercel preview build with: Type error: Property 'error' does not exist on type 'ResolveSessionCloneUrlResult'. Same compile-only divergence we hit on PR #603 — vitest's tsc is more permissive than next build's. Flatten to a single interface with `cloneUrl: string | null` and `error?: string` so callers read both fields directly after checking `ok`. Co-Authored-By: Claude Opus 4.7 --- lib/sessions/resolveSessionCloneUrl.ts | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/sessions/resolveSessionCloneUrl.ts b/lib/sessions/resolveSessionCloneUrl.ts index cb14a4af0..1beaac241 100644 --- a/lib/sessions/resolveSessionCloneUrl.ts +++ b/lib/sessions/resolveSessionCloneUrl.ts @@ -2,9 +2,17 @@ import { getAccountWithDetails } from "@/lib/supabase/accounts/getAccountWithDet import { ensurePersonalRepo } from "@/lib/recoupable/ensurePersonalRepo"; import type { AuthContext } from "@/lib/auth/validateAuthContext"; -export type ResolveSessionCloneUrlResult = - | { ok: true; cloneUrl: string | null } - | { ok: false; error: string }; +/** + * Flat shape (not a discriminated union) — Next.js 16's `next build` + * doesn't narrow `{ ok: true; cloneUrl } | { ok: false; error }` + * through `if (!result.ok)` cleanly, so we make `error` optional and + * the caller checks `ok` plus reads `error` directly. + */ +export interface ResolveSessionCloneUrlResult { + ok: boolean; + cloneUrl: string | null; + error?: string; +} /** * Determines the final `clone_url` for a new session row. @@ -43,6 +51,7 @@ export async function resolveSessionCloneUrl(params: { if (!account) { return { ok: false, + cloneUrl: null, error: "Account not found for personal-session provisioning", }; } @@ -51,6 +60,7 @@ export async function resolveSessionCloneUrl(params: { if (!accountName) { return { ok: false, + cloneUrl: null, error: "Account has no name or email to derive a personal repo identifier from", }; } @@ -61,7 +71,11 @@ export async function resolveSessionCloneUrl(params: { }); if (!ensured) { - return { ok: false, error: "Failed to provision personal repository" }; + return { + ok: false, + cloneUrl: null, + error: "Failed to provision personal repository", + }; } return { ok: true, cloneUrl: ensured.cloneUrl }; From 8f161c351b173085521b62aa1f126f0467511232 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 11:11:51 -0500 Subject: [PATCH 03/12] refactor(repo-naming): unify workspace repos as recoupable/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the - slug — account names are mutable, so any URL that embeds the name eventually drifts. The repo URL is now just the account UUID: stable across renames, identical shape for personal and org workspaces, trivial to parse (`recoupable/`). Provisioning flow in ensurePersonalRepo: 1. recoupable/ exists -> return URL (idempotent) 2. legacy - exists (via GitHub search) -> rename to . GitHub auto-redirects the old URL forever, so any sessions.clone_url rows that still reference the old name keep working without a DB backfill. 3. nothing exists -> create fresh recoupable/ extractOrgId regex now accepts both legacy `-` and bare `` shapes so old + new clone URLs both parse. scripts/migrate-workspace-repo-names.ts: one-time backfill. Lists all recoupable org repos, finds those matching ^.+-$, renames each to just . Defaults to dry-run; --apply commits. Idempotent. resolveSessionCloneUrl no longer needs to look up the account row to derive a slug — ensurePersonalRepo only needs the accountId from auth. Deleted unused lib/string/toKebabCase. Co-Authored-By: Claude Opus 4.7 --- .../__tests__/findLegacyAccountRepo.test.ts | 96 ++++++++++++ lib/github/__tests__/renameRepository.test.ts | 93 ++++++++++++ lib/github/findLegacyAccountRepo.ts | 66 ++++++++ lib/github/renameRepository.ts | 89 +++++++++++ .../buildPersonalRepoIdentifier.test.ts | 20 +-- .../__tests__/ensurePersonalRepo.test.ts | 132 +++++++++++----- lib/recoupable/__tests__/extractOrgId.test.ts | 12 ++ lib/recoupable/buildPersonalRepoIdentifier.ts | 26 ++-- lib/recoupable/buildPersonalRepoUrl.ts | 15 +- lib/recoupable/ensurePersonalRepo.ts | 76 +++++++--- lib/recoupable/extractOrgId.ts | 11 +- .../__tests__/resolveSessionCloneUrl.test.ts | 79 +--------- lib/sessions/resolveSessionCloneUrl.ts | 68 ++------- lib/string/__tests__/toKebabCase.test.ts | 28 ---- lib/string/toKebabCase.ts | 17 --- scripts/migrate-workspace-repo-names.ts | 143 ++++++++++++++++++ 16 files changed, 701 insertions(+), 270 deletions(-) create mode 100644 lib/github/__tests__/findLegacyAccountRepo.test.ts create mode 100644 lib/github/__tests__/renameRepository.test.ts create mode 100644 lib/github/findLegacyAccountRepo.ts create mode 100644 lib/github/renameRepository.ts delete mode 100644 lib/string/__tests__/toKebabCase.test.ts delete mode 100644 lib/string/toKebabCase.ts create mode 100644 scripts/migrate-workspace-repo-names.ts diff --git a/lib/github/__tests__/findLegacyAccountRepo.test.ts b/lib/github/__tests__/findLegacyAccountRepo.test.ts new file mode 100644 index 000000000..ec44ac9c4 --- /dev/null +++ b/lib/github/__tests__/findLegacyAccountRepo.test.ts @@ -0,0 +1,96 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { findLegacyAccountRepo } from "@/lib/github/findLegacyAccountRepo"; + +const accountId = "fb678396-a68f-4294-ae50-b8cacf9ce77b"; + +describe("findLegacyAccountRepo", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("returns the legacy name when a *- match exists", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response( + JSON.stringify({ + items: [{ name: `sweetman-${accountId}` }, { name: "some-unrelated-repo" }], + }), + { status: 200, headers: { "content-type": "application/json" } }, + ), + ); + + const result = await findLegacyAccountRepo({ + owner: "recoupable", + accountId, + token: "tok", + }); + + expect(result).toBe(`sweetman-${accountId}`); + }); + + it("ignores the new bare- repo (we don't want to rename it onto itself)", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({ items: [{ name: accountId }] }), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + + const result = await findLegacyAccountRepo({ + owner: "recoupable", + accountId, + token: "tok", + }); + + expect(result).toBeNull(); + }); + + it("returns null when no items match the legacy pattern", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({ items: [] }), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + + const result = await findLegacyAccountRepo({ + owner: "recoupable", + accountId, + token: "tok", + }); + + expect(result).toBeNull(); + }); + + it("returns undefined on non-OK response (caller treats as miss)", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 503 })); + + const result = await findLegacyAccountRepo({ + owner: "recoupable", + accountId, + token: "tok", + }); + + expect(result).toBeUndefined(); + }); + + it("encodes the search query as +in:name+org:", async () => { + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({ items: [] }), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + + await findLegacyAccountRepo({ + owner: "recoupable", + accountId: "id-1", + token: "tok", + }); + + const [url] = fetchSpy.mock.calls[0] as [string]; + expect(url).toContain("/search/repositories?q="); + expect(decodeURIComponent(url.split("?q=")[1].split("&")[0])).toBe( + "id-1 in:name org:recoupable", + ); + }); +}); diff --git a/lib/github/__tests__/renameRepository.test.ts b/lib/github/__tests__/renameRepository.test.ts new file mode 100644 index 000000000..2af9b5689 --- /dev/null +++ b/lib/github/__tests__/renameRepository.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { renameRepository } from "@/lib/github/renameRepository"; + +describe("renameRepository", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("rejects invalid newName without hitting the network", async () => { + const fetchSpy = vi.spyOn(globalThis, "fetch"); + + const result = await renameRepository({ + owner: "recoupable", + repo: "sweetman-id-1", + newName: "bad name", + token: "tok", + }); + + expect(result.success).toBe(false); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("PATCHes /repos/{owner}/{repo} with {name} and returns the new URLs on 200", async () => { + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response( + JSON.stringify({ + html_url: "https://github.com/recoupable/id-1", + clone_url: "https://github.com/recoupable/id-1.git", + }), + { status: 200, headers: { "content-type": "application/json" } }, + ), + ); + + const result = await renameRepository({ + owner: "recoupable", + repo: "sweetman-id-1", + newName: "id-1", + token: "tok", + }); + + expect(result).toEqual({ + success: true, + cloneUrl: "https://github.com/recoupable/id-1.git", + repoUrl: "https://github.com/recoupable/id-1", + }); + const [url, init] = fetchSpy.mock.calls[0] as [string, RequestInit]; + expect(url).toBe("https://api.github.com/repos/recoupable/sweetman-id-1"); + expect(init.method).toBe("PATCH"); + expect(JSON.parse(init.body as string)).toEqual({ name: "id-1" }); + }); + + it("returns target-conflict error on 422", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 422 })); + + const result = await renameRepository({ + owner: "recoupable", + repo: "sweetman-id-1", + newName: "id-1", + token: "tok", + }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/already exists/i); + }); + + it("returns source-not-found error on 404", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 404 })); + + const result = await renameRepository({ + owner: "recoupable", + repo: "sweetman-id-1", + newName: "id-1", + token: "tok", + }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/not found/i); + }); + + it("returns network-error on fetch rejection", async () => { + vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("ECONNRESET")); + + const result = await renameRepository({ + owner: "recoupable", + repo: "sweetman-id-1", + newName: "id-1", + token: "tok", + }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/network/i); + }); +}); diff --git a/lib/github/findLegacyAccountRepo.ts b/lib/github/findLegacyAccountRepo.ts new file mode 100644 index 000000000..5cd0858d6 --- /dev/null +++ b/lib/github/findLegacyAccountRepo.ts @@ -0,0 +1,66 @@ +const LEGACY_SUFFIX_PATTERN = /^.+-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** + * Search the `recoupable` org for a legacy-named workspace repo whose + * name ends with `-` — the old `-` + * convention. Returns the legacy repo name (the one before the + * trailing `-` suffix is what made the name unique) so + * `ensurePersonalRepo` can rename it to the new bare-`` + * convention without losing the user's git history. + * + * Returns: + * - the legacy repo name (e.g. `sweetman-fb678396-...`) on hit + * - `null` on miss (no legacy repo exists for this account) + * - `undefined` on unexpected failure (caller should treat as miss + * and continue with create — losing history is preferable to + * blocking provisioning on a flaky GitHub search) + * + * Uses GitHub's `GET /search/repositories` endpoint with + * `+in:name+org:recoupable` so we don't have to paginate + * the entire org. + */ +export async function findLegacyAccountRepo(params: { + owner: string; + accountId: string; + token: string; +}): Promise { + const { owner, accountId, token } = params; + + try { + const query = encodeURIComponent(`${accountId} in:name org:${owner}`); + const response = await fetch( + `https://api.github.com/search/repositories?q=${query}&per_page=10`, + { + method: "GET", + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "X-GitHub-Api-Version": "2022-11-28", + }, + }, + ); + + if (!response.ok) { + console.error( + `[findLegacyAccountRepo] unexpected status ${response.status} for accountId ${accountId}`, + ); + return undefined; + } + + const data = (await response.json()) as { + items?: Array<{ name: string }>; + }; + + const legacy = (data.items ?? []).find( + item => + item.name !== accountId && + item.name.endsWith(`-${accountId}`) && + LEGACY_SUFFIX_PATTERN.test(item.name), + ); + + return legacy?.name ?? null; + } catch (error) { + console.error("[findLegacyAccountRepo] network error:", error); + return undefined; + } +} diff --git a/lib/github/renameRepository.ts b/lib/github/renameRepository.ts new file mode 100644 index 000000000..979f8ee7f --- /dev/null +++ b/lib/github/renameRepository.ts @@ -0,0 +1,89 @@ +export interface RenameRepositoryResult { + success: boolean; + /** New `clone_url` returned by GitHub on success. */ + cloneUrl?: string; + /** New `html_url` returned by GitHub on success. */ + repoUrl?: string; + error?: string; +} + +/** + * Rename a GitHub repository via `PATCH /repos/{owner}/{repo}`. + * + * GitHub automatically issues HTTP redirects from the old name to the + * new name for both git operations and the REST API, so existing + * `sessions.clone_url` rows that still reference the old name keep + * resolving after the rename — no DB backfill required for clone_url + * (the migration script only renames repos, not stored URLs). + * + * Uses plain `fetch` to match recoup-api's existing `lib/github/*` + * style (no Octokit dependency). + */ +export async function renameRepository(params: { + owner: string; + repo: string; + newName: string; + token: string; +}): Promise { + const { owner, repo, newName, token } = params; + + if (!/^[\w.-]+$/.test(newName)) { + return { + success: false, + error: + "Invalid repository name. Use only letters, numbers, hyphens, underscores, and periods.", + }; + } + + try { + const response = await fetch(`https://api.github.com/repos/${owner}/${repo}`, { + method: "PATCH", + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + "X-GitHub-Api-Version": "2022-11-28", + }, + body: JSON.stringify({ name: newName }), + }); + + if (response.status === 200) { + const data = (await response.json()) as { + html_url: string; + clone_url: string; + }; + return { + success: true, + cloneUrl: data.clone_url, + repoUrl: data.html_url, + }; + } + + if (response.status === 422) { + return { + success: false, + error: "Target repository name already exists or is invalid", + }; + } + if (response.status === 403) { + return { success: false, error: "Permission denied" }; + } + if (response.status === 404) { + return { success: false, error: "Source repository not found" }; + } + + let body = ""; + try { + body = await response.text(); + } catch { + body = ""; + } + console.error( + `[renameRepository] unexpected status ${response.status} for ${owner}/${repo}: ${body}`, + ); + return { success: false, error: `GitHub returned ${response.status}` }; + } catch (error) { + console.error("[renameRepository] network error:", error); + return { success: false, error: "Network error talking to GitHub" }; + } +} diff --git a/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts b/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts index fe7e15c6f..def66d3df 100644 --- a/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts +++ b/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts @@ -3,35 +3,29 @@ import { buildPersonalRepoIdentifier } from "@/lib/recoupable/buildPersonalRepoI import { buildPersonalRepoUrl } from "@/lib/recoupable/buildPersonalRepoUrl"; describe("buildPersonalRepoIdentifier", () => { - it("produces recoupable owner + - repo", () => { + it("returns recoupable owner + bare accountId repo", () => { expect( buildPersonalRepoIdentifier({ - accountName: "Sweetman", accountId: "fb678396-a68f-4294-ae50-b8cacf9ce77b", }), ).toEqual({ owner: "recoupable", - repo: "sweetman-fb678396-a68f-4294-ae50-b8cacf9ce77b", + repo: "fb678396-a68f-4294-ae50-b8cacf9ce77b", }); }); - it("kebabs dotted display names", () => { - expect( - buildPersonalRepoIdentifier({ - accountName: "sweetman.eth", - accountId: "id-1", - }), - ).toEqual({ owner: "recoupable", repo: "sweetman-eth-id-1" }); + it("does not embed any name — naming stays stable across renames", () => { + const { repo } = buildPersonalRepoIdentifier({ accountId: "id-1" }); + expect(repo).toBe("id-1"); }); }); describe("buildPersonalRepoUrl", () => { - it("composes the GitHub URL from the identifier", () => { + it("composes the GitHub URL as recoupable/", () => { expect( buildPersonalRepoUrl({ - accountName: "Sweetman", accountId: "fb678396-a68f-4294-ae50-b8cacf9ce77b", }), - ).toBe("https://github.com/recoupable/sweetman-fb678396-a68f-4294-ae50-b8cacf9ce77b"); + ).toBe("https://github.com/recoupable/fb678396-a68f-4294-ae50-b8cacf9ce77b"); }); }); diff --git a/lib/recoupable/__tests__/ensurePersonalRepo.test.ts b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts index f1869f9c5..8f66731ba 100644 --- a/lib/recoupable/__tests__/ensurePersonalRepo.test.ts +++ b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts @@ -2,6 +2,8 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { ensurePersonalRepo } from "@/lib/recoupable/ensurePersonalRepo"; import { repositoryExists } from "@/lib/github/repositoryExists"; import { createRepository } from "@/lib/github/createRepository"; +import { findLegacyAccountRepo } from "@/lib/github/findLegacyAccountRepo"; +import { renameRepository } from "@/lib/github/renameRepository"; import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; vi.mock("@/lib/github/getServiceGithubToken", () => ({ @@ -13,6 +15,14 @@ vi.mock("@/lib/github/repositoryExists", () => ({ vi.mock("@/lib/github/createRepository", () => ({ createRepository: vi.fn(), })); +vi.mock("@/lib/github/findLegacyAccountRepo", () => ({ + findLegacyAccountRepo: vi.fn(), +})); +vi.mock("@/lib/github/renameRepository", () => ({ + renameRepository: vi.fn(), +})); + +const accountId = "fb678396-a68f-4294-ae50-b8cacf9ce77b"; describe("ensurePersonalRepo", () => { beforeEach(() => { @@ -22,31 +32,26 @@ describe("ensurePersonalRepo", () => { it("returns null when GITHUB_TOKEN is missing", async () => { vi.mocked(getServiceGithubToken).mockReturnValue(undefined); - const result = await ensurePersonalRepo({ - accountName: "Sweetman", - accountId: "id-1", - }); + const result = await ensurePersonalRepo({ accountId }); expect(result).toBeNull(); expect(repositoryExists).not.toHaveBeenCalled(); - expect(createRepository).not.toHaveBeenCalled(); }); - it("returns existing repo URL without creating when repository already exists", async () => { + it("returns the existing repo URL when recoupable/ already exists", async () => { vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(true); - const result = await ensurePersonalRepo({ - accountName: "Sweetman", - accountId: "id-1", - }); + const result = await ensurePersonalRepo({ accountId }); expect(result).toEqual({ - cloneUrl: "https://github.com/recoupable/sweetman-id-1", - repoUrl: "https://github.com/recoupable/sweetman-id-1", + cloneUrl: `https://github.com/recoupable/${accountId}`, + repoUrl: `https://github.com/recoupable/${accountId}`, owner: "recoupable", - repoName: "sweetman-id-1", + repoName: accountId, }); + expect(findLegacyAccountRepo).not.toHaveBeenCalled(); + expect(renameRepository).not.toHaveBeenCalled(); expect(createRepository).not.toHaveBeenCalled(); }); @@ -54,60 +59,113 @@ describe("ensurePersonalRepo", () => { vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(null); - const result = await ensurePersonalRepo({ - accountName: "Sweetman", - accountId: "id-1", + expect(await ensurePersonalRepo({ accountId })).toBeNull(); + expect(findLegacyAccountRepo).not.toHaveBeenCalled(); + }); + + it("renames a legacy - repo when present (history preserved)", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + vi.mocked(repositoryExists).mockResolvedValue(false); + vi.mocked(findLegacyAccountRepo).mockResolvedValue(`sweetman-${accountId}`); + vi.mocked(renameRepository).mockResolvedValue({ + success: true, + cloneUrl: `https://github.com/recoupable/${accountId}.git`, + repoUrl: `https://github.com/recoupable/${accountId}`, }); - expect(result).toBeNull(); + const result = await ensurePersonalRepo({ accountId }); + + expect(renameRepository).toHaveBeenCalledWith({ + owner: "recoupable", + repo: `sweetman-${accountId}`, + newName: accountId, + token: "tok", + }); + expect(result).toEqual({ + cloneUrl: `https://github.com/recoupable/${accountId}`, + repoUrl: `https://github.com/recoupable/${accountId}`, + owner: "recoupable", + repoName: accountId, + }); expect(createRepository).not.toHaveBeenCalled(); }); - it("creates the repo when it doesn't exist and returns the new URL", async () => { + it("falls through to create when the legacy rename fails", async () => { vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(false); + vi.mocked(findLegacyAccountRepo).mockResolvedValue(`sweetman-${accountId}`); + vi.mocked(renameRepository).mockResolvedValue({ + success: false, + error: "boom", + }); vi.mocked(createRepository).mockResolvedValue({ success: true, - cloneUrl: "https://github.com/recoupable/sweetman-id-1.git", - repoUrl: "https://github.com/recoupable/sweetman-id-1", + cloneUrl: `https://github.com/recoupable/${accountId}.git`, + repoUrl: `https://github.com/recoupable/${accountId}`, owner: "recoupable", - repoName: "sweetman-id-1", + repoName: accountId, }); - const result = await ensurePersonalRepo({ - accountName: "Sweetman", - accountId: "id-1", - }); + const result = await ensurePersonalRepo({ accountId }); - expect(result).toEqual({ - cloneUrl: "https://github.com/recoupable/sweetman-id-1.git", - repoUrl: "https://github.com/recoupable/sweetman-id-1", + expect(createRepository).toHaveBeenCalled(); + expect(result?.repoName).toBe(accountId); + }); + + it("creates a fresh repo when no legacy match exists", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + vi.mocked(repositoryExists).mockResolvedValue(false); + vi.mocked(findLegacyAccountRepo).mockResolvedValue(null); + vi.mocked(createRepository).mockResolvedValue({ + success: true, + cloneUrl: `https://github.com/recoupable/${accountId}.git`, + repoUrl: `https://github.com/recoupable/${accountId}`, owner: "recoupable", - repoName: "sweetman-id-1", + repoName: accountId, }); + + const result = await ensurePersonalRepo({ accountId }); + expect(createRepository).toHaveBeenCalledWith( expect.objectContaining({ owner: "recoupable", - name: "sweetman-id-1", + name: accountId, isPrivate: true, token: "tok", }), ); + expect(renameRepository).not.toHaveBeenCalled(); + expect(result?.repoName).toBe(accountId); }); - it("returns null when repo creation fails", async () => { + it("creates a fresh repo when legacy search itself errors (undefined)", async () => { vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(false); + vi.mocked(findLegacyAccountRepo).mockResolvedValue(undefined); vi.mocked(createRepository).mockResolvedValue({ - success: false, - error: "Permission denied", + success: true, + cloneUrl: `https://github.com/recoupable/${accountId}.git`, + repoUrl: `https://github.com/recoupable/${accountId}`, + owner: "recoupable", + repoName: accountId, }); - const result = await ensurePersonalRepo({ - accountName: "Sweetman", - accountId: "id-1", + const result = await ensurePersonalRepo({ accountId }); + + expect(renameRepository).not.toHaveBeenCalled(); + expect(createRepository).toHaveBeenCalled(); + expect(result?.repoName).toBe(accountId); + }); + + it("returns null when creation outright fails", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + vi.mocked(repositoryExists).mockResolvedValue(false); + vi.mocked(findLegacyAccountRepo).mockResolvedValue(null); + vi.mocked(createRepository).mockResolvedValue({ + success: false, + error: "Permission denied", }); - expect(result).toBeNull(); + expect(await ensurePersonalRepo({ accountId })).toBeNull(); }); }); diff --git a/lib/recoupable/__tests__/extractOrgId.test.ts b/lib/recoupable/__tests__/extractOrgId.test.ts index c38232c4c..dc161cc52 100644 --- a/lib/recoupable/__tests__/extractOrgId.test.ts +++ b/lib/recoupable/__tests__/extractOrgId.test.ts @@ -38,6 +38,18 @@ describe("extractOrgId", () => { ); }); + it("extracts the UUID from a bare new-naming repo (recoupable/)", () => { + expect(extractOrgId("https://github.com/recoupable/fb678396-a68f-4294-ae50-b8cacf9ce77b")).toBe( + "fb678396-a68f-4294-ae50-b8cacf9ce77b", + ); + }); + + it("accepts a bare new-naming repo name", () => { + expect(extractOrgId("fb678396-a68f-4294-ae50-b8cacf9ce77b")).toBe( + "fb678396-a68f-4294-ae50-b8cacf9ce77b", + ); + }); + it("returns null for non-Recoupable clone URLs", () => { expect( extractOrgId( diff --git a/lib/recoupable/buildPersonalRepoIdentifier.ts b/lib/recoupable/buildPersonalRepoIdentifier.ts index c1340a0cc..30f22ba5d 100644 --- a/lib/recoupable/buildPersonalRepoIdentifier.ts +++ b/lib/recoupable/buildPersonalRepoIdentifier.ts @@ -1,24 +1,22 @@ -import { toKebabCase } from "@/lib/string/toKebabCase"; import { RECOUPABLE_GITHUB_OWNER } from "./githubOwner"; /** - * Returns the `` pair for an account's personal Recoupable - * workspace, mirroring `buildPersonalRepoUrl` for callers that talk to - * the GitHub API directly without re-parsing the URL. + * Returns the `` pair for an account's Recoupable + * workspace repo. The repo name is the account UUID with no slug + * prefix — keeps the lookup stable across account renames and unifies + * the personal vs. org repo naming so every workspace repo is keyed + * the same way. * - * Convention: owner = `recoupable`, repo = `-`. + * Convention: `recoupable/`. * - * Ported from open-agents - * `apps/web/lib/recoupable/build-personal-repo-identifier.ts` — keep in - * lockstep with `buildPersonalRepoUrl` and the chat-side helper. + * Account names are mutable (`account_info.name` can be edited at any + * time), so any naming scheme that embeds the name eventually drifts + * — see [[feedback_unified_workspace_repo_naming]] for the design call + * to drop the slug entirely. */ -export function buildPersonalRepoIdentifier(params: { accountName: string; accountId: string }): { +export function buildPersonalRepoIdentifier(params: { accountId: string }): { owner: string; repo: string; } { - const slug = toKebabCase(params.accountName); - return { - owner: RECOUPABLE_GITHUB_OWNER, - repo: `${slug}-${params.accountId}`, - }; + return { owner: RECOUPABLE_GITHUB_OWNER, repo: params.accountId }; } diff --git a/lib/recoupable/buildPersonalRepoUrl.ts b/lib/recoupable/buildPersonalRepoUrl.ts index 86a1283b9..68b097065 100644 --- a/lib/recoupable/buildPersonalRepoUrl.ts +++ b/lib/recoupable/buildPersonalRepoUrl.ts @@ -1,18 +1,13 @@ import { buildPersonalRepoIdentifier } from "./buildPersonalRepoIdentifier"; /** - * Builds the GitHub URL for an account's per-account ("personal") - * repository, used as the fallback when the user has no Recoupable - * organization selected. - * Convention: `https://github.com/recoupable/-`. + * Builds the GitHub URL for an account's Recoupable workspace repo. + * Convention: `https://github.com/recoupable/` — the + * account UUID is the repo name with no slug prefix. * - * Example: `recoupable/sweetman-fb678396-a68f-4294-ae50-b8cacf9ce77b`. - * - * Ported from open-agents `apps/web/lib/recoupable/build-personal-repo-url.ts` - * — keep in lockstep with the chat-side helper and - * `buildPersonalRepoIdentifier`. + * Example: `https://github.com/recoupable/fb678396-a68f-4294-ae50-b8cacf9ce77b`. */ -export function buildPersonalRepoUrl(params: { accountName: string; accountId: string }): string { +export function buildPersonalRepoUrl(params: { accountId: string }): string { const { owner, repo } = buildPersonalRepoIdentifier(params); return `https://github.com/${owner}/${repo}`; } diff --git a/lib/recoupable/ensurePersonalRepo.ts b/lib/recoupable/ensurePersonalRepo.ts index 882fbaf26..2b841e3e3 100644 --- a/lib/recoupable/ensurePersonalRepo.ts +++ b/lib/recoupable/ensurePersonalRepo.ts @@ -1,6 +1,8 @@ import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; import { createRepository } from "@/lib/github/createRepository"; import { repositoryExists } from "@/lib/github/repositoryExists"; +import { findLegacyAccountRepo } from "@/lib/github/findLegacyAccountRepo"; +import { renameRepository } from "@/lib/github/renameRepository"; import { buildPersonalRepoIdentifier } from "./buildPersonalRepoIdentifier"; import { buildPersonalRepoUrl } from "./buildPersonalRepoUrl"; import { RECOUPABLE_GITHUB_OWNER } from "./githubOwner"; @@ -13,27 +15,30 @@ export interface EnsurePersonalRepoResult { } /** - * Idempotently ensures a personal repo exists for the given account. + * Idempotently ensure an account has a workspace repo at the + * canonical `recoupable/` location. * - * Naming follows `recoupable/-` (see - * `buildPersonalRepoUrl`). We check existence with `GET /repos/...` - * first so a pre-existing repo is a clean no-op; only when the repo - * is genuinely absent (404) do we attempt creation. Avoids the 422 - * ambiguity where the GitHub API returns the same status for "name - * taken" and "name invalid". + * Resolution order: + * 1. If `recoupable/` already exists → return its URL + * (clean idempotent case — most calls). + * 2. Look for a legacy `recoupable/<*>-` repo via + * GitHub search. If found, rename it to `` so the + * account's git history follows them onto the new convention. + * GitHub auto-redirects the old URL for clones + REST, so any + * `sessions.clone_url` rows that still reference the old name + * keep working. + * 3. Otherwise, create a fresh `recoupable/` with + * `auto_init: true` so the sandbox has a `main` branch to clone. * - * Returns `null` when the service token is missing, the existence - * check fails for non-404 reasons, or creation fails — all treated - * as fatal by callers. The caller is responsible for surfacing a - * user-visible error (`createSessionHandler` returns a 502). + * Returns `null` only when the service token is missing or repo + * creation outright fails — the caller surfaces that as a 502. * - * Ported from open-agents - * `apps/web/lib/recoupable/ensure-personal-repo.ts` — keep behavior - * in lockstep so chat.recoupable.com and sandbox.recoupable.com - * converge on the same repo for the same account. + * The legacy-rename branch never blocks provisioning: if the search + * API throws or returns ambiguous results, we fall through to step 3 + * and create a fresh repo. Losing history for an edge-case account is + * preferable to blocking their session on a flaky GitHub search. */ export async function ensurePersonalRepo(params: { - accountName: string; accountId: string; }): Promise { const token = getServiceGithubToken(); @@ -42,10 +47,7 @@ export async function ensurePersonalRepo(params: { return null; } - const { repo: repoName } = buildPersonalRepoIdentifier({ - accountName: params.accountName, - accountId: params.accountId, - }); + const { repo: repoName } = buildPersonalRepoIdentifier(params); const existing = await repositoryExists({ owner: RECOUPABLE_GITHUB_OWNER, @@ -67,10 +69,42 @@ export async function ensurePersonalRepo(params: { }; } + // Try to find and rename a legacy `-` repo so the + // account's existing code follows them onto the new naming + // convention without a manual migration step. + const legacyName = await findLegacyAccountRepo({ + owner: RECOUPABLE_GITHUB_OWNER, + accountId: params.accountId, + token, + }); + + if (legacyName) { + const renamed = await renameRepository({ + owner: RECOUPABLE_GITHUB_OWNER, + repo: legacyName, + newName: repoName, + token, + }); + if (renamed.success) { + console.log( + `[ensurePersonalRepo] renamed legacy ${RECOUPABLE_GITHUB_OWNER}/${legacyName} → ${repoName}`, + ); + return { + cloneUrl: buildPersonalRepoUrl(params), + repoUrl: `https://github.com/${RECOUPABLE_GITHUB_OWNER}/${repoName}`, + owner: RECOUPABLE_GITHUB_OWNER, + repoName, + }; + } + console.error( + `[ensurePersonalRepo] legacy rename failed (${renamed.error ?? "unknown"}); falling through to create`, + ); + } + const created = await createRepository({ owner: RECOUPABLE_GITHUB_OWNER, name: repoName, - description: `Personal Recoupable workspace for account ${params.accountId}`, + description: `Recoupable workspace for account ${params.accountId}`, isPrivate: true, token, }); diff --git a/lib/recoupable/extractOrgId.ts b/lib/recoupable/extractOrgId.ts index ac30985c5..174fb63c9 100644 --- a/lib/recoupable/extractOrgId.ts +++ b/lib/recoupable/extractOrgId.ts @@ -1,6 +1,15 @@ import { extractOrgRepoName } from "@/lib/recoupable/extractOrgRepoName"; -const UUID_TAIL_PATTERN = /-([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})$/i; +const UUID_RAW = "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"; +/** + * Match the trailing UUID either bare (new naming: ``) or after + * a slug + dash (legacy: `-` / `org--`). Both + * shapes coexist until the migration script renames every legacy repo + * — and even after, `sessions.clone_url` rows from before the rename + * still resolve via GitHub's redirect, so this parser keeps working + * for old rows forever. + */ +const UUID_TAIL_PATTERN = new RegExp(`(?:^|-)(${UUID_RAW})$`, "i"); /** * Extracts the organization UUID from a Recoupable org clone URL or diff --git a/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts b/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts index 6ceaad153..3f199d77f 100644 --- a/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts +++ b/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts @@ -1,18 +1,15 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { resolveSessionCloneUrl } from "@/lib/sessions/resolveSessionCloneUrl"; -import { getAccountWithDetails } from "@/lib/supabase/accounts/getAccountWithDetails"; import { ensurePersonalRepo } from "@/lib/recoupable/ensurePersonalRepo"; import type { AuthContext } from "@/lib/auth/validateAuthContext"; -vi.mock("@/lib/supabase/accounts/getAccountWithDetails", () => ({ - getAccountWithDetails: vi.fn(), -})); vi.mock("@/lib/recoupable/ensurePersonalRepo", () => ({ ensurePersonalRepo: vi.fn(), })); +const accountId = "fb678396-a68f-4294-ae50-b8cacf9ce77b"; const baseAuth: AuthContext = { - accountId: "acc-1", + accountId, orgId: null, authToken: "tok", }; @@ -32,7 +29,6 @@ describe("resolveSessionCloneUrl", () => { ok: true, cloneUrl: "https://github.com/recoupable/org-foo-id-1", }); - expect(getAccountWithDetails).not.toHaveBeenCalled(); expect(ensurePersonalRepo).not.toHaveBeenCalled(); }); @@ -47,15 +43,11 @@ describe("resolveSessionCloneUrl", () => { }); it("provisions a personal repo when no body cloneUrl and no org", async () => { - vi.mocked(getAccountWithDetails).mockResolvedValue({ - name: "Sweetman", - email: "sweetman@example.com", - } as never); vi.mocked(ensurePersonalRepo).mockResolvedValue({ - cloneUrl: "https://github.com/recoupable/sweetman-acc-1", - repoUrl: "https://github.com/recoupable/sweetman-acc-1", + cloneUrl: `https://github.com/recoupable/${accountId}`, + repoUrl: `https://github.com/recoupable/${accountId}`, owner: "recoupable", - repoName: "sweetman-acc-1", + repoName: accountId, }); const result = await resolveSessionCloneUrl({ @@ -65,68 +57,12 @@ describe("resolveSessionCloneUrl", () => { expect(result).toEqual({ ok: true, - cloneUrl: "https://github.com/recoupable/sweetman-acc-1", - }); - expect(ensurePersonalRepo).toHaveBeenCalledWith({ - accountName: "Sweetman", - accountId: "acc-1", + cloneUrl: `https://github.com/recoupable/${accountId}`, }); - }); - - it("falls back to email local-part when name is missing", async () => { - vi.mocked(getAccountWithDetails).mockResolvedValue({ - name: null, - email: "sweetmantech@gmail.com", - } as never); - vi.mocked(ensurePersonalRepo).mockResolvedValue({ - cloneUrl: "url", - repoUrl: "url", - owner: "recoupable", - repoName: "sweetmantech-acc-1", - }); - - await resolveSessionCloneUrl({ - bodyCloneUrl: undefined, - auth: baseAuth, - }); - - expect(ensurePersonalRepo).toHaveBeenCalledWith({ - accountName: "sweetmantech", - accountId: "acc-1", - }); - }); - - it("returns an error when account has no name and no email", async () => { - vi.mocked(getAccountWithDetails).mockResolvedValue({ - name: null, - email: null, - } as never); - - const result = await resolveSessionCloneUrl({ - bodyCloneUrl: undefined, - auth: baseAuth, - }); - - expect(result.ok).toBe(false); - expect(ensurePersonalRepo).not.toHaveBeenCalled(); - }); - - it("returns an error when the account row is missing", async () => { - vi.mocked(getAccountWithDetails).mockResolvedValue(null); - - const result = await resolveSessionCloneUrl({ - bodyCloneUrl: undefined, - auth: baseAuth, - }); - - expect(result.ok).toBe(false); - expect(ensurePersonalRepo).not.toHaveBeenCalled(); + expect(ensurePersonalRepo).toHaveBeenCalledWith({ accountId }); }); it("returns an error when ensurePersonalRepo fails", async () => { - vi.mocked(getAccountWithDetails).mockResolvedValue({ - name: "Sweetman", - } as never); vi.mocked(ensurePersonalRepo).mockResolvedValue(null); const result = await resolveSessionCloneUrl({ @@ -135,5 +71,6 @@ describe("resolveSessionCloneUrl", () => { }); expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); }); }); diff --git a/lib/sessions/resolveSessionCloneUrl.ts b/lib/sessions/resolveSessionCloneUrl.ts index 1beaac241..ce0d9c4ca 100644 --- a/lib/sessions/resolveSessionCloneUrl.ts +++ b/lib/sessions/resolveSessionCloneUrl.ts @@ -1,4 +1,3 @@ -import { getAccountWithDetails } from "@/lib/supabase/accounts/getAccountWithDetails"; import { ensurePersonalRepo } from "@/lib/recoupable/ensurePersonalRepo"; import type { AuthContext } from "@/lib/auth/validateAuthContext"; @@ -15,23 +14,22 @@ export interface ResolveSessionCloneUrlResult { } /** - * Determines the final `clone_url` for a new session row. + * Determine the final `clone_url` for a new session row. * * 1. If the body explicitly provided `cloneUrl`, use it. Both chat * (active-org case) and sandbox.recoupable.com construct the org * URL client-side and send it through. * 2. If the body omitted `cloneUrl` AND the caller has no `orgId` * bound to their auth context, treat this as a personal session - * and call `ensurePersonalRepo` so a brand-new user gets a fresh - * `recoupable/-` repo provisioned before - * `POST /api/sandbox` tries to clone it (Path C cutover for - * chat.recoupable.com — recoupable/chat#1747). + * and call `ensurePersonalRepo` so the account's + * `recoupable/` repo exists (with a legacy-rename + * step for accounts that had `-` workspaces + * under the old naming convention) before `POST /api/sandbox` + * tries to clone it. * 3. Otherwise (no `cloneUrl`, but an org is bound), return `null` - * so the session row stores no `clone_url` and the sandbox falls - * back to whatever existing behavior handles that case. This - * branch is intentionally narrow — org-repo URL derivation - * stays a client-side concern for now to keep this PR focused - * on personal-repo provisioning. + * so the session row stores no `clone_url`. Org-repo URL + * derivation stays a client-side concern for now to keep this PR + * focused on personal-repo provisioning. */ export async function resolveSessionCloneUrl(params: { bodyCloneUrl: string | undefined; @@ -47,28 +45,7 @@ export async function resolveSessionCloneUrl(params: { return { ok: true, cloneUrl: null }; } - const account = await getAccountWithDetails(auth.accountId); - if (!account) { - return { - ok: false, - cloneUrl: null, - error: "Account not found for personal-session provisioning", - }; - } - - const accountName = resolvePersonalRepoAccountName(account); - if (!accountName) { - return { - ok: false, - cloneUrl: null, - error: "Account has no name or email to derive a personal repo identifier from", - }; - } - - const ensured = await ensurePersonalRepo({ - accountName, - accountId: auth.accountId, - }); + const ensured = await ensurePersonalRepo({ accountId: auth.accountId }); if (!ensured) { return { @@ -80,28 +57,3 @@ export async function resolveSessionCloneUrl(params: { return { ok: true, cloneUrl: ensured.cloneUrl }; } - -interface PersonalRepoAccountFields { - name?: string | null; - email?: string | null; -} - -/** - * Pick the most stable string to kebab into the personal repo slug. - * Prefers `account_info.name`; falls back to the local-part of the - * account's email (matches open-agents `fetchOrCreateAccount` - * fallback). Returns `null` when neither is usable so callers can - * surface a clean error rather than building a slug like `-`. - */ -function resolvePersonalRepoAccountName(account: PersonalRepoAccountFields): string | null { - const trimmedName = account.name?.trim(); - if (trimmedName) return trimmedName; - - const trimmedEmail = account.email?.trim(); - if (trimmedEmail) { - const localPart = trimmedEmail.split("@")[0]; - if (localPart && localPart.length > 0) return localPart; - } - - return null; -} diff --git a/lib/string/__tests__/toKebabCase.test.ts b/lib/string/__tests__/toKebabCase.test.ts deleted file mode 100644 index 3329f0e47..000000000 --- a/lib/string/__tests__/toKebabCase.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { toKebabCase } from "@/lib/string/toKebabCase"; - -describe("toKebabCase", () => { - it("lowercases input", () => { - expect(toKebabCase("Hello")).toBe("hello"); - }); - - it("replaces dots with single hyphen — guards the sweetman.eth -> sweetman-eth bug from chat", () => { - expect(toKebabCase("sweetman.eth")).toBe("sweetman-eth"); - }); - - it("collapses runs of non-alphanumerics into a single hyphen", () => { - expect(toKebabCase("Foo Bar___Baz")).toBe("foo-bar-baz"); - }); - - it("trims leading and trailing hyphens", () => { - expect(toKebabCase("---foo bar---")).toBe("foo-bar"); - }); - - it("preserves digits", () => { - expect(toKebabCase("Artist 42")).toBe("artist-42"); - }); - - it("returns empty string for all-symbol input", () => { - expect(toKebabCase("@@@")).toBe(""); - }); -}); diff --git a/lib/string/toKebabCase.ts b/lib/string/toKebabCase.ts deleted file mode 100644 index 18dd4383b..000000000 --- a/lib/string/toKebabCase.ts +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Converts an arbitrary string into kebab-case slug form. - * - lowercases - * - collapses any non-alphanumeric runs to a single `-` - * - trims leading / trailing `-` - * - * Used to build per-org and per-account repo identifiers from - * human-entered names. Ported from open-agents - * `apps/web/lib/string/to-kebab-case.ts` so chat.recoupable.com, - * sandbox.recoupable.com, and recoup-api all kebab the same way. - */ -export function toKebabCase(input: string): string { - return input - .toLowerCase() - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, ""); -} diff --git a/scripts/migrate-workspace-repo-names.ts b/scripts/migrate-workspace-repo-names.ts new file mode 100644 index 000000000..f9e417ad6 --- /dev/null +++ b/scripts/migrate-workspace-repo-names.ts @@ -0,0 +1,143 @@ +/** + * One-time migration: rename every `recoupable/*-` workspace + * repository to the canonical `recoupable/` convention. + * + * - Scans all repos under the `recoupable` GitHub org, paginated. + * - For each repo whose name matches `^.+-$`, extracts the + * trailing UUID and renames the repo to just ``. + * - Skips when the target name already exists (idempotent re-run + * safe — first invocation handles the rename, subsequent ones are + * no-ops). + * - Defaults to dry-run; pass `--apply` to actually perform renames. + * + * GitHub auto-redirects the old name for both git clones and the REST + * API after a rename, so `sessions.clone_url` rows that still + * reference the old name keep working without any DB backfill. + * + * Run via: + * pnpm dotenv -e .env.local -- pnpm tsx scripts/migrate-workspace-repo-names.ts + * pnpm dotenv -e .env.local -- pnpm tsx scripts/migrate-workspace-repo-names.ts --apply + * + * Requires `GITHUB_TOKEN` with `admin:repo` scope on the `recoupable` + * org in the environment (or `.env.local`). + */ + +import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; +import { renameRepository } from "@/lib/github/renameRepository"; + +const LEGACY_PATTERN = + /^(?.+)-(?[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})$/i; + +interface OrgRepo { + name: string; +} + +async function listAllOrgRepos(token: string): Promise { + const repos: OrgRepo[] = []; + let page = 1; + while (true) { + const response = await fetch( + `https://api.github.com/orgs/${RECOUPABLE_GITHUB_OWNER}/repos?per_page=100&page=${page}`, + { + method: "GET", + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "X-GitHub-Api-Version": "2022-11-28", + }, + }, + ); + + if (!response.ok) { + const body = await response.text().catch(() => ""); + throw new Error(`Failed to list org repos at page ${page}: ${response.status} ${body}`); + } + + const batch = (await response.json()) as OrgRepo[]; + if (batch.length === 0) break; + repos.push(...batch); + if (batch.length < 100) break; + page += 1; + } + return repos; +} + +async function main(): Promise { + const apply = process.argv.includes("--apply"); + const token = process.env.GITHUB_TOKEN; + if (!token) { + console.error("GITHUB_TOKEN must be set (admin:repo on recoupable org)."); + process.exit(1); + } + + console.log(`[migrate] mode=${apply ? "APPLY" : "dry-run"} owner=${RECOUPABLE_GITHUB_OWNER}`); + + const repos = await listAllOrgRepos(token); + console.log(`[migrate] discovered ${repos.length} repos`); + + const existingNames = new Set(repos.map(r => r.name)); + + type Action = + | { type: "rename"; from: string; to: string } + | { type: "skip-target-exists"; from: string; to: string } + | { type: "skip-not-workspace"; name: string }; + const actions: Action[] = []; + + for (const { name } of repos) { + const match = name.match(LEGACY_PATTERN); + if (!match?.groups) { + actions.push({ type: "skip-not-workspace", name }); + continue; + } + const uuid = match.groups.uuid.toLowerCase(); + if (existingNames.has(uuid)) { + actions.push({ type: "skip-target-exists", from: name, to: uuid }); + continue; + } + actions.push({ type: "rename", from: name, to: uuid }); + } + + const toRename = actions.filter( + (a): a is { type: "rename"; from: string; to: string } => a.type === "rename", + ); + const skipTarget = actions.filter(a => a.type === "skip-target-exists"); + const skipNotWorkspace = actions.filter(a => a.type === "skip-not-workspace"); + + console.log( + `[migrate] plan: rename=${toRename.length}, skip-target-exists=${skipTarget.length}, skip-not-workspace=${skipNotWorkspace.length}`, + ); + + for (const a of toRename) { + console.log(` rename: ${a.from} -> ${a.to}`); + } + + if (!apply) { + console.log("[migrate] dry-run — no changes made. Re-run with --apply."); + return; + } + + let succeeded = 0; + let failed = 0; + for (const a of toRename) { + const result = await renameRepository({ + owner: RECOUPABLE_GITHUB_OWNER, + repo: a.from, + newName: a.to, + token, + }); + if (result.success) { + succeeded += 1; + console.log(` ok: ${a.from} -> ${a.to}`); + } else { + failed += 1; + console.error(` fail: ${a.from} -> ${a.to} (${result.error ?? "?"})`); + } + } + + console.log( + `[migrate] done. renamed=${succeeded} failed=${failed} skipped=${skipTarget.length + skipNotWorkspace.length}`, + ); + if (failed > 0) process.exit(1); +} + +void main(); From 1688646d0ea8f614f3b08bcd8d77610005d233c0 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 11:38:57 -0500 Subject: [PATCH 04/12] review(PR #618): drop owner/token/description/isPrivate params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Sweet's review comments on createRepository.ts, hard-code the constants that only have one sensible value: - owner = "recoupable" (RECOUPABLE_GITHUB_OWNER) - token = read from env via getServiceGithubToken (no param plumbing) - description = dropped (GitHub doesn't render anything meaningful) - private = false (workspace repos are public) Applied the same simplification consistently to the other three new github helpers so the surface stays symmetric: - repositoryExists, renameRepository, findLegacyAccountRepo all drop owner + token params; each reads the token via getServiceGithubToken and short-circuits when missing. - ensurePersonalRepo no longer threads token/owner through; its call sites simplified to e.g. createRepository({ name }) and repositoryExists({ repo }). - migrate-workspace-repo-names.ts script likewise calls renameRepository({ repo, newName }) — token plumbing removed. Tests updated to mock getServiceGithubToken instead of passing a fake token through the call. Co-Authored-By: Claude Opus 4.7 --- lib/github/__tests__/createRepository.test.ts | 70 +++++++++---------- .../__tests__/findLegacyAccountRepo.test.ts | 50 ++++++------- lib/github/__tests__/renameRepository.test.ts | 34 +++++---- lib/github/__tests__/repositoryExists.test.ts | 52 ++++++-------- lib/github/createRepository.ts | 58 ++++++++------- lib/github/findLegacyAccountRepo.ts | 25 ++++--- lib/github/renameRepository.ts | 46 +++++++----- lib/github/repositoryExists.ts | 58 ++++++++------- .../__tests__/ensurePersonalRepo.test.ts | 33 +-------- lib/recoupable/ensurePersonalRepo.ts | 30 ++------ scripts/migrate-workspace-repo-names.ts | 7 +- 11 files changed, 217 insertions(+), 246 deletions(-) diff --git a/lib/github/__tests__/createRepository.test.ts b/lib/github/__tests__/createRepository.test.ts index 1b24c2b6b..bb55d4a4a 100644 --- a/lib/github/__tests__/createRepository.test.ts +++ b/lib/github/__tests__/createRepository.test.ts @@ -1,58 +1,66 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { createRepository } from "@/lib/github/createRepository"; +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; + +vi.mock("@/lib/github/getServiceGithubToken", () => ({ + getServiceGithubToken: vi.fn(() => "tok"), +})); describe("createRepository", () => { beforeEach(() => { - vi.restoreAllMocks(); + vi.clearAllMocks(); + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + }); + + it("returns failure when GITHUB_TOKEN is missing", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue(undefined); + const fetchSpy = vi.spyOn(globalThis, "fetch"); + + const result = await createRepository({ name: "id-1" }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/GITHUB_TOKEN/i); + expect(fetchSpy).not.toHaveBeenCalled(); }); it("rejects invalid names without hitting the network", async () => { const fetchSpy = vi.spyOn(globalThis, "fetch"); - const result = await createRepository({ - owner: "recoupable", - name: "bad name with spaces", - token: "tok", - }); + const result = await createRepository({ name: "bad name with spaces" }); expect(result.success).toBe(false); expect(result.error).toMatch(/invalid/i); expect(fetchSpy).not.toHaveBeenCalled(); }); - it("creates an org repo on 201, returning clone url + html url", async () => { + it("POSTs to /orgs/recoupable/repos with hard-coded private=false + auto_init=true", async () => { const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( new Response( JSON.stringify({ - html_url: "https://github.com/recoupable/sweetman-id-1", - clone_url: "https://github.com/recoupable/sweetman-id-1.git", + html_url: "https://github.com/recoupable/id-1", + clone_url: "https://github.com/recoupable/id-1.git", owner: { login: "recoupable" }, - name: "sweetman-id-1", + name: "id-1", }), { status: 201, headers: { "content-type": "application/json" } }, ), ); - const result = await createRepository({ - owner: "recoupable", - name: "sweetman-id-1", - token: "tok", - }); + const result = await createRepository({ name: "id-1" }); expect(result).toEqual({ success: true, - repoUrl: "https://github.com/recoupable/sweetman-id-1", - cloneUrl: "https://github.com/recoupable/sweetman-id-1.git", + repoUrl: "https://github.com/recoupable/id-1", + cloneUrl: "https://github.com/recoupable/id-1.git", owner: "recoupable", - repoName: "sweetman-id-1", + repoName: "id-1", }); const [url, init] = fetchSpy.mock.calls[0] as [string, RequestInit]; expect(url).toBe("https://api.github.com/orgs/recoupable/repos"); expect(init.method).toBe("POST"); - const body = JSON.parse(init.body as string); - expect(body).toMatchObject({ - name: "sweetman-id-1", - private: true, + expect(JSON.parse(init.body as string)).toEqual({ + name: "id-1", + private: false, auto_init: true, }); }); @@ -60,11 +68,7 @@ describe("createRepository", () => { it("returns name-conflict error on 422", async () => { vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 422 })); - const result = await createRepository({ - owner: "recoupable", - name: "sweetman-id-1", - token: "tok", - }); + const result = await createRepository({ name: "id-1" }); expect(result.success).toBe(false); expect(result.error).toMatch(/already exists/i); @@ -73,11 +77,7 @@ describe("createRepository", () => { it("returns permission-denied error on 403", async () => { vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 403 })); - const result = await createRepository({ - owner: "recoupable", - name: "sweetman-id-1", - token: "tok", - }); + const result = await createRepository({ name: "id-1" }); expect(result.success).toBe(false); expect(result.error).toMatch(/permission/i); @@ -86,11 +86,7 @@ describe("createRepository", () => { it("returns network-error on fetch rejection", async () => { vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("ECONNRESET")); - const result = await createRepository({ - owner: "recoupable", - name: "sweetman-id-1", - token: "tok", - }); + const result = await createRepository({ name: "id-1" }); expect(result.success).toBe(false); expect(result.error).toMatch(/network/i); diff --git a/lib/github/__tests__/findLegacyAccountRepo.test.ts b/lib/github/__tests__/findLegacyAccountRepo.test.ts index ec44ac9c4..1483568ef 100644 --- a/lib/github/__tests__/findLegacyAccountRepo.test.ts +++ b/lib/github/__tests__/findLegacyAccountRepo.test.ts @@ -1,11 +1,27 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { findLegacyAccountRepo } from "@/lib/github/findLegacyAccountRepo"; +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; + +vi.mock("@/lib/github/getServiceGithubToken", () => ({ + getServiceGithubToken: vi.fn(() => "tok"), +})); const accountId = "fb678396-a68f-4294-ae50-b8cacf9ce77b"; describe("findLegacyAccountRepo", () => { beforeEach(() => { - vi.restoreAllMocks(); + vi.clearAllMocks(); + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + }); + + it("returns undefined when GITHUB_TOKEN is missing", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue(undefined); + const fetchSpy = vi.spyOn(globalThis, "fetch"); + + const result = await findLegacyAccountRepo({ accountId }); + + expect(result).toBeUndefined(); + expect(fetchSpy).not.toHaveBeenCalled(); }); it("returns the legacy name when a *- match exists", async () => { @@ -18,11 +34,7 @@ describe("findLegacyAccountRepo", () => { ), ); - const result = await findLegacyAccountRepo({ - owner: "recoupable", - accountId, - token: "tok", - }); + const result = await findLegacyAccountRepo({ accountId }); expect(result).toBe(`sweetman-${accountId}`); }); @@ -35,11 +47,7 @@ describe("findLegacyAccountRepo", () => { }), ); - const result = await findLegacyAccountRepo({ - owner: "recoupable", - accountId, - token: "tok", - }); + const result = await findLegacyAccountRepo({ accountId }); expect(result).toBeNull(); }); @@ -52,11 +60,7 @@ describe("findLegacyAccountRepo", () => { }), ); - const result = await findLegacyAccountRepo({ - owner: "recoupable", - accountId, - token: "tok", - }); + const result = await findLegacyAccountRepo({ accountId }); expect(result).toBeNull(); }); @@ -64,16 +68,12 @@ describe("findLegacyAccountRepo", () => { it("returns undefined on non-OK response (caller treats as miss)", async () => { vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 503 })); - const result = await findLegacyAccountRepo({ - owner: "recoupable", - accountId, - token: "tok", - }); + const result = await findLegacyAccountRepo({ accountId }); expect(result).toBeUndefined(); }); - it("encodes the search query as +in:name+org:", async () => { + it("encodes the search query as +in:name+org:recoupable", async () => { const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( new Response(JSON.stringify({ items: [] }), { status: 200, @@ -81,11 +81,7 @@ describe("findLegacyAccountRepo", () => { }), ); - await findLegacyAccountRepo({ - owner: "recoupable", - accountId: "id-1", - token: "tok", - }); + await findLegacyAccountRepo({ accountId: "id-1" }); const [url] = fetchSpy.mock.calls[0] as [string]; expect(url).toContain("/search/repositories?q="); diff --git a/lib/github/__tests__/renameRepository.test.ts b/lib/github/__tests__/renameRepository.test.ts index 2af9b5689..22421c743 100644 --- a/lib/github/__tests__/renameRepository.test.ts +++ b/lib/github/__tests__/renameRepository.test.ts @@ -1,26 +1,44 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { renameRepository } from "@/lib/github/renameRepository"; +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; + +vi.mock("@/lib/github/getServiceGithubToken", () => ({ + getServiceGithubToken: vi.fn(() => "tok"), +})); describe("renameRepository", () => { beforeEach(() => { - vi.restoreAllMocks(); + vi.clearAllMocks(); + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + }); + + it("returns failure when GITHUB_TOKEN is missing", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue(undefined); + const fetchSpy = vi.spyOn(globalThis, "fetch"); + + const result = await renameRepository({ + repo: "sweetman-id-1", + newName: "id-1", + }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/GITHUB_TOKEN/i); + expect(fetchSpy).not.toHaveBeenCalled(); }); it("rejects invalid newName without hitting the network", async () => { const fetchSpy = vi.spyOn(globalThis, "fetch"); const result = await renameRepository({ - owner: "recoupable", repo: "sweetman-id-1", newName: "bad name", - token: "tok", }); expect(result.success).toBe(false); expect(fetchSpy).not.toHaveBeenCalled(); }); - it("PATCHes /repos/{owner}/{repo} with {name} and returns the new URLs on 200", async () => { + it("PATCHes /repos/recoupable/ with {name} and returns the new URLs on 200", async () => { const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( new Response( JSON.stringify({ @@ -32,10 +50,8 @@ describe("renameRepository", () => { ); const result = await renameRepository({ - owner: "recoupable", repo: "sweetman-id-1", newName: "id-1", - token: "tok", }); expect(result).toEqual({ @@ -53,10 +69,8 @@ describe("renameRepository", () => { vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 422 })); const result = await renameRepository({ - owner: "recoupable", repo: "sweetman-id-1", newName: "id-1", - token: "tok", }); expect(result.success).toBe(false); @@ -67,10 +81,8 @@ describe("renameRepository", () => { vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 404 })); const result = await renameRepository({ - owner: "recoupable", repo: "sweetman-id-1", newName: "id-1", - token: "tok", }); expect(result.success).toBe(false); @@ -81,10 +93,8 @@ describe("renameRepository", () => { vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("ECONNRESET")); const result = await renameRepository({ - owner: "recoupable", repo: "sweetman-id-1", newName: "id-1", - token: "tok", }); expect(result.success).toBe(false); diff --git a/lib/github/__tests__/repositoryExists.test.ts b/lib/github/__tests__/repositoryExists.test.ts index 81e5a34c8..66853591a 100644 --- a/lib/github/__tests__/repositoryExists.test.ts +++ b/lib/github/__tests__/repositoryExists.test.ts @@ -1,25 +1,37 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { repositoryExists } from "@/lib/github/repositoryExists"; +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; + +vi.mock("@/lib/github/getServiceGithubToken", () => ({ + getServiceGithubToken: vi.fn(() => "tok"), +})); describe("repositoryExists", () => { beforeEach(() => { - vi.restoreAllMocks(); + vi.clearAllMocks(); + vi.mocked(getServiceGithubToken).mockReturnValue("tok"); + }); + + it("returns null when GITHUB_TOKEN is missing", async () => { + vi.mocked(getServiceGithubToken).mockReturnValue(undefined); + const fetchSpy = vi.spyOn(globalThis, "fetch"); + + const result = await repositoryExists({ repo: "id-1" }); + + expect(result).toBeNull(); + expect(fetchSpy).not.toHaveBeenCalled(); }); - it("returns true on 200", async () => { + it("returns true on 200 and calls GET /repos/recoupable/", async () => { const fetchSpy = vi .spyOn(globalThis, "fetch") .mockResolvedValue(new Response(null, { status: 200 })); - const result = await repositoryExists({ - owner: "recoupable", - repo: "sweetman-id-1", - token: "tok", - }); + const result = await repositoryExists({ repo: "id-1" }); expect(result).toBe(true); expect(fetchSpy).toHaveBeenCalledWith( - "https://api.github.com/repos/recoupable/sweetman-id-1", + "https://api.github.com/repos/recoupable/id-1", expect.objectContaining({ method: "GET", headers: expect.objectContaining({ @@ -32,36 +44,18 @@ describe("repositoryExists", () => { it("returns false on 404", async () => { vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 404 })); - expect( - await repositoryExists({ - owner: "recoupable", - repo: "missing", - token: "tok", - }), - ).toBe(false); + expect(await repositoryExists({ repo: "missing" })).toBe(false); }); it("returns null on other statuses (auth, rate limit, etc.)", async () => { vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 403 })); - expect( - await repositoryExists({ - owner: "recoupable", - repo: "rate-limited", - token: "tok", - }), - ).toBeNull(); + expect(await repositoryExists({ repo: "rate-limited" })).toBeNull(); }); it("returns null on network failure", async () => { vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("ECONNRESET")); - expect( - await repositoryExists({ - owner: "recoupable", - repo: "anything", - token: "tok", - }), - ).toBeNull(); + expect(await repositoryExists({ repo: "anything" })).toBeNull(); }); }); diff --git a/lib/github/createRepository.ts b/lib/github/createRepository.ts index fa3b69cec..8bfeba459 100644 --- a/lib/github/createRepository.ts +++ b/lib/github/createRepository.ts @@ -1,9 +1,13 @@ +import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; +import { getServiceGithubToken } from "./getServiceGithubToken"; + export interface CreateRepositoryResult { success: boolean; - /** GitHub UI URL (`html_url`) — `https://github.com//`. */ + /** GitHub UI URL (`html_url`). */ repoUrl?: string; - /** Git clone URL (`clone_url`) — same shape, used by sandboxes to `git clone`. */ + /** Git clone URL (`clone_url`). */ cloneUrl?: string; + /** Owner login (always `recoupable`). */ owner?: string; repoName?: string; /** Human-readable error message; only set when `success` is false. */ @@ -11,30 +15,33 @@ export interface CreateRepositoryResult { } /** - * Create a GitHub repository under a Recoupable-owned organization. + * Create a workspace repository under the Recoupable GitHub org. * - * Ported from open-agents `apps/web/lib/github/client.ts#createRepository` - * with two intentional reductions: - * - Plain `fetch` (no Octokit) to match recoup-api's existing - * `lib/github/*` style. - * - Org-only creation: open-agents allowed a `User` `accountType` - * fallback, but Recoupable workspace repos are always created - * under the `recoupable` org (see `RECOUPABLE_GITHUB_OWNER`), so - * the `createForAuthenticatedUser` branch is unreachable in this - * codebase and has been dropped. + * Hard-coded conventions (per PR #618 review — KISS / YAGNI): + * - owner = `recoupable` (no other owner makes sense; see + * `RECOUPABLE_GITHUB_OWNER`). + * - private = false (workspace repos are public so future surfaces + * can clone without per-caller auth). + * - description = none (GitHub doesn't render anything meaningful + * for these per-account repos). + * - token = read once from `GITHUB_TOKEN` via + * `getServiceGithubToken` (single source of truth — callers no + * longer thread the token through). * * `auto_init: true` so the repo has an initial `main` branch the - * sandbox can `git clone`. Without it, a clone of a 0-commit repo - * fails. + * sandbox can `git clone`. Without it, cloning a 0-commit repo fails. + * + * Uses plain `fetch` to match recoup-api's existing `lib/github/*` + * style (no Octokit dependency). */ -export async function createRepository(params: { - owner: string; - name: string; - description?: string; - isPrivate?: boolean; - token: string; -}): Promise { - const { owner, name, description = "", isPrivate = true, token } = params; +export async function createRepository(params: { name: string }): Promise { + const { name } = params; + + const token = getServiceGithubToken(); + if (!token) { + console.error("[createRepository] GITHUB_TOKEN missing"); + return { success: false, error: "GITHUB_TOKEN missing" }; + } if (!/^[\w.-]+$/.test(name)) { return { @@ -45,7 +52,7 @@ export async function createRepository(params: { } try { - const response = await fetch(`https://api.github.com/orgs/${owner}/repos`, { + const response = await fetch(`https://api.github.com/orgs/${RECOUPABLE_GITHUB_OWNER}/repos`, { method: "POST", headers: { Accept: "application/vnd.github+json", @@ -55,8 +62,7 @@ export async function createRepository(params: { }, body: JSON.stringify({ name, - description, - private: isPrivate, + private: false, auto_init: true, }), }); @@ -94,7 +100,7 @@ export async function createRepository(params: { body = ""; } console.error( - `[createRepository] unexpected status ${response.status} for ${owner}/${name}: ${body}`, + `[createRepository] unexpected status ${response.status} for ${RECOUPABLE_GITHUB_OWNER}/${name}: ${body}`, ); return { success: false, error: `GitHub returned ${response.status}` }; } catch (error) { diff --git a/lib/github/findLegacyAccountRepo.ts b/lib/github/findLegacyAccountRepo.ts index 5cd0858d6..4c8b03904 100644 --- a/lib/github/findLegacyAccountRepo.ts +++ b/lib/github/findLegacyAccountRepo.ts @@ -1,12 +1,14 @@ +import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; +import { getServiceGithubToken } from "./getServiceGithubToken"; + const LEGACY_SUFFIX_PATTERN = /^.+-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; /** * Search the `recoupable` org for a legacy-named workspace repo whose * name ends with `-` — the old `-` - * convention. Returns the legacy repo name (the one before the - * trailing `-` suffix is what made the name unique) so - * `ensurePersonalRepo` can rename it to the new bare-`` - * convention without losing the user's git history. + * convention. Returns the legacy repo name so `ensurePersonalRepo` + * can rename it to the new bare-`` convention without + * losing the user's git history. * * Returns: * - the legacy repo name (e.g. `sweetman-fb678396-...`) on hit @@ -18,16 +20,23 @@ const LEGACY_SUFFIX_PATTERN = /^.+-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{ * Uses GitHub's `GET /search/repositories` endpoint with * `+in:name+org:recoupable` so we don't have to paginate * the entire org. + * + * Owner is hard-coded to `recoupable` and the GitHub token is read + * from the environment (per PR #618 review — single source of truth). */ export async function findLegacyAccountRepo(params: { - owner: string; accountId: string; - token: string; }): Promise { - const { owner, accountId, token } = params; + const { accountId } = params; + + const token = getServiceGithubToken(); + if (!token) { + console.error("[findLegacyAccountRepo] GITHUB_TOKEN missing"); + return undefined; + } try { - const query = encodeURIComponent(`${accountId} in:name org:${owner}`); + const query = encodeURIComponent(`${accountId} in:name org:${RECOUPABLE_GITHUB_OWNER}`); const response = await fetch( `https://api.github.com/search/repositories?q=${query}&per_page=10`, { diff --git a/lib/github/renameRepository.ts b/lib/github/renameRepository.ts index 979f8ee7f..b015f87de 100644 --- a/lib/github/renameRepository.ts +++ b/lib/github/renameRepository.ts @@ -1,3 +1,6 @@ +import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; +import { getServiceGithubToken } from "./getServiceGithubToken"; + export interface RenameRepositoryResult { success: boolean; /** New `clone_url` returned by GitHub on success. */ @@ -8,24 +11,28 @@ export interface RenameRepositoryResult { } /** - * Rename a GitHub repository via `PATCH /repos/{owner}/{repo}`. + * Rename a Recoupable workspace repository via + * `PATCH /repos/{owner}/{repo}`. * * GitHub automatically issues HTTP redirects from the old name to the * new name for both git operations and the REST API, so existing * `sessions.clone_url` rows that still reference the old name keep - * resolving after the rename — no DB backfill required for clone_url - * (the migration script only renames repos, not stored URLs). + * resolving — no DB backfill required. * - * Uses plain `fetch` to match recoup-api's existing `lib/github/*` - * style (no Octokit dependency). + * Owner is hard-coded to `recoupable` and the GitHub token is read + * from the environment (per PR #618 review — single source of truth). */ export async function renameRepository(params: { - owner: string; repo: string; newName: string; - token: string; }): Promise { - const { owner, repo, newName, token } = params; + const { repo, newName } = params; + + const token = getServiceGithubToken(); + if (!token) { + console.error("[renameRepository] GITHUB_TOKEN missing"); + return { success: false, error: "GITHUB_TOKEN missing" }; + } if (!/^[\w.-]+$/.test(newName)) { return { @@ -36,16 +43,19 @@ export async function renameRepository(params: { } try { - const response = await fetch(`https://api.github.com/repos/${owner}/${repo}`, { - method: "PATCH", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${token}`, - "Content-Type": "application/json", - "X-GitHub-Api-Version": "2022-11-28", + const response = await fetch( + `https://api.github.com/repos/${RECOUPABLE_GITHUB_OWNER}/${repo}`, + { + method: "PATCH", + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + "X-GitHub-Api-Version": "2022-11-28", + }, + body: JSON.stringify({ name: newName }), }, - body: JSON.stringify({ name: newName }), - }); + ); if (response.status === 200) { const data = (await response.json()) as { @@ -79,7 +89,7 @@ export async function renameRepository(params: { body = ""; } console.error( - `[renameRepository] unexpected status ${response.status} for ${owner}/${repo}: ${body}`, + `[renameRepository] unexpected status ${response.status} for ${RECOUPABLE_GITHUB_OWNER}/${repo}: ${body}`, ); return { success: false, error: `GitHub returned ${response.status}` }; } catch (error) { diff --git a/lib/github/repositoryExists.ts b/lib/github/repositoryExists.ts index bf7f6e42a..62e4e5f16 100644 --- a/lib/github/repositoryExists.ts +++ b/lib/github/repositoryExists.ts @@ -1,39 +1,43 @@ +import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; +import { getServiceGithubToken } from "./getServiceGithubToken"; + /** - * Returns `true` if `/` exists, `false` if 404, `null` on - * any other failure (auth, rate limit, network). Lets callers - * distinguish "doesn't exist yet" from "couldn't reach GitHub" before - * attempting destructive ops like create. - * - * Ported from open-agents `apps/web/lib/github/repository-exists.ts` — - * recoup-api uses plain `fetch` rather than Octokit to match the - * existing `lib/github/*` style. + * Returns `true` if `recoupable/` exists, `false` if 404, `null` + * on any other failure (auth, rate limit, network, missing token). + * Lets callers distinguish "doesn't exist yet" from "couldn't reach + * GitHub" before attempting destructive ops like create. * - * @param owner - GitHub owner (org or user login). - * @param repo - Repository name (the part after the slash). - * @param token - GitHub PAT or service token. Required — public - * `GET /repos/{owner}/{repo}` is rate-limited and private repos 404 - * when unauthenticated, so an unauthenticated probe is ambiguous. + * Owner is hard-coded to `recoupable` and the GitHub token is read + * from the environment (per PR #618 review — single source of truth). */ -export async function repositoryExists(params: { - owner: string; - repo: string; - token: string; -}): Promise { - const { owner, repo, token } = params; +export async function repositoryExists(params: { repo: string }): Promise { + const { repo } = params; + + const token = getServiceGithubToken(); + if (!token) { + console.error("[repositoryExists] GITHUB_TOKEN missing"); + return null; + } + try { - const response = await fetch(`https://api.github.com/repos/${owner}/${repo}`, { - method: "GET", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${token}`, - "X-GitHub-Api-Version": "2022-11-28", + const response = await fetch( + `https://api.github.com/repos/${RECOUPABLE_GITHUB_OWNER}/${repo}`, + { + method: "GET", + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "X-GitHub-Api-Version": "2022-11-28", + }, }, - }); + ); if (response.status === 200) return true; if (response.status === 404) return false; - console.error(`[repositoryExists] unexpected status ${response.status} for ${owner}/${repo}`); + console.error( + `[repositoryExists] unexpected status ${response.status} for ${RECOUPABLE_GITHUB_OWNER}/${repo}`, + ); return null; } catch (error) { console.error("[repositoryExists] network error:", error); diff --git a/lib/recoupable/__tests__/ensurePersonalRepo.test.ts b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts index 8f66731ba..156b15ffc 100644 --- a/lib/recoupable/__tests__/ensurePersonalRepo.test.ts +++ b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts @@ -4,11 +4,7 @@ import { repositoryExists } from "@/lib/github/repositoryExists"; import { createRepository } from "@/lib/github/createRepository"; import { findLegacyAccountRepo } from "@/lib/github/findLegacyAccountRepo"; import { renameRepository } from "@/lib/github/renameRepository"; -import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; -vi.mock("@/lib/github/getServiceGithubToken", () => ({ - getServiceGithubToken: vi.fn(), -})); vi.mock("@/lib/github/repositoryExists", () => ({ repositoryExists: vi.fn(), })); @@ -29,17 +25,7 @@ describe("ensurePersonalRepo", () => { vi.clearAllMocks(); }); - it("returns null when GITHUB_TOKEN is missing", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue(undefined); - - const result = await ensurePersonalRepo({ accountId }); - - expect(result).toBeNull(); - expect(repositoryExists).not.toHaveBeenCalled(); - }); - it("returns the existing repo URL when recoupable/ already exists", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(true); const result = await ensurePersonalRepo({ accountId }); @@ -56,7 +42,6 @@ describe("ensurePersonalRepo", () => { }); it("returns null when the existence check fails for non-404 reasons", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(null); expect(await ensurePersonalRepo({ accountId })).toBeNull(); @@ -64,7 +49,6 @@ describe("ensurePersonalRepo", () => { }); it("renames a legacy - repo when present (history preserved)", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(false); vi.mocked(findLegacyAccountRepo).mockResolvedValue(`sweetman-${accountId}`); vi.mocked(renameRepository).mockResolvedValue({ @@ -76,10 +60,8 @@ describe("ensurePersonalRepo", () => { const result = await ensurePersonalRepo({ accountId }); expect(renameRepository).toHaveBeenCalledWith({ - owner: "recoupable", repo: `sweetman-${accountId}`, newName: accountId, - token: "tok", }); expect(result).toEqual({ cloneUrl: `https://github.com/recoupable/${accountId}`, @@ -91,7 +73,6 @@ describe("ensurePersonalRepo", () => { }); it("falls through to create when the legacy rename fails", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(false); vi.mocked(findLegacyAccountRepo).mockResolvedValue(`sweetman-${accountId}`); vi.mocked(renameRepository).mockResolvedValue({ @@ -108,12 +89,11 @@ describe("ensurePersonalRepo", () => { const result = await ensurePersonalRepo({ accountId }); - expect(createRepository).toHaveBeenCalled(); + expect(createRepository).toHaveBeenCalledWith({ name: accountId }); expect(result?.repoName).toBe(accountId); }); it("creates a fresh repo when no legacy match exists", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(false); vi.mocked(findLegacyAccountRepo).mockResolvedValue(null); vi.mocked(createRepository).mockResolvedValue({ @@ -126,20 +106,12 @@ describe("ensurePersonalRepo", () => { const result = await ensurePersonalRepo({ accountId }); - expect(createRepository).toHaveBeenCalledWith( - expect.objectContaining({ - owner: "recoupable", - name: accountId, - isPrivate: true, - token: "tok", - }), - ); + expect(createRepository).toHaveBeenCalledWith({ name: accountId }); expect(renameRepository).not.toHaveBeenCalled(); expect(result?.repoName).toBe(accountId); }); it("creates a fresh repo when legacy search itself errors (undefined)", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(false); vi.mocked(findLegacyAccountRepo).mockResolvedValue(undefined); vi.mocked(createRepository).mockResolvedValue({ @@ -158,7 +130,6 @@ describe("ensurePersonalRepo", () => { }); it("returns null when creation outright fails", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue("tok"); vi.mocked(repositoryExists).mockResolvedValue(false); vi.mocked(findLegacyAccountRepo).mockResolvedValue(null); vi.mocked(createRepository).mockResolvedValue({ diff --git a/lib/recoupable/ensurePersonalRepo.ts b/lib/recoupable/ensurePersonalRepo.ts index 2b841e3e3..339aa6f62 100644 --- a/lib/recoupable/ensurePersonalRepo.ts +++ b/lib/recoupable/ensurePersonalRepo.ts @@ -1,4 +1,3 @@ -import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; import { createRepository } from "@/lib/github/createRepository"; import { repositoryExists } from "@/lib/github/repositoryExists"; import { findLegacyAccountRepo } from "@/lib/github/findLegacyAccountRepo"; @@ -30,8 +29,9 @@ export interface EnsurePersonalRepoResult { * 3. Otherwise, create a fresh `recoupable/` with * `auto_init: true` so the sandbox has a `main` branch to clone. * - * Returns `null` only when the service token is missing or repo - * creation outright fails — the caller surfaces that as a 502. + * Returns `null` only when the underlying GitHub helpers can't get + * a service token or repo creation outright fails — the caller + * surfaces that as a 502. * * The legacy-rename branch never blocks provisioning: if the search * API throws or returns ambiguous results, we fall through to step 3 @@ -41,19 +41,9 @@ export interface EnsurePersonalRepoResult { export async function ensurePersonalRepo(params: { accountId: string; }): Promise { - const token = getServiceGithubToken(); - if (!token) { - console.error("[ensurePersonalRepo] GITHUB_TOKEN missing; cannot ensure repo"); - return null; - } - const { repo: repoName } = buildPersonalRepoIdentifier(params); - const existing = await repositoryExists({ - owner: RECOUPABLE_GITHUB_OWNER, - repo: repoName, - token, - }); + const existing = await repositoryExists({ repo: repoName }); if (existing === null) { console.error(`[ensurePersonalRepo] failed to check ${RECOUPABLE_GITHUB_OWNER}/${repoName}`); @@ -73,17 +63,13 @@ export async function ensurePersonalRepo(params: { // account's existing code follows them onto the new naming // convention without a manual migration step. const legacyName = await findLegacyAccountRepo({ - owner: RECOUPABLE_GITHUB_OWNER, accountId: params.accountId, - token, }); if (legacyName) { const renamed = await renameRepository({ - owner: RECOUPABLE_GITHUB_OWNER, repo: legacyName, newName: repoName, - token, }); if (renamed.success) { console.log( @@ -101,13 +87,7 @@ export async function ensurePersonalRepo(params: { ); } - const created = await createRepository({ - owner: RECOUPABLE_GITHUB_OWNER, - name: repoName, - description: `Recoupable workspace for account ${params.accountId}`, - isPrivate: true, - token, - }); + const created = await createRepository({ name: repoName }); if (!created.success || !created.cloneUrl || !created.repoUrl) { console.error(`[ensurePersonalRepo] createRepository failed: ${created.error ?? "unknown"}`); diff --git a/scripts/migrate-workspace-repo-names.ts b/scripts/migrate-workspace-repo-names.ts index f9e417ad6..3c2892be4 100644 --- a/scripts/migrate-workspace-repo-names.ts +++ b/scripts/migrate-workspace-repo-names.ts @@ -119,12 +119,7 @@ async function main(): Promise { let succeeded = 0; let failed = 0; for (const a of toRename) { - const result = await renameRepository({ - owner: RECOUPABLE_GITHUB_OWNER, - repo: a.from, - newName: a.to, - token, - }); + const result = await renameRepository({ repo: a.from, newName: a.to }); if (result.success) { succeeded += 1; console.log(` ok: ${a.from} -> ${a.to}`); From 3c9e18dbbc88b7be168a6b323a68c90f2be9a7f0 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 11:46:35 -0500 Subject: [PATCH 05/12] prune: drop runtime legacy-rename branch (script-once is enough) The migration script renames every legacy - repo once before merge, after which no legacy repo can exist for ensurePersonalRepo to find. The runtime self-healing branch was pure YAGNI. Removed: - lib/github/findLegacyAccountRepo.ts (only caller was the runtime-rename branch) - lib/github/renameRepository.ts (sole consumer is the migration script; PATCH-rename inlined there) - the legacy-rename branch + its tests in ensurePersonalRepo ensurePersonalRepo is now just exists -> create. Co-Authored-By: Claude Opus 4.7 --- .../__tests__/findLegacyAccountRepo.test.ts | 92 ---------------- lib/github/__tests__/renameRepository.test.ts | 103 ------------------ lib/github/findLegacyAccountRepo.ts | 75 ------------- lib/github/renameRepository.ts | 99 ----------------- .../__tests__/ensurePersonalRepo.test.ts | 78 +------------ lib/recoupable/ensurePersonalRepo.ts | 58 ++-------- scripts/migrate-workspace-repo-names.ts | 36 +++++- 7 files changed, 44 insertions(+), 497 deletions(-) delete mode 100644 lib/github/__tests__/findLegacyAccountRepo.test.ts delete mode 100644 lib/github/__tests__/renameRepository.test.ts delete mode 100644 lib/github/findLegacyAccountRepo.ts delete mode 100644 lib/github/renameRepository.ts diff --git a/lib/github/__tests__/findLegacyAccountRepo.test.ts b/lib/github/__tests__/findLegacyAccountRepo.test.ts deleted file mode 100644 index 1483568ef..000000000 --- a/lib/github/__tests__/findLegacyAccountRepo.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; -import { findLegacyAccountRepo } from "@/lib/github/findLegacyAccountRepo"; -import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; - -vi.mock("@/lib/github/getServiceGithubToken", () => ({ - getServiceGithubToken: vi.fn(() => "tok"), -})); - -const accountId = "fb678396-a68f-4294-ae50-b8cacf9ce77b"; - -describe("findLegacyAccountRepo", () => { - beforeEach(() => { - vi.clearAllMocks(); - vi.mocked(getServiceGithubToken).mockReturnValue("tok"); - }); - - it("returns undefined when GITHUB_TOKEN is missing", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue(undefined); - const fetchSpy = vi.spyOn(globalThis, "fetch"); - - const result = await findLegacyAccountRepo({ accountId }); - - expect(result).toBeUndefined(); - expect(fetchSpy).not.toHaveBeenCalled(); - }); - - it("returns the legacy name when a *- match exists", async () => { - vi.spyOn(globalThis, "fetch").mockResolvedValue( - new Response( - JSON.stringify({ - items: [{ name: `sweetman-${accountId}` }, { name: "some-unrelated-repo" }], - }), - { status: 200, headers: { "content-type": "application/json" } }, - ), - ); - - const result = await findLegacyAccountRepo({ accountId }); - - expect(result).toBe(`sweetman-${accountId}`); - }); - - it("ignores the new bare- repo (we don't want to rename it onto itself)", async () => { - vi.spyOn(globalThis, "fetch").mockResolvedValue( - new Response(JSON.stringify({ items: [{ name: accountId }] }), { - status: 200, - headers: { "content-type": "application/json" }, - }), - ); - - const result = await findLegacyAccountRepo({ accountId }); - - expect(result).toBeNull(); - }); - - it("returns null when no items match the legacy pattern", async () => { - vi.spyOn(globalThis, "fetch").mockResolvedValue( - new Response(JSON.stringify({ items: [] }), { - status: 200, - headers: { "content-type": "application/json" }, - }), - ); - - const result = await findLegacyAccountRepo({ accountId }); - - expect(result).toBeNull(); - }); - - it("returns undefined on non-OK response (caller treats as miss)", async () => { - vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 503 })); - - const result = await findLegacyAccountRepo({ accountId }); - - expect(result).toBeUndefined(); - }); - - it("encodes the search query as +in:name+org:recoupable", async () => { - const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( - new Response(JSON.stringify({ items: [] }), { - status: 200, - headers: { "content-type": "application/json" }, - }), - ); - - await findLegacyAccountRepo({ accountId: "id-1" }); - - const [url] = fetchSpy.mock.calls[0] as [string]; - expect(url).toContain("/search/repositories?q="); - expect(decodeURIComponent(url.split("?q=")[1].split("&")[0])).toBe( - "id-1 in:name org:recoupable", - ); - }); -}); diff --git a/lib/github/__tests__/renameRepository.test.ts b/lib/github/__tests__/renameRepository.test.ts deleted file mode 100644 index 22421c743..000000000 --- a/lib/github/__tests__/renameRepository.test.ts +++ /dev/null @@ -1,103 +0,0 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; -import { renameRepository } from "@/lib/github/renameRepository"; -import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; - -vi.mock("@/lib/github/getServiceGithubToken", () => ({ - getServiceGithubToken: vi.fn(() => "tok"), -})); - -describe("renameRepository", () => { - beforeEach(() => { - vi.clearAllMocks(); - vi.mocked(getServiceGithubToken).mockReturnValue("tok"); - }); - - it("returns failure when GITHUB_TOKEN is missing", async () => { - vi.mocked(getServiceGithubToken).mockReturnValue(undefined); - const fetchSpy = vi.spyOn(globalThis, "fetch"); - - const result = await renameRepository({ - repo: "sweetman-id-1", - newName: "id-1", - }); - - expect(result.success).toBe(false); - expect(result.error).toMatch(/GITHUB_TOKEN/i); - expect(fetchSpy).not.toHaveBeenCalled(); - }); - - it("rejects invalid newName without hitting the network", async () => { - const fetchSpy = vi.spyOn(globalThis, "fetch"); - - const result = await renameRepository({ - repo: "sweetman-id-1", - newName: "bad name", - }); - - expect(result.success).toBe(false); - expect(fetchSpy).not.toHaveBeenCalled(); - }); - - it("PATCHes /repos/recoupable/ with {name} and returns the new URLs on 200", async () => { - const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( - new Response( - JSON.stringify({ - html_url: "https://github.com/recoupable/id-1", - clone_url: "https://github.com/recoupable/id-1.git", - }), - { status: 200, headers: { "content-type": "application/json" } }, - ), - ); - - const result = await renameRepository({ - repo: "sweetman-id-1", - newName: "id-1", - }); - - expect(result).toEqual({ - success: true, - cloneUrl: "https://github.com/recoupable/id-1.git", - repoUrl: "https://github.com/recoupable/id-1", - }); - const [url, init] = fetchSpy.mock.calls[0] as [string, RequestInit]; - expect(url).toBe("https://api.github.com/repos/recoupable/sweetman-id-1"); - expect(init.method).toBe("PATCH"); - expect(JSON.parse(init.body as string)).toEqual({ name: "id-1" }); - }); - - it("returns target-conflict error on 422", async () => { - vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 422 })); - - const result = await renameRepository({ - repo: "sweetman-id-1", - newName: "id-1", - }); - - expect(result.success).toBe(false); - expect(result.error).toMatch(/already exists/i); - }); - - it("returns source-not-found error on 404", async () => { - vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 404 })); - - const result = await renameRepository({ - repo: "sweetman-id-1", - newName: "id-1", - }); - - expect(result.success).toBe(false); - expect(result.error).toMatch(/not found/i); - }); - - it("returns network-error on fetch rejection", async () => { - vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("ECONNRESET")); - - const result = await renameRepository({ - repo: "sweetman-id-1", - newName: "id-1", - }); - - expect(result.success).toBe(false); - expect(result.error).toMatch(/network/i); - }); -}); diff --git a/lib/github/findLegacyAccountRepo.ts b/lib/github/findLegacyAccountRepo.ts deleted file mode 100644 index 4c8b03904..000000000 --- a/lib/github/findLegacyAccountRepo.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; -import { getServiceGithubToken } from "./getServiceGithubToken"; - -const LEGACY_SUFFIX_PATTERN = /^.+-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; - -/** - * Search the `recoupable` org for a legacy-named workspace repo whose - * name ends with `-` — the old `-` - * convention. Returns the legacy repo name so `ensurePersonalRepo` - * can rename it to the new bare-`` convention without - * losing the user's git history. - * - * Returns: - * - the legacy repo name (e.g. `sweetman-fb678396-...`) on hit - * - `null` on miss (no legacy repo exists for this account) - * - `undefined` on unexpected failure (caller should treat as miss - * and continue with create — losing history is preferable to - * blocking provisioning on a flaky GitHub search) - * - * Uses GitHub's `GET /search/repositories` endpoint with - * `+in:name+org:recoupable` so we don't have to paginate - * the entire org. - * - * Owner is hard-coded to `recoupable` and the GitHub token is read - * from the environment (per PR #618 review — single source of truth). - */ -export async function findLegacyAccountRepo(params: { - accountId: string; -}): Promise { - const { accountId } = params; - - const token = getServiceGithubToken(); - if (!token) { - console.error("[findLegacyAccountRepo] GITHUB_TOKEN missing"); - return undefined; - } - - try { - const query = encodeURIComponent(`${accountId} in:name org:${RECOUPABLE_GITHUB_OWNER}`); - const response = await fetch( - `https://api.github.com/search/repositories?q=${query}&per_page=10`, - { - method: "GET", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${token}`, - "X-GitHub-Api-Version": "2022-11-28", - }, - }, - ); - - if (!response.ok) { - console.error( - `[findLegacyAccountRepo] unexpected status ${response.status} for accountId ${accountId}`, - ); - return undefined; - } - - const data = (await response.json()) as { - items?: Array<{ name: string }>; - }; - - const legacy = (data.items ?? []).find( - item => - item.name !== accountId && - item.name.endsWith(`-${accountId}`) && - LEGACY_SUFFIX_PATTERN.test(item.name), - ); - - return legacy?.name ?? null; - } catch (error) { - console.error("[findLegacyAccountRepo] network error:", error); - return undefined; - } -} diff --git a/lib/github/renameRepository.ts b/lib/github/renameRepository.ts deleted file mode 100644 index b015f87de..000000000 --- a/lib/github/renameRepository.ts +++ /dev/null @@ -1,99 +0,0 @@ -import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; -import { getServiceGithubToken } from "./getServiceGithubToken"; - -export interface RenameRepositoryResult { - success: boolean; - /** New `clone_url` returned by GitHub on success. */ - cloneUrl?: string; - /** New `html_url` returned by GitHub on success. */ - repoUrl?: string; - error?: string; -} - -/** - * Rename a Recoupable workspace repository via - * `PATCH /repos/{owner}/{repo}`. - * - * GitHub automatically issues HTTP redirects from the old name to the - * new name for both git operations and the REST API, so existing - * `sessions.clone_url` rows that still reference the old name keep - * resolving — no DB backfill required. - * - * Owner is hard-coded to `recoupable` and the GitHub token is read - * from the environment (per PR #618 review — single source of truth). - */ -export async function renameRepository(params: { - repo: string; - newName: string; -}): Promise { - const { repo, newName } = params; - - const token = getServiceGithubToken(); - if (!token) { - console.error("[renameRepository] GITHUB_TOKEN missing"); - return { success: false, error: "GITHUB_TOKEN missing" }; - } - - if (!/^[\w.-]+$/.test(newName)) { - return { - success: false, - error: - "Invalid repository name. Use only letters, numbers, hyphens, underscores, and periods.", - }; - } - - try { - const response = await fetch( - `https://api.github.com/repos/${RECOUPABLE_GITHUB_OWNER}/${repo}`, - { - method: "PATCH", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${token}`, - "Content-Type": "application/json", - "X-GitHub-Api-Version": "2022-11-28", - }, - body: JSON.stringify({ name: newName }), - }, - ); - - if (response.status === 200) { - const data = (await response.json()) as { - html_url: string; - clone_url: string; - }; - return { - success: true, - cloneUrl: data.clone_url, - repoUrl: data.html_url, - }; - } - - if (response.status === 422) { - return { - success: false, - error: "Target repository name already exists or is invalid", - }; - } - if (response.status === 403) { - return { success: false, error: "Permission denied" }; - } - if (response.status === 404) { - return { success: false, error: "Source repository not found" }; - } - - let body = ""; - try { - body = await response.text(); - } catch { - body = ""; - } - console.error( - `[renameRepository] unexpected status ${response.status} for ${RECOUPABLE_GITHUB_OWNER}/${repo}: ${body}`, - ); - return { success: false, error: `GitHub returned ${response.status}` }; - } catch (error) { - console.error("[renameRepository] network error:", error); - return { success: false, error: "Network error talking to GitHub" }; - } -} diff --git a/lib/recoupable/__tests__/ensurePersonalRepo.test.ts b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts index 156b15ffc..96c622311 100644 --- a/lib/recoupable/__tests__/ensurePersonalRepo.test.ts +++ b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts @@ -2,8 +2,6 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { ensurePersonalRepo } from "@/lib/recoupable/ensurePersonalRepo"; import { repositoryExists } from "@/lib/github/repositoryExists"; import { createRepository } from "@/lib/github/createRepository"; -import { findLegacyAccountRepo } from "@/lib/github/findLegacyAccountRepo"; -import { renameRepository } from "@/lib/github/renameRepository"; vi.mock("@/lib/github/repositoryExists", () => ({ repositoryExists: vi.fn(), @@ -11,12 +9,6 @@ vi.mock("@/lib/github/repositoryExists", () => ({ vi.mock("@/lib/github/createRepository", () => ({ createRepository: vi.fn(), })); -vi.mock("@/lib/github/findLegacyAccountRepo", () => ({ - findLegacyAccountRepo: vi.fn(), -})); -vi.mock("@/lib/github/renameRepository", () => ({ - renameRepository: vi.fn(), -})); const accountId = "fb678396-a68f-4294-ae50-b8cacf9ce77b"; @@ -36,8 +28,6 @@ describe("ensurePersonalRepo", () => { owner: "recoupable", repoName: accountId, }); - expect(findLegacyAccountRepo).not.toHaveBeenCalled(); - expect(renameRepository).not.toHaveBeenCalled(); expect(createRepository).not.toHaveBeenCalled(); }); @@ -45,57 +35,11 @@ describe("ensurePersonalRepo", () => { vi.mocked(repositoryExists).mockResolvedValue(null); expect(await ensurePersonalRepo({ accountId })).toBeNull(); - expect(findLegacyAccountRepo).not.toHaveBeenCalled(); - }); - - it("renames a legacy - repo when present (history preserved)", async () => { - vi.mocked(repositoryExists).mockResolvedValue(false); - vi.mocked(findLegacyAccountRepo).mockResolvedValue(`sweetman-${accountId}`); - vi.mocked(renameRepository).mockResolvedValue({ - success: true, - cloneUrl: `https://github.com/recoupable/${accountId}.git`, - repoUrl: `https://github.com/recoupable/${accountId}`, - }); - - const result = await ensurePersonalRepo({ accountId }); - - expect(renameRepository).toHaveBeenCalledWith({ - repo: `sweetman-${accountId}`, - newName: accountId, - }); - expect(result).toEqual({ - cloneUrl: `https://github.com/recoupable/${accountId}`, - repoUrl: `https://github.com/recoupable/${accountId}`, - owner: "recoupable", - repoName: accountId, - }); expect(createRepository).not.toHaveBeenCalled(); }); - it("falls through to create when the legacy rename fails", async () => { - vi.mocked(repositoryExists).mockResolvedValue(false); - vi.mocked(findLegacyAccountRepo).mockResolvedValue(`sweetman-${accountId}`); - vi.mocked(renameRepository).mockResolvedValue({ - success: false, - error: "boom", - }); - vi.mocked(createRepository).mockResolvedValue({ - success: true, - cloneUrl: `https://github.com/recoupable/${accountId}.git`, - repoUrl: `https://github.com/recoupable/${accountId}`, - owner: "recoupable", - repoName: accountId, - }); - - const result = await ensurePersonalRepo({ accountId }); - - expect(createRepository).toHaveBeenCalledWith({ name: accountId }); - expect(result?.repoName).toBe(accountId); - }); - - it("creates a fresh repo when no legacy match exists", async () => { + it("creates a fresh repo when none exists", async () => { vi.mocked(repositoryExists).mockResolvedValue(false); - vi.mocked(findLegacyAccountRepo).mockResolvedValue(null); vi.mocked(createRepository).mockResolvedValue({ success: true, cloneUrl: `https://github.com/recoupable/${accountId}.git`, @@ -107,31 +51,11 @@ describe("ensurePersonalRepo", () => { const result = await ensurePersonalRepo({ accountId }); expect(createRepository).toHaveBeenCalledWith({ name: accountId }); - expect(renameRepository).not.toHaveBeenCalled(); - expect(result?.repoName).toBe(accountId); - }); - - it("creates a fresh repo when legacy search itself errors (undefined)", async () => { - vi.mocked(repositoryExists).mockResolvedValue(false); - vi.mocked(findLegacyAccountRepo).mockResolvedValue(undefined); - vi.mocked(createRepository).mockResolvedValue({ - success: true, - cloneUrl: `https://github.com/recoupable/${accountId}.git`, - repoUrl: `https://github.com/recoupable/${accountId}`, - owner: "recoupable", - repoName: accountId, - }); - - const result = await ensurePersonalRepo({ accountId }); - - expect(renameRepository).not.toHaveBeenCalled(); - expect(createRepository).toHaveBeenCalled(); expect(result?.repoName).toBe(accountId); }); it("returns null when creation outright fails", async () => { vi.mocked(repositoryExists).mockResolvedValue(false); - vi.mocked(findLegacyAccountRepo).mockResolvedValue(null); vi.mocked(createRepository).mockResolvedValue({ success: false, error: "Permission denied", diff --git a/lib/recoupable/ensurePersonalRepo.ts b/lib/recoupable/ensurePersonalRepo.ts index 339aa6f62..cdce73c1b 100644 --- a/lib/recoupable/ensurePersonalRepo.ts +++ b/lib/recoupable/ensurePersonalRepo.ts @@ -1,7 +1,5 @@ import { createRepository } from "@/lib/github/createRepository"; import { repositoryExists } from "@/lib/github/repositoryExists"; -import { findLegacyAccountRepo } from "@/lib/github/findLegacyAccountRepo"; -import { renameRepository } from "@/lib/github/renameRepository"; import { buildPersonalRepoIdentifier } from "./buildPersonalRepoIdentifier"; import { buildPersonalRepoUrl } from "./buildPersonalRepoUrl"; import { RECOUPABLE_GITHUB_OWNER } from "./githubOwner"; @@ -17,26 +15,18 @@ export interface EnsurePersonalRepoResult { * Idempotently ensure an account has a workspace repo at the * canonical `recoupable/` location. * - * Resolution order: - * 1. If `recoupable/` already exists → return its URL - * (clean idempotent case — most calls). - * 2. Look for a legacy `recoupable/<*>-` repo via - * GitHub search. If found, rename it to `` so the - * account's git history follows them onto the new convention. - * GitHub auto-redirects the old URL for clones + REST, so any - * `sessions.clone_url` rows that still reference the old name - * keep working. - * 3. Otherwise, create a fresh `recoupable/` with - * `auto_init: true` so the sandbox has a `main` branch to clone. + * 1. If `recoupable/` already exists → return its URL. + * 2. Otherwise create it with `auto_init: true` so the sandbox has + * a `main` branch to clone. * - * Returns `null` only when the underlying GitHub helpers can't get - * a service token or repo creation outright fails — the caller - * surfaces that as a 502. + * Returns `null` only when the GitHub helpers can't get a service + * token or repo creation outright fails — the caller surfaces that as + * a 502. * - * The legacy-rename branch never blocks provisioning: if the search - * API throws or returns ambiguous results, we fall through to step 3 - * and create a fresh repo. Losing history for an edge-case account is - * preferable to blocking their session on a flaky GitHub search. + * Legacy `-` repos are renamed once, ahead of time, + * by `scripts/migrate-workspace-repo-names.ts`. After that one-shot + * migration there's no legacy repo to find at runtime, so no + * rename-on-demand branch lives here. */ export async function ensurePersonalRepo(params: { accountId: string; @@ -59,34 +49,6 @@ export async function ensurePersonalRepo(params: { }; } - // Try to find and rename a legacy `-` repo so the - // account's existing code follows them onto the new naming - // convention without a manual migration step. - const legacyName = await findLegacyAccountRepo({ - accountId: params.accountId, - }); - - if (legacyName) { - const renamed = await renameRepository({ - repo: legacyName, - newName: repoName, - }); - if (renamed.success) { - console.log( - `[ensurePersonalRepo] renamed legacy ${RECOUPABLE_GITHUB_OWNER}/${legacyName} → ${repoName}`, - ); - return { - cloneUrl: buildPersonalRepoUrl(params), - repoUrl: `https://github.com/${RECOUPABLE_GITHUB_OWNER}/${repoName}`, - owner: RECOUPABLE_GITHUB_OWNER, - repoName, - }; - } - console.error( - `[ensurePersonalRepo] legacy rename failed (${renamed.error ?? "unknown"}); falling through to create`, - ); - } - const created = await createRepository({ name: repoName }); if (!created.success || !created.cloneUrl || !created.repoUrl) { diff --git a/scripts/migrate-workspace-repo-names.ts b/scripts/migrate-workspace-repo-names.ts index 3c2892be4..a67abbd1f 100644 --- a/scripts/migrate-workspace-repo-names.ts +++ b/scripts/migrate-workspace-repo-names.ts @@ -20,10 +20,13 @@ * * Requires `GITHUB_TOKEN` with `admin:repo` scope on the `recoupable` * org in the environment (or `.env.local`). + * + * The PATCH-rename request lives inline (rather than as a `lib/github/*` + * helper) because this script is its only caller — once it runs every + * legacy repo is renamed and there is no runtime caller to reuse. */ import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; -import { renameRepository } from "@/lib/github/renameRepository"; const LEGACY_PATTERN = /^(?.+)-(?[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})$/i; @@ -62,6 +65,33 @@ async function listAllOrgRepos(token: string): Promise { return repos; } +async function renameRepo( + from: string, + to: string, + token: string, +): Promise<{ ok: boolean; error?: string }> { + try { + const response = await fetch( + `https://api.github.com/repos/${RECOUPABLE_GITHUB_OWNER}/${from}`, + { + method: "PATCH", + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + "X-GitHub-Api-Version": "2022-11-28", + }, + body: JSON.stringify({ name: to }), + }, + ); + if (response.status === 200) return { ok: true }; + const body = await response.text().catch(() => ""); + return { ok: false, error: `${response.status} ${body}` }; + } catch (error) { + return { ok: false, error: (error as Error).message }; + } +} + async function main(): Promise { const apply = process.argv.includes("--apply"); const token = process.env.GITHUB_TOKEN; @@ -119,8 +149,8 @@ async function main(): Promise { let succeeded = 0; let failed = 0; for (const a of toRename) { - const result = await renameRepository({ repo: a.from, newName: a.to }); - if (result.success) { + const result = await renameRepo(a.from, a.to, token); + if (result.ok) { succeeded += 1; console.log(` ok: ${a.from} -> ${a.to}`); } else { From b061ff0e1b38fae38fe17ee01891b0adfc672d18 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 11:53:39 -0500 Subject: [PATCH 06/12] prune: inline repo-name + URL into ensurePersonalRepo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Sweet's review on buildPersonalRepoIdentifier.ts: post-refactor the helpers hide nothing — the repo name IS the accountId, the URL is one string concat. ensurePersonalRepo was their only runtime caller. Removed: - lib/recoupable/buildPersonalRepoIdentifier.ts (+ test) - lib/recoupable/buildPersonalRepoUrl.ts ensurePersonalRepo now derives the two values inline at the top of the function. Co-Authored-By: Claude Opus 4.7 --- .../buildPersonalRepoIdentifier.test.ts | 31 ------------------- lib/recoupable/buildPersonalRepoIdentifier.ts | 22 ------------- lib/recoupable/buildPersonalRepoUrl.ts | 13 -------- lib/recoupable/ensurePersonalRepo.ts | 9 +++--- 4 files changed, 4 insertions(+), 71 deletions(-) delete mode 100644 lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts delete mode 100644 lib/recoupable/buildPersonalRepoIdentifier.ts delete mode 100644 lib/recoupable/buildPersonalRepoUrl.ts diff --git a/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts b/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts deleted file mode 100644 index def66d3df..000000000 --- a/lib/recoupable/__tests__/buildPersonalRepoIdentifier.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { buildPersonalRepoIdentifier } from "@/lib/recoupable/buildPersonalRepoIdentifier"; -import { buildPersonalRepoUrl } from "@/lib/recoupable/buildPersonalRepoUrl"; - -describe("buildPersonalRepoIdentifier", () => { - it("returns recoupable owner + bare accountId repo", () => { - expect( - buildPersonalRepoIdentifier({ - accountId: "fb678396-a68f-4294-ae50-b8cacf9ce77b", - }), - ).toEqual({ - owner: "recoupable", - repo: "fb678396-a68f-4294-ae50-b8cacf9ce77b", - }); - }); - - it("does not embed any name — naming stays stable across renames", () => { - const { repo } = buildPersonalRepoIdentifier({ accountId: "id-1" }); - expect(repo).toBe("id-1"); - }); -}); - -describe("buildPersonalRepoUrl", () => { - it("composes the GitHub URL as recoupable/", () => { - expect( - buildPersonalRepoUrl({ - accountId: "fb678396-a68f-4294-ae50-b8cacf9ce77b", - }), - ).toBe("https://github.com/recoupable/fb678396-a68f-4294-ae50-b8cacf9ce77b"); - }); -}); diff --git a/lib/recoupable/buildPersonalRepoIdentifier.ts b/lib/recoupable/buildPersonalRepoIdentifier.ts deleted file mode 100644 index 30f22ba5d..000000000 --- a/lib/recoupable/buildPersonalRepoIdentifier.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { RECOUPABLE_GITHUB_OWNER } from "./githubOwner"; - -/** - * Returns the `` pair for an account's Recoupable - * workspace repo. The repo name is the account UUID with no slug - * prefix — keeps the lookup stable across account renames and unifies - * the personal vs. org repo naming so every workspace repo is keyed - * the same way. - * - * Convention: `recoupable/`. - * - * Account names are mutable (`account_info.name` can be edited at any - * time), so any naming scheme that embeds the name eventually drifts - * — see [[feedback_unified_workspace_repo_naming]] for the design call - * to drop the slug entirely. - */ -export function buildPersonalRepoIdentifier(params: { accountId: string }): { - owner: string; - repo: string; -} { - return { owner: RECOUPABLE_GITHUB_OWNER, repo: params.accountId }; -} diff --git a/lib/recoupable/buildPersonalRepoUrl.ts b/lib/recoupable/buildPersonalRepoUrl.ts deleted file mode 100644 index 68b097065..000000000 --- a/lib/recoupable/buildPersonalRepoUrl.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { buildPersonalRepoIdentifier } from "./buildPersonalRepoIdentifier"; - -/** - * Builds the GitHub URL for an account's Recoupable workspace repo. - * Convention: `https://github.com/recoupable/` — the - * account UUID is the repo name with no slug prefix. - * - * Example: `https://github.com/recoupable/fb678396-a68f-4294-ae50-b8cacf9ce77b`. - */ -export function buildPersonalRepoUrl(params: { accountId: string }): string { - const { owner, repo } = buildPersonalRepoIdentifier(params); - return `https://github.com/${owner}/${repo}`; -} diff --git a/lib/recoupable/ensurePersonalRepo.ts b/lib/recoupable/ensurePersonalRepo.ts index cdce73c1b..0c91e7954 100644 --- a/lib/recoupable/ensurePersonalRepo.ts +++ b/lib/recoupable/ensurePersonalRepo.ts @@ -1,7 +1,5 @@ import { createRepository } from "@/lib/github/createRepository"; import { repositoryExists } from "@/lib/github/repositoryExists"; -import { buildPersonalRepoIdentifier } from "./buildPersonalRepoIdentifier"; -import { buildPersonalRepoUrl } from "./buildPersonalRepoUrl"; import { RECOUPABLE_GITHUB_OWNER } from "./githubOwner"; export interface EnsurePersonalRepoResult { @@ -31,7 +29,8 @@ export interface EnsurePersonalRepoResult { export async function ensurePersonalRepo(params: { accountId: string; }): Promise { - const { repo: repoName } = buildPersonalRepoIdentifier(params); + const repoName = params.accountId; + const repoUrl = `https://github.com/${RECOUPABLE_GITHUB_OWNER}/${repoName}`; const existing = await repositoryExists({ repo: repoName }); @@ -42,8 +41,8 @@ export async function ensurePersonalRepo(params: { if (existing) { return { - cloneUrl: buildPersonalRepoUrl(params), - repoUrl: `https://github.com/${RECOUPABLE_GITHUB_OWNER}/${repoName}`, + cloneUrl: repoUrl, + repoUrl, owner: RECOUPABLE_GITHUB_OWNER, repoName, }; From 7a9d4dde0b50a1b380449492c8b31b1b569cd9b6 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 12:00:11 -0500 Subject: [PATCH 07/12] prune: ensurePersonalRepo now returns just the clone URL string Per Sweet's review on EnsurePersonalRepoResult: the only caller (resolveSessionCloneUrl) reads `cloneUrl` and nothing else. The other three fields (repoUrl, owner, repoName) were written into the response but never consumed. Drop the interface; return Promise. Co-Authored-By: Claude Opus 4.7 --- .../__tests__/ensurePersonalRepo.test.ts | 11 ++---- lib/recoupable/ensurePersonalRepo.ts | 35 +++++-------------- .../__tests__/resolveSessionCloneUrl.test.ts | 7 +--- lib/sessions/resolveSessionCloneUrl.ts | 6 ++-- 4 files changed, 16 insertions(+), 43 deletions(-) diff --git a/lib/recoupable/__tests__/ensurePersonalRepo.test.ts b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts index 96c622311..081a54382 100644 --- a/lib/recoupable/__tests__/ensurePersonalRepo.test.ts +++ b/lib/recoupable/__tests__/ensurePersonalRepo.test.ts @@ -22,12 +22,7 @@ describe("ensurePersonalRepo", () => { const result = await ensurePersonalRepo({ accountId }); - expect(result).toEqual({ - cloneUrl: `https://github.com/recoupable/${accountId}`, - repoUrl: `https://github.com/recoupable/${accountId}`, - owner: "recoupable", - repoName: accountId, - }); + expect(result).toBe(`https://github.com/recoupable/${accountId}`); expect(createRepository).not.toHaveBeenCalled(); }); @@ -38,7 +33,7 @@ describe("ensurePersonalRepo", () => { expect(createRepository).not.toHaveBeenCalled(); }); - it("creates a fresh repo when none exists", async () => { + it("creates a fresh repo when none exists and returns its clone URL", async () => { vi.mocked(repositoryExists).mockResolvedValue(false); vi.mocked(createRepository).mockResolvedValue({ success: true, @@ -51,7 +46,7 @@ describe("ensurePersonalRepo", () => { const result = await ensurePersonalRepo({ accountId }); expect(createRepository).toHaveBeenCalledWith({ name: accountId }); - expect(result?.repoName).toBe(accountId); + expect(result).toBe(`https://github.com/recoupable/${accountId}.git`); }); it("returns null when creation outright fails", async () => { diff --git a/lib/recoupable/ensurePersonalRepo.ts b/lib/recoupable/ensurePersonalRepo.ts index 0c91e7954..eddbce68a 100644 --- a/lib/recoupable/ensurePersonalRepo.ts +++ b/lib/recoupable/ensurePersonalRepo.ts @@ -2,33 +2,26 @@ import { createRepository } from "@/lib/github/createRepository"; import { repositoryExists } from "@/lib/github/repositoryExists"; import { RECOUPABLE_GITHUB_OWNER } from "./githubOwner"; -export interface EnsurePersonalRepoResult { - cloneUrl: string; - repoUrl: string; - owner: string; - repoName: string; -} - /** * Idempotently ensure an account has a workspace repo at the * canonical `recoupable/` location. * * 1. If `recoupable/` already exists → return its URL. * 2. Otherwise create it with `auto_init: true` so the sandbox has - * a `main` branch to clone. + * a `main` branch to clone, and return the new clone URL. * * Returns `null` only when the GitHub helpers can't get a service - * token or repo creation outright fails — the caller surfaces that as - * a 502. + * token or repo creation outright fails — the caller surfaces that + * as a 502. The single-string return matches what callers actually + * consume (the clone URL); owner / repo name are trivially + * recoverable from the URL if ever needed. * * Legacy `-` repos are renamed once, ahead of time, * by `scripts/migrate-workspace-repo-names.ts`. After that one-shot * migration there's no legacy repo to find at runtime, so no * rename-on-demand branch lives here. */ -export async function ensurePersonalRepo(params: { - accountId: string; -}): Promise { +export async function ensurePersonalRepo(params: { accountId: string }): Promise { const repoName = params.accountId; const repoUrl = `https://github.com/${RECOUPABLE_GITHUB_OWNER}/${repoName}`; @@ -40,25 +33,15 @@ export async function ensurePersonalRepo(params: { } if (existing) { - return { - cloneUrl: repoUrl, - repoUrl, - owner: RECOUPABLE_GITHUB_OWNER, - repoName, - }; + return repoUrl; } const created = await createRepository({ name: repoName }); - if (!created.success || !created.cloneUrl || !created.repoUrl) { + if (!created.success || !created.cloneUrl) { console.error(`[ensurePersonalRepo] createRepository failed: ${created.error ?? "unknown"}`); return null; } - return { - cloneUrl: created.cloneUrl, - repoUrl: created.repoUrl, - owner: created.owner ?? RECOUPABLE_GITHUB_OWNER, - repoName: created.repoName ?? repoName, - }; + return created.cloneUrl; } diff --git a/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts b/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts index 3f199d77f..0c7c1db26 100644 --- a/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts +++ b/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts @@ -43,12 +43,7 @@ describe("resolveSessionCloneUrl", () => { }); it("provisions a personal repo when no body cloneUrl and no org", async () => { - vi.mocked(ensurePersonalRepo).mockResolvedValue({ - cloneUrl: `https://github.com/recoupable/${accountId}`, - repoUrl: `https://github.com/recoupable/${accountId}`, - owner: "recoupable", - repoName: accountId, - }); + vi.mocked(ensurePersonalRepo).mockResolvedValue(`https://github.com/recoupable/${accountId}`); const result = await resolveSessionCloneUrl({ bodyCloneUrl: undefined, diff --git a/lib/sessions/resolveSessionCloneUrl.ts b/lib/sessions/resolveSessionCloneUrl.ts index ce0d9c4ca..e67348272 100644 --- a/lib/sessions/resolveSessionCloneUrl.ts +++ b/lib/sessions/resolveSessionCloneUrl.ts @@ -45,9 +45,9 @@ export async function resolveSessionCloneUrl(params: { return { ok: true, cloneUrl: null }; } - const ensured = await ensurePersonalRepo({ accountId: auth.accountId }); + const cloneUrl = await ensurePersonalRepo({ accountId: auth.accountId }); - if (!ensured) { + if (!cloneUrl) { return { ok: false, cloneUrl: null, @@ -55,5 +55,5 @@ export async function resolveSessionCloneUrl(params: { }; } - return { ok: true, cloneUrl: ensured.cloneUrl }; + return { ok: true, cloneUrl }; } From 90d194cd214c5745cc25f930c237df1a0968ec40 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 12:13:35 -0500 Subject: [PATCH 08/12] unify: provision org workspace repo too (cloneUrl: null branch fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Sweet's review on resolveSessionCloneUrl.ts:45 — the cloneUrl: null early-return for auth.orgId was leftover hedging that contradicts the unified recoupable/ design. Organizations ARE accounts in the data model (account_organization_ids.organization joins the accounts table), so auth.orgId is itself an account_id. The fix: drop the null-return branch and always call ensurePersonalRepo, keyed on auth.orgId ?? auth.accountId. Personal and org sessions now provision the same way — at recoupable/, where accountId is either the user's or the org's. Error message updated from "personal repository" to "workspace repository" to reflect the unified naming. Co-Authored-By: Claude Opus 4.7 --- .../__tests__/resolveSessionCloneUrl.test.ts | 27 +++++++++------ lib/sessions/resolveSessionCloneUrl.ts | 34 ++++++++----------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts b/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts index 0c7c1db26..ed2104789 100644 --- a/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts +++ b/lib/sessions/__tests__/resolveSessionCloneUrl.test.ts @@ -8,6 +8,7 @@ vi.mock("@/lib/recoupable/ensurePersonalRepo", () => ({ })); const accountId = "fb678396-a68f-4294-ae50-b8cacf9ce77b"; +const orgId = "0a0a0a0a-a0a0-4a0a-8a0a-aaaaaaaaaaaa"; const baseAuth: AuthContext = { accountId, orgId: null, @@ -22,7 +23,7 @@ describe("resolveSessionCloneUrl", () => { it("uses the body cloneUrl when provided, regardless of org state", async () => { const result = await resolveSessionCloneUrl({ bodyCloneUrl: "https://github.com/recoupable/org-foo-id-1", - auth: { ...baseAuth, orgId: "org-1" }, + auth: { ...baseAuth, orgId }, }); expect(result).toEqual({ @@ -32,29 +33,35 @@ describe("resolveSessionCloneUrl", () => { expect(ensurePersonalRepo).not.toHaveBeenCalled(); }); - it("returns cloneUrl=null when no body cloneUrl and an org is bound (current narrow scope)", async () => { + it("provisions the user's workspace when no body cloneUrl and no org", async () => { + vi.mocked(ensurePersonalRepo).mockResolvedValue(`https://github.com/recoupable/${accountId}`); + const result = await resolveSessionCloneUrl({ bodyCloneUrl: undefined, - auth: { ...baseAuth, orgId: "org-1" }, + auth: baseAuth, }); - expect(result).toEqual({ ok: true, cloneUrl: null }); - expect(ensurePersonalRepo).not.toHaveBeenCalled(); + expect(result).toEqual({ + ok: true, + cloneUrl: `https://github.com/recoupable/${accountId}`, + }); + expect(ensurePersonalRepo).toHaveBeenCalledWith({ accountId }); }); - it("provisions a personal repo when no body cloneUrl and no org", async () => { - vi.mocked(ensurePersonalRepo).mockResolvedValue(`https://github.com/recoupable/${accountId}`); + it("provisions the ORG's workspace when no body cloneUrl and an org is bound", async () => { + vi.mocked(ensurePersonalRepo).mockResolvedValue(`https://github.com/recoupable/${orgId}`); const result = await resolveSessionCloneUrl({ bodyCloneUrl: undefined, - auth: baseAuth, + auth: { ...baseAuth, orgId }, }); expect(result).toEqual({ ok: true, - cloneUrl: `https://github.com/recoupable/${accountId}`, + cloneUrl: `https://github.com/recoupable/${orgId}`, }); - expect(ensurePersonalRepo).toHaveBeenCalledWith({ accountId }); + // Keyed on orgId (organizations are accounts), not the caller's accountId. + expect(ensurePersonalRepo).toHaveBeenCalledWith({ accountId: orgId }); }); it("returns an error when ensurePersonalRepo fails", async () => { diff --git a/lib/sessions/resolveSessionCloneUrl.ts b/lib/sessions/resolveSessionCloneUrl.ts index e67348272..be1fd8bf3 100644 --- a/lib/sessions/resolveSessionCloneUrl.ts +++ b/lib/sessions/resolveSessionCloneUrl.ts @@ -16,20 +16,15 @@ export interface ResolveSessionCloneUrlResult { /** * Determine the final `clone_url` for a new session row. * - * 1. If the body explicitly provided `cloneUrl`, use it. Both chat - * (active-org case) and sandbox.recoupable.com construct the org - * URL client-side and send it through. - * 2. If the body omitted `cloneUrl` AND the caller has no `orgId` - * bound to their auth context, treat this as a personal session - * and call `ensurePersonalRepo` so the account's - * `recoupable/` repo exists (with a legacy-rename - * step for accounts that had `-` workspaces - * under the old naming convention) before `POST /api/sandbox` - * tries to clone it. - * 3. Otherwise (no `cloneUrl`, but an org is bound), return `null` - * so the session row stores no `clone_url`. Org-repo URL - * derivation stays a client-side concern for now to keep this PR - * focused on personal-repo provisioning. + * 1. If the body explicitly provided `cloneUrl`, use it verbatim + * (the caller already decided which repo this session targets). + * 2. Otherwise, ensure a `recoupable/` workspace exists + * and use that URL. The provisioning is the same for personal + * sessions and org-bound sessions because organizations are + * themselves accounts (`auth.orgId` IS an account_id — see + * `account_organization_ids.organization` joining `accounts`). + * When an org is bound, the workspace is keyed on `auth.orgId`; + * otherwise on the user's own `auth.accountId`. */ export async function resolveSessionCloneUrl(params: { bodyCloneUrl: string | undefined; @@ -41,17 +36,16 @@ export async function resolveSessionCloneUrl(params: { return { ok: true, cloneUrl: bodyCloneUrl }; } - if (auth.orgId) { - return { ok: true, cloneUrl: null }; - } - - const cloneUrl = await ensurePersonalRepo({ accountId: auth.accountId }); + const workspaceAccountId = auth.orgId ?? auth.accountId; + const cloneUrl = await ensurePersonalRepo({ + accountId: workspaceAccountId, + }); if (!cloneUrl) { return { ok: false, cloneUrl: null, - error: "Failed to provision personal repository", + error: "Failed to provision workspace repository", }; } From 019229f903881d65b779766c15a563fa9ddaa948 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 13:51:52 -0500 Subject: [PATCH 09/12] fix(migrate): match empty-slug - repos too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The regex used .+ for the slug, requiring at least 1 char before the dash. Accounts that had no display name at repo-creation time produced - names (literal leading dash, empty kebab), and the first migration run skipped those as "non-workspace". 6 leading-dash repos turned up in the recoupable org after the first apply pass — 5 of them collided with already-renamed siblings (their losers were deleted manually); 1 was free and renamed. Changed .+ to .* so future runs of this script catch empty-slug names. Bare names still don't match (no separator before the UUID). Co-Authored-By: Claude Opus 4.7 --- scripts/migrate-workspace-repo-names.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/migrate-workspace-repo-names.ts b/scripts/migrate-workspace-repo-names.ts index a67abbd1f..475d50fd4 100644 --- a/scripts/migrate-workspace-repo-names.ts +++ b/scripts/migrate-workspace-repo-names.ts @@ -28,8 +28,16 @@ import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; +/** + * Matches `-` workspace repo names where `` may be + * empty — accounts that had no display name at the time the repo was + * created produced `-` (literal leading dash, empty kebab) and + * must be migrated alongside the populated-slug repos. Bare `` + * names (post-migration) deliberately do NOT match: the required `-` + * separator before the UUID is absent. + */ const LEGACY_PATTERN = - /^(?.+)-(?[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})$/i; + /^(?.*)-(?[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})$/i; interface OrgRepo { name: string; From 8576e2e80b07a1cf149a33e0236097222ebc6f8d Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 13:58:02 -0500 Subject: [PATCH 10/12] revert: createRepository back to private: true MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Smoke test of the api PR's preview surfaced an inconsistency — the 153 legacy workspace repos in the recoupable org are all private (created by old open-agents code with private: true), but my earlier review-feedback change set new repos to public. Per Sweet's follow-up, flip back to private so the entire fleet stays uniform. Co-Authored-By: Claude Opus 4.7 --- lib/github/__tests__/createRepository.test.ts | 4 ++-- lib/github/createRepository.ts | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/github/__tests__/createRepository.test.ts b/lib/github/__tests__/createRepository.test.ts index bb55d4a4a..868bc2318 100644 --- a/lib/github/__tests__/createRepository.test.ts +++ b/lib/github/__tests__/createRepository.test.ts @@ -33,7 +33,7 @@ describe("createRepository", () => { expect(fetchSpy).not.toHaveBeenCalled(); }); - it("POSTs to /orgs/recoupable/repos with hard-coded private=false + auto_init=true", async () => { + it("POSTs to /orgs/recoupable/repos with hard-coded private=true + auto_init=true", async () => { const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( new Response( JSON.stringify({ @@ -60,7 +60,7 @@ describe("createRepository", () => { expect(init.method).toBe("POST"); expect(JSON.parse(init.body as string)).toEqual({ name: "id-1", - private: false, + private: true, auto_init: true, }); }); diff --git a/lib/github/createRepository.ts b/lib/github/createRepository.ts index 8bfeba459..cdfe1cfcd 100644 --- a/lib/github/createRepository.ts +++ b/lib/github/createRepository.ts @@ -20,8 +20,9 @@ export interface CreateRepositoryResult { * Hard-coded conventions (per PR #618 review — KISS / YAGNI): * - owner = `recoupable` (no other owner makes sense; see * `RECOUPABLE_GITHUB_OWNER`). - * - private = false (workspace repos are public so future surfaces - * can clone without per-caller auth). + * - private = true (matches the 153 legacy workspace repos that + * pre-date this code path — keeps the fleet uniform; clones from + * sandboxes auth via the GITHUB_TOKEN service token). * - description = none (GitHub doesn't render anything meaningful * for these per-account repos). * - token = read once from `GITHUB_TOKEN` via @@ -62,7 +63,7 @@ export async function createRepository(params: { name: string }): Promise Date: Tue, 26 May 2026 14:14:47 -0500 Subject: [PATCH 11/12] chore: drop migration script (already applied to prod GitHub) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/migrate-workspace-repo-names.ts was a one-time backfill — ran against the recoupable org on 2026-05-26, renamed every legacy - workspace repo to bare , then verified zero pending via final dry-run. Keeping it in the codebase forever would be dead weight (per the same KISS principle we applied to findLegacyAccountRepo + renameRepository). Updated the ensurePersonalRepo docstring to reflect that the migration is historical, not an ongoing reference. Co-Authored-By: Claude Opus 4.7 --- lib/recoupable/ensurePersonalRepo.ts | 9 +- scripts/migrate-workspace-repo-names.ts | 176 ------------------------ 2 files changed, 5 insertions(+), 180 deletions(-) delete mode 100644 scripts/migrate-workspace-repo-names.ts diff --git a/lib/recoupable/ensurePersonalRepo.ts b/lib/recoupable/ensurePersonalRepo.ts index eddbce68a..69a854bfc 100644 --- a/lib/recoupable/ensurePersonalRepo.ts +++ b/lib/recoupable/ensurePersonalRepo.ts @@ -16,10 +16,11 @@ import { RECOUPABLE_GITHUB_OWNER } from "./githubOwner"; * consume (the clone URL); owner / repo name are trivially * recoverable from the URL if ever needed. * - * Legacy `-` repos are renamed once, ahead of time, - * by `scripts/migrate-workspace-repo-names.ts`. After that one-shot - * migration there's no legacy repo to find at runtime, so no - * rename-on-demand branch lives here. + * Legacy `-` repos were renamed to the canonical + * `` shape in a one-time backfill (see PR #618), so the + * runtime never needs to look for a legacy name — every workspace + * either already lives at `recoupable/` or doesn't exist + * yet. */ export async function ensurePersonalRepo(params: { accountId: string }): Promise { const repoName = params.accountId; diff --git a/scripts/migrate-workspace-repo-names.ts b/scripts/migrate-workspace-repo-names.ts deleted file mode 100644 index 475d50fd4..000000000 --- a/scripts/migrate-workspace-repo-names.ts +++ /dev/null @@ -1,176 +0,0 @@ -/** - * One-time migration: rename every `recoupable/*-` workspace - * repository to the canonical `recoupable/` convention. - * - * - Scans all repos under the `recoupable` GitHub org, paginated. - * - For each repo whose name matches `^.+-$`, extracts the - * trailing UUID and renames the repo to just ``. - * - Skips when the target name already exists (idempotent re-run - * safe — first invocation handles the rename, subsequent ones are - * no-ops). - * - Defaults to dry-run; pass `--apply` to actually perform renames. - * - * GitHub auto-redirects the old name for both git clones and the REST - * API after a rename, so `sessions.clone_url` rows that still - * reference the old name keep working without any DB backfill. - * - * Run via: - * pnpm dotenv -e .env.local -- pnpm tsx scripts/migrate-workspace-repo-names.ts - * pnpm dotenv -e .env.local -- pnpm tsx scripts/migrate-workspace-repo-names.ts --apply - * - * Requires `GITHUB_TOKEN` with `admin:repo` scope on the `recoupable` - * org in the environment (or `.env.local`). - * - * The PATCH-rename request lives inline (rather than as a `lib/github/*` - * helper) because this script is its only caller — once it runs every - * legacy repo is renamed and there is no runtime caller to reuse. - */ - -import { RECOUPABLE_GITHUB_OWNER } from "@/lib/recoupable/githubOwner"; - -/** - * Matches `-` workspace repo names where `` may be - * empty — accounts that had no display name at the time the repo was - * created produced `-` (literal leading dash, empty kebab) and - * must be migrated alongside the populated-slug repos. Bare `` - * names (post-migration) deliberately do NOT match: the required `-` - * separator before the UUID is absent. - */ -const LEGACY_PATTERN = - /^(?.*)-(?[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})$/i; - -interface OrgRepo { - name: string; -} - -async function listAllOrgRepos(token: string): Promise { - const repos: OrgRepo[] = []; - let page = 1; - while (true) { - const response = await fetch( - `https://api.github.com/orgs/${RECOUPABLE_GITHUB_OWNER}/repos?per_page=100&page=${page}`, - { - method: "GET", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${token}`, - "X-GitHub-Api-Version": "2022-11-28", - }, - }, - ); - - if (!response.ok) { - const body = await response.text().catch(() => ""); - throw new Error(`Failed to list org repos at page ${page}: ${response.status} ${body}`); - } - - const batch = (await response.json()) as OrgRepo[]; - if (batch.length === 0) break; - repos.push(...batch); - if (batch.length < 100) break; - page += 1; - } - return repos; -} - -async function renameRepo( - from: string, - to: string, - token: string, -): Promise<{ ok: boolean; error?: string }> { - try { - const response = await fetch( - `https://api.github.com/repos/${RECOUPABLE_GITHUB_OWNER}/${from}`, - { - method: "PATCH", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${token}`, - "Content-Type": "application/json", - "X-GitHub-Api-Version": "2022-11-28", - }, - body: JSON.stringify({ name: to }), - }, - ); - if (response.status === 200) return { ok: true }; - const body = await response.text().catch(() => ""); - return { ok: false, error: `${response.status} ${body}` }; - } catch (error) { - return { ok: false, error: (error as Error).message }; - } -} - -async function main(): Promise { - const apply = process.argv.includes("--apply"); - const token = process.env.GITHUB_TOKEN; - if (!token) { - console.error("GITHUB_TOKEN must be set (admin:repo on recoupable org)."); - process.exit(1); - } - - console.log(`[migrate] mode=${apply ? "APPLY" : "dry-run"} owner=${RECOUPABLE_GITHUB_OWNER}`); - - const repos = await listAllOrgRepos(token); - console.log(`[migrate] discovered ${repos.length} repos`); - - const existingNames = new Set(repos.map(r => r.name)); - - type Action = - | { type: "rename"; from: string; to: string } - | { type: "skip-target-exists"; from: string; to: string } - | { type: "skip-not-workspace"; name: string }; - const actions: Action[] = []; - - for (const { name } of repos) { - const match = name.match(LEGACY_PATTERN); - if (!match?.groups) { - actions.push({ type: "skip-not-workspace", name }); - continue; - } - const uuid = match.groups.uuid.toLowerCase(); - if (existingNames.has(uuid)) { - actions.push({ type: "skip-target-exists", from: name, to: uuid }); - continue; - } - actions.push({ type: "rename", from: name, to: uuid }); - } - - const toRename = actions.filter( - (a): a is { type: "rename"; from: string; to: string } => a.type === "rename", - ); - const skipTarget = actions.filter(a => a.type === "skip-target-exists"); - const skipNotWorkspace = actions.filter(a => a.type === "skip-not-workspace"); - - console.log( - `[migrate] plan: rename=${toRename.length}, skip-target-exists=${skipTarget.length}, skip-not-workspace=${skipNotWorkspace.length}`, - ); - - for (const a of toRename) { - console.log(` rename: ${a.from} -> ${a.to}`); - } - - if (!apply) { - console.log("[migrate] dry-run — no changes made. Re-run with --apply."); - return; - } - - let succeeded = 0; - let failed = 0; - for (const a of toRename) { - const result = await renameRepo(a.from, a.to, token); - if (result.ok) { - succeeded += 1; - console.log(` ok: ${a.from} -> ${a.to}`); - } else { - failed += 1; - console.error(` fail: ${a.from} -> ${a.to} (${result.error ?? "?"})`); - } - } - - console.log( - `[migrate] done. renamed=${succeeded} failed=${failed} skipped=${skipTarget.length + skipNotWorkspace.length}`, - ); - if (failed > 0) process.exit(1); -} - -void main(); From 7e1b9e866908de9e6628e4e243372eb456c29033 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 26 May 2026 14:19:26 -0500 Subject: [PATCH 12/12] prune: slim CreateRepositoryResult to {success, repoUrl, error} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Sweet's review — cloneUrl, owner, repoName are all derivable from repoUrl (cloneUrl = repoUrl + ".git" which git also accepts as repoUrl; owner = "recoupable"; repoName = trailing path segment). Dropped them from the interface and the parse + return shape. ensurePersonalRepo now consumes created.repoUrl directly. The existing-repo branch already returned repoUrl, so the function is now fully consistent on the no-".git" URL form. Co-Authored-By: Claude Opus 4.7 --- lib/github/__tests__/createRepository.test.ts | 6 ------ lib/github/createRepository.ts | 20 ++----------------- .../__tests__/ensurePersonalRepo.test.ts | 7 ++----- lib/recoupable/ensurePersonalRepo.ts | 4 ++-- 4 files changed, 6 insertions(+), 31 deletions(-) diff --git a/lib/github/__tests__/createRepository.test.ts b/lib/github/__tests__/createRepository.test.ts index 868bc2318..ee9e013ac 100644 --- a/lib/github/__tests__/createRepository.test.ts +++ b/lib/github/__tests__/createRepository.test.ts @@ -38,9 +38,6 @@ describe("createRepository", () => { new Response( JSON.stringify({ html_url: "https://github.com/recoupable/id-1", - clone_url: "https://github.com/recoupable/id-1.git", - owner: { login: "recoupable" }, - name: "id-1", }), { status: 201, headers: { "content-type": "application/json" } }, ), @@ -51,9 +48,6 @@ describe("createRepository", () => { expect(result).toEqual({ success: true, repoUrl: "https://github.com/recoupable/id-1", - cloneUrl: "https://github.com/recoupable/id-1.git", - owner: "recoupable", - repoName: "id-1", }); const [url, init] = fetchSpy.mock.calls[0] as [string, RequestInit]; expect(url).toBe("https://api.github.com/orgs/recoupable/repos"); diff --git a/lib/github/createRepository.ts b/lib/github/createRepository.ts index cdfe1cfcd..c6eebaf03 100644 --- a/lib/github/createRepository.ts +++ b/lib/github/createRepository.ts @@ -5,11 +5,6 @@ export interface CreateRepositoryResult { success: boolean; /** GitHub UI URL (`html_url`). */ repoUrl?: string; - /** Git clone URL (`clone_url`). */ - cloneUrl?: string; - /** Owner login (always `recoupable`). */ - owner?: string; - repoName?: string; /** Human-readable error message; only set when `success` is false. */ error?: string; } @@ -69,19 +64,8 @@ export async function createRepository(params: { name: string }): Promise { expect(createRepository).not.toHaveBeenCalled(); }); - it("creates a fresh repo when none exists and returns its clone URL", async () => { + it("creates a fresh repo when none exists and returns its URL", async () => { vi.mocked(repositoryExists).mockResolvedValue(false); vi.mocked(createRepository).mockResolvedValue({ success: true, - cloneUrl: `https://github.com/recoupable/${accountId}.git`, repoUrl: `https://github.com/recoupable/${accountId}`, - owner: "recoupable", - repoName: accountId, }); const result = await ensurePersonalRepo({ accountId }); expect(createRepository).toHaveBeenCalledWith({ name: accountId }); - expect(result).toBe(`https://github.com/recoupable/${accountId}.git`); + expect(result).toBe(`https://github.com/recoupable/${accountId}`); }); it("returns null when creation outright fails", async () => { diff --git a/lib/recoupable/ensurePersonalRepo.ts b/lib/recoupable/ensurePersonalRepo.ts index 69a854bfc..0b60f3bcc 100644 --- a/lib/recoupable/ensurePersonalRepo.ts +++ b/lib/recoupable/ensurePersonalRepo.ts @@ -39,10 +39,10 @@ export async function ensurePersonalRepo(params: { accountId: string }): Promise const created = await createRepository({ name: repoName }); - if (!created.success || !created.cloneUrl) { + if (!created.success || !created.repoUrl) { console.error(`[ensurePersonalRepo] createRepository failed: ${created.error ?? "unknown"}`); return null; } - return created.cloneUrl; + return created.repoUrl; }