From 055939b3528f5f26f448771b12714419df5893b3 Mon Sep 17 00:00:00 2001 From: yaowenc2 Date: Fri, 11 Sep 2026 15:47:17 -0700 Subject: [PATCH] feat(github): connect-repo installs the App from a terminal when the repo's account lacks it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The device flow (#207) linked the terminal's GitHub identity, but with no App installation granted to the org the command could only say "install it in the console". GitHub has no API that installs an App, so the terminal now prints the install page instead and then claims what the person installed, proved by the identity it already linked — the platform's new POST /orgs/:orgId/github/installations/claim (insta-platform #428). - After the caller-scoped listing has no hit, claim with accountLogin = the repo's owner. Granted → list again and connect. Install URL → print it (stderr, so --json stays one document), poll the claim every 5s for up to ten minutes with the device flow's backoff on 429/dropped links, then list again. No reader (--json, a bare pipe) → fail with the URL in hand. Several accounts → name them. - 400 "not linked" on the claim runs the device flow once, then claims again. - 404 on the claim (a backend from before it) keeps today's console messages. - connect-repo's description says so. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GvKBDKEyErsLvNdqErtVoR --- src/commands/github.ts | 92 +++++++++++++++++++++++++++---- src/index.ts | 2 +- test/github-connect.test.ts | 106 +++++++++++++++++++++++++++++++++++- 3 files changed, 188 insertions(+), 12 deletions(-) diff --git a/src/commands/github.ts b/src/commands/github.ts index 15a5a14..7c0964e 100644 --- a/src/commands/github.ts +++ b/src/commands/github.ts @@ -151,11 +151,72 @@ export function canAuthorizeHere(opts: { json?: boolean } = {}): boolean { return !!agentMode() || !!process.stderr.isTTY } -export async function findCallerRepo(api: ApiClient, orgId: string, ref: RepoRef, authorize: typeof authorizeTerminal = authorizeTerminal, canAuthorize = canAuthorizeHere()): Promise<{ installationId: number; repoId: number }> { +const NO_READER = 'this GitHub account is not authorized for InstaCloud yet, and nothing here can read the code GitHub shows — run `insta compute connect-repo` from a terminal, connect the repository from the console, or pass --public for a public repository' + +// The platform answers a claim with exactly one of these: the installation it granted the org, the +// accounts it could have claimed when none was named, or where to install the App when there is nothing. +type ClaimAnswer = { + installation?: { installation_id?: number | string; account_login?: string } + installations?: Array<{ installationId: number; accountLogin: string; accountType: string }> + installUrl?: string +} + +// Ten minutes of waiting, counted in the waits themselves so a test's instant wait still ends it. +const INSTALL_WAIT_SECONDS = 600 + +// GitHub has no API that installs an App: the person does that in a browser, and the platform then +// claims what they installed with the identity the device flow linked — so nothing here needs a redirect. +// Answers whether an installation on the repo's account is granted to the org now; `null` means the +// platform predates the claim route, and the caller falls back to sending the person to the console. +export async function claimInstallation(api: ApiClient, orgId: string, ref: RepoRef, authorize: typeof authorizeTerminal = authorizeTerminal, canAuthorize = canAuthorizeHere(), wait: (s: number) => Promise = sleepSeconds): Promise { + const claim = () => api.request('POST', `/orgs/${encodeURIComponent(orgId)}/github/installations/claim`, { accountLogin: ref.owner }) + let answer: ClaimAnswer + try { + answer = await claim() + } catch (e) { + // The route itself is missing on a backend from before it; any other 404 would have failed the listing first. + if (e instanceof ApiError && e.status === 404) return null + if (!needsAuthorization(e)) throw e + if (!canAuthorize) throw new Error(NO_READER) + await authorize(api, orgId) + answer = await claim() + } + if (answer.installation) return true + if (answer.installations?.length) { + throw new Error(`the InstaCloud GitHub App is installed on ${answer.installations.map((i) => i.accountLogin).join(', ')} but not on ${ref.owner} — install it there (the account that owns ${ref.owner}/${ref.repo}), or pass --public for a public repository`) + } + if (!answer.installUrl) throw new Error('the GitHub App claim answered with nothing to act on — is the platform up to date?') + if (!canAuthorize) { + throw new Error(`the InstaCloud GitHub App is not installed on ${ref.owner} — install it at ${answer.installUrl} (pick the account ${ref.owner} and include ${ref.repo}), then run this command again, or pass --public for a public repository`) + } + // stderr, not stdout: --json must stay one parseable document. + const say = (line: string) => process.stderr.write(line + '\n') + say(`the InstaCloud GitHub App is not installed on ${ref.owner} — install it once:`) + say(` open ${answer.installUrl}`) + say(` (pick the account ${ref.owner} and include ${ref.repo} in the repositories it can reach)`) + say('waiting for the installation… (ctrl-c to abort)') + let interval = pollDelay(5) + for (let waited = 0; waited < INSTALL_WAIT_SECONDS; waited += interval) { + await wait(interval) + try { + answer = await claim() + } catch (e) { + // A dropped link, or the per-IP limiter, must not end an installation the person may be one click from finishing. + if (e instanceof ApiError && e.status !== 429) throw e + interval = pollDelay(interval + 5) + continue + } + if (answer.installation) return true + } + throw new Error('the GitHub App installation was not completed in time — run the command again') +} + +export async function findCallerRepo(api: ApiClient, orgId: string, ref: RepoRef, authorize: typeof authorizeTerminal = authorizeTerminal, canAuthorize = canAuthorizeHere(), wait: (s: number) => Promise = sleepSeconds): Promise<{ installationId: number; repoId: number }> { if (!orgId) throw new Error('this directory is linked without an org — set INSTA_ORG_ID alongside INSTA_PROJECT_ID, or link it with `insta project link`') + const list = async () => (await api.request<{ repos?: RepoRow[] }>('POST', `/orgs/${encodeURIComponent(orgId)}/github/repos`, {})).repos ?? [] let repos: RepoRow[] try { - repos = (await api.request<{ repos?: RepoRow[] }>('POST', `/orgs/${encodeURIComponent(orgId)}/github/repos`, {})).repos ?? [] + repos = await list() } catch (e) { // Two 403s carry an action; a third kind would be guessed at, so it is rethrown as the platform put it. if (e instanceof ApiError && e.status === 403 && /unclassified_agent_action/.test(e.message)) { @@ -165,17 +226,28 @@ export async function findCallerRepo(api: ApiClient, orgId: string, ref: RepoRef throw new Error('connecting a repository needs the org admin role — ask an admin to connect it, or pass --public for a public repository') } if (!needsAuthorization(e)) throw e - if (!canAuthorize) { - throw new Error('this GitHub account is not authorized for InstaCloud yet, and nothing here can read the code GitHub shows — run `insta compute connect-repo` from a terminal, connect the repository from the console, or pass --public for a public repository') - } + if (!canAuthorize) throw new Error(NO_READER) repos = await authorize(api, orgId) } const whole = (n: unknown) => n !== null && n !== '' && Number.isInteger(Number(n)) && Number(n) > 0 - const hit = repos.find((r) => r.owner.toLowerCase() === ref.owner.toLowerCase() && r.repo.toLowerCase() === ref.repo.toLowerCase()) - if (hit && whole(hit.installationId) && whole(hit.id)) return { installationId: Number(hit.installationId), repoId: Number(hit.id) } - if (hit) throw new Error(`${ref.owner}/${ref.repo} came back without an installation to build it through — reconnect GitHub in the console, or pass --public for a public repository`) - if (repos.length === 0) throw new Error('the InstaCloud GitHub App reaches none of your repositories — install it on the account that owns this one (console → Add Service → GitHub Repo → Connect GitHub), or pass --public for a public repository') - throw new Error(`${ref.owner}/${ref.repo} is not one your GitHub account can reach through the App — grant the App access to it on GitHub, or pass --public for a public repository`) + const pick = (rows: RepoRow[]) => { + const hit = rows.find((r) => r.owner.toLowerCase() === ref.owner.toLowerCase() && r.repo.toLowerCase() === ref.repo.toLowerCase()) + if (hit && whole(hit.installationId) && whole(hit.id)) return { installationId: Number(hit.installationId), repoId: Number(hit.id) } + if (hit) throw new Error(`${ref.owner}/${ref.repo} came back without an installation to build it through — reconnect GitHub in the console, or pass --public for a public repository`) + return null + } + const found = pick(repos) + if (found) return found + // Unreachable means the App is not installed on the repo's account, or is installed without this repo. + // The claim tells the two apart: it grants an installation the caller administers, or says where to install one. + const claimed = await claimInstallation(api, orgId, ref, authorize, canAuthorize, wait) + if (claimed === null) { + if (repos.length === 0) throw new Error('the InstaCloud GitHub App reaches none of your repositories — install it on the account that owns this one (console → Add Service → GitHub Repo → Connect GitHub), or pass --public for a public repository') + throw new Error(`${ref.owner}/${ref.repo} is not one your GitHub account can reach through the App — grant the App access to it on GitHub, or pass --public for a public repository`) + } + const again = pick(await list()) + if (again) return again + throw new Error(`${ref.owner}/${ref.repo} is not one the App's installation on ${ref.owner} can reach — grant the App access to it on GitHub (Settings → Applications → InstaCloud → Repository access), or pass --public for a public repository`) } async function targetService(api: ApiClient, projectId: string, branch: string | undefined, serviceName: string | undefined) { diff --git a/src/index.ts b/src/index.ts index 0f9aab7..a107793 100644 --- a/src/index.ts +++ b/src/index.ts @@ -265,7 +265,7 @@ const execCmd = compute.command('exec [service]').description("Run a one-shot co for (const [flags, description] of computeCmd.EXEC_OPTIONS) execCmd.option(flags, description) compute.command('repo [service]').description('Show what a compute service deploys from: the image it runs, or the GitHub repository — owner/repo, the branch it builds, root directory, which paths a push must change to redeploy it, and whether pushes redeploy it at all') .option('--json').option('--branch ', 'branch (default: current)').action(guard((service, o) => githubCmd.computeRepo(service, o))) -compute.command('connect-repo [service]').description("Connect a GitHub repository to an EXISTING compute service: the repo is built (its Dockerfile, or nixpacks when there is none) and deployed into that service, and every later push to the tracked repository branch redeploys it. The repo must be one your own GitHub account can reach through the InstaCloud App, or be public; the first connect from a terminal prints a GitHub URL and a code to authorize it once. Build and start commands come from detection and cannot be set. Connecting again replaces the service's current source") +compute.command('connect-repo [service]').description("Connect a GitHub repository to an EXISTING compute service: the repo is built (its Dockerfile, or nixpacks when there is none) and deployed into that service, and every later push to the tracked repository branch redeploys it. The repo must be one your own GitHub account can reach through the InstaCloud App, or be public; the first connect from a terminal prints a GitHub URL and a code to authorize it once, and when the App is not installed on the account that owns the repo, the URL to install it, then waits for that. Build and start commands come from detection and cannot be set. Connecting again replaces the service's current source") .option('--public', 'the repo is public and no GitHub App installation is needed (deploys are manual; pushes cannot redeploy)') .option('--root-dir ', 'the directory of the repo to build (a monorepo with several deployable directories lists them and exits 1 without it)') .option('--repo-branch ', "the repository branch to build (default: the repo's default branch)") diff --git a/test/github-connect.test.ts b/test/github-connect.test.ts index b576fdb..84692fc 100644 --- a/test/github-connect.test.ts +++ b/test/github-connect.test.ts @@ -129,7 +129,9 @@ describe('findCallerRepo', () => { } }) as unknown as ApiClient const ref = { owner: 'acme', repo: 'app' } const never = async () => { throw new Error('must not authorize') } - const listed = (repos: unknown[]) => fake({ '/orgs/org_1/github/repos': { repos } }) + // A backend from before the claim route (404 there): the messages these assert are the console hand-off + // ones it keeps. What a claim changes is covered in the describe below. + const listed = (repos: unknown[]) => fake({ '/orgs/org_1/github/repos': { repos }, '/orgs/org_1/github/installations/claim': new ApiError(404, 'Route POST:/orgs/org_1/github/installations/claim not found', {}) }) it('answers the installation the repo came from, as numbers, matching case-insensitively', async () => { await expect(findCallerRepo(listed([{ id: 42, owner: 'Acme', repo: 'App', installationId: 7 }]), 'org_1', ref, never)).resolves.toEqual({ installationId: 7, repoId: 42 }) }) @@ -175,6 +177,108 @@ describe('findCallerRepo', () => { it('an org-less link is refused before any request', async () => { await expect(findCallerRepo(fake({}), '', ref, never)).rejects.toThrow(/INSTA_ORG_ID/) }) + + // The claim route answers differently as the person acts at GitHub, so these fakes answer per call: + // each path's answers are consumed in order, and the last one repeats. + describe('an unreachable repo claims the App installation on its account', () => { + afterEach(() => vi.restoreAllMocks()) + const CLAIM = '/orgs/org_1/github/installations/claim' + const LIST = '/orgs/org_1/github/repos' + const hit = { id: 42, owner: 'acme', repo: 'app', installationId: 7 } + const url = 'https://github.com/apps/insta-cloud/installations/new' + const script = (answers: Record) => { + const calls: Array<{ path: string; body: unknown }> = [] + const said: string[] = [] + const waits: number[] = [] + const api = { request: async (_m: string, path: string, body?: unknown) => { + const key = path.split('?')[0]! + const queue = answers[key] + if (!queue?.length) throw new Error(`unexpected path ${path}`) + calls.push({ path: key, body }) + const a = queue.length > 1 ? queue.shift() : queue[0] + if (a instanceof Error) throw a + return a + } } as unknown as ApiClient + // restoreAllMocks leaves the spied method a mock, so a "spy once" guard would skip every test after + // the first; spyOn on an already spied method reuses the spy, so a second fake in one test is fine. + vi.spyOn(process.stderr, 'write').mockImplementation((l: any) => { said.push(String(l)); return true }) + const claims = () => calls.filter((c) => c.path === CLAIM) + return { api, calls, claims, said, waits, wait: async (s: number) => { waits.push(s) } } + } + it('a claim that lands lists again and answers from the installation just granted', async () => { + const d = script({ [LIST]: [{ repos: [] }, { repos: [hit] }], [CLAIM]: [{ installation: { installation_id: 7, account_login: 'acme' } }] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, true, d.wait)).resolves.toEqual({ installationId: 7, repoId: 42 }) + // Named by the repo's owner: the claim must not grant whatever else this person administers. + expect(d.claims().map((c) => c.body)).toEqual([{ accountLogin: 'acme' }]) + expect(d.waits).toEqual([]) + }) + it('a repo reachable through another installation still claims — the account is what is missing', async () => { + const d = script({ [LIST]: [{ repos: [{ id: 9, owner: 'other', repo: 'x', installationId: 3 }] }, { repos: [hit] }], [CLAIM]: [{ installation: { installation_id: 7 } }] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, true, d.wait)).resolves.toEqual({ installationId: 7, repoId: 42 }) + }) + it('granted, but the installation leaves this repo out: say where on GitHub to include it', async () => { + const d = script({ [LIST]: [{ repos: [] }], [CLAIM]: [{ installation: { installation_id: 7 } }] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, true, d.wait)).rejects.toThrow(/installation on acme can reach[\s\S]*Repository access[\s\S]*--public/) + }) + it('with nothing that can read the URL, it fails with the URL in hand instead of waiting', async () => { + const d = script({ [LIST]: [{ repos: [] }], [CLAIM]: [{ installUrl: url }] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, false, d.wait)).rejects.toThrow(new RegExp(`not installed on acme[\\s\\S]*${url}[\\s\\S]*run this command again[\\s\\S]*--public`)) + expect(d.waits).toEqual([]) + }) + it('a terminal prints the install URL, polls the claim until it lands, then lists again', async () => { + const d = script({ [LIST]: [{ repos: [] }, { repos: [hit] }], [CLAIM]: [{ installUrl: url }, { installUrl: url }, { installation: { installation_id: 7 } }] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, true, d.wait)).resolves.toEqual({ installationId: 7, repoId: 42 }) + // The URL IS the flow: without it on screen there is nothing for the person to do. + expect(d.said.join('')).toContain(url) + expect(d.said.join('')).toContain('not installed on acme') + expect(d.waits).toEqual([5, 5]) + expect(d.claims()).toHaveLength(3) + }) + it('a rate-limited or dropped poll backs off instead of ending the wait', async () => { + const d = script({ [LIST]: [{ repos: [] }, { repos: [hit] }], [CLAIM]: [{ installUrl: url }, new ApiError(429, 'HTTP 429', {}), new TypeError('socket hang up'), { installation: { installation_id: 7 } }] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, true, d.wait)).resolves.toEqual({ installationId: 7, repoId: 42 }) + expect(d.waits).toEqual([5, 10, 15]) + }) + it('a poll that fails for a real reason ends the wait with that reason', async () => { + const d = script({ [LIST]: [{ repos: [] }], [CLAIM]: [{ installUrl: url }, new ApiError(403, 'requires admin role', {})] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, true, d.wait)).rejects.toThrow(/requires admin role/) + }) + it('gives up after ten minutes of nobody installing', async () => { + const d = script({ [LIST]: [{ repos: [] }], [CLAIM]: [{ installUrl: url }] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, true, d.wait)).rejects.toThrow(/not completed in time/) + expect(d.waits.reduce((a, b) => a + b, 0)).toBeGreaterThanOrEqual(600) + expect(d.waits.length).toBeLessThanOrEqual(120) + }) + it('a backend without the claim route keeps the console messages, for an empty list and a partial one', async () => { + const missing = new ApiError(404, 'Route POST:/orgs/org_1/github/installations/claim not found', {}) + const empty = script({ [LIST]: [{ repos: [] }], [CLAIM]: [missing] }) + await expect(findCallerRepo(empty.api, 'org_1', ref, never, true, empty.wait)).rejects.toThrow(/install it on the account[\s\S]*console/) + const partial = script({ [LIST]: [{ repos: [{ id: 9, owner: 'acme', repo: 'other', installationId: 7 }] }], [CLAIM]: [missing] }) + await expect(findCallerRepo(partial.api, 'org_1', ref, never, true, partial.wait)).rejects.toThrow(/not one your GitHub account can reach[\s\S]*--public/) + expect(empty.waits).toEqual([]) + }) + it('a claim refused as unlinked authorizes once, then claims again', async () => { + let authorized = 0 + const authorize = async () => { authorized++; return [] } + const d = script({ [LIST]: [{ repos: [] }, { repos: [hit] }], [CLAIM]: [new ApiError(400, 'your github account is not linked — authorize github, then claim again', {}), { installation: { installation_id: 7 } }] }) + await expect(findCallerRepo(d.api, 'org_1', ref, authorize, true, d.wait)).resolves.toEqual({ installationId: 7, repoId: 42 }) + expect(authorized).toBe(1) + expect(d.claims()).toHaveLength(2) + }) + it('a claim refused as unlinked with no reader is the old fast failure', async () => { + const d = script({ [LIST]: [{ repos: [] }], [CLAIM]: [new ApiError(400, 'your github account is not linked — authorize github, then claim again', {})] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, false, d.wait)).rejects.toThrow(/nothing here can read the code/) + }) + it('installations on other accounts only are named, with where the repo lives', async () => { + const d = script({ [LIST]: [{ repos: [] }], [CLAIM]: [{ installations: [{ installationId: 3, accountLogin: 'someoneelse', accountType: 'User' }] }] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, true, d.wait)).rejects.toThrow(/installed on someoneelse but not on acme[\s\S]*--public/) + }) + it('an answer with nothing to act on is a platform mismatch, not a silent wait', async () => { + const d = script({ [LIST]: [{ repos: [] }], [CLAIM]: [{}] }) + await expect(findCallerRepo(d.api, 'org_1', ref, never, true, d.wait)).rejects.toThrow(/nothing to act on/) + expect(d.waits).toEqual([]) + }) + }) }) describe('canAuthorizeHere', () => {