diff --git a/src/app/api/gigs/[id]/invoice/invoice-amount-e2e.test.ts b/src/app/api/gigs/[id]/invoice/invoice-amount-e2e.test.ts index 49d60440..e9886780 100644 --- a/src/app/api/gigs/[id]/invoice/invoice-amount-e2e.test.ts +++ b/src/app/api/gigs/[id]/invoice/invoice-amount-e2e.test.ts @@ -19,6 +19,7 @@ vi.mock("@/lib/coinpayportal", () => ({ vi.mock("@/lib/coinpay-oauth", () => ({ getConnectedCoinpayAccessToken: vi.fn(), + getCoinpayLink: vi.fn(), })); vi.mock("@/lib/email", () => ({ @@ -152,7 +153,7 @@ import { getCoinpayGlobalWalletTokens, preferredCoinToPaymentCurrency, } from "@/lib/coinpayportal"; -import { getConnectedCoinpayAccessToken } from "@/lib/coinpay-oauth"; +import { getConnectedCoinpayAccessToken, getCoinpayLink } from "@/lib/coinpay-oauth"; function req(body?: unknown) { return { json: () => Promise.resolve(body) } as any; @@ -166,6 +167,7 @@ describe("invoice money path (sats gig, end to end)", () => { (v: string | null) => v?.toLowerCase() || null ); (getConnectedCoinpayAccessToken as any).mockResolvedValue("token"); + (getCoinpayLink as any).mockResolvedValue({ state: "connected", accessToken: "token" }); (getCoinpayGlobalWalletTokens as any).mockResolvedValue([ { currency: "sol", cryptocurrency: "SOL", label: "Solana", address: SOL_ADDRESS }, ]); diff --git a/src/app/api/gigs/[id]/invoice/route.test.ts b/src/app/api/gigs/[id]/invoice/route.test.ts index 7811d753..ba5d02dd 100644 --- a/src/app/api/gigs/[id]/invoice/route.test.ts +++ b/src/app/api/gigs/[id]/invoice/route.test.ts @@ -14,6 +14,7 @@ vi.mock("@/lib/coinpayportal", () => ({ vi.mock("@/lib/coinpay-oauth", () => ({ getConnectedCoinpayAccessToken: vi.fn(), + getCoinpayLink: vi.fn(), })); vi.mock("@/lib/auth/get-user", () => ({ @@ -55,7 +56,7 @@ import { getCoinpayGlobalWalletTokens, resolveSupportedPaymentCurrency, } from "@/lib/coinpayportal"; -import { getConnectedCoinpayAccessToken } from "@/lib/coinpay-oauth"; +import { getConnectedCoinpayAccessToken, getCoinpayLink } from "@/lib/coinpay-oauth"; import { invoiceReceivedEmail, sendEmail } from "@/lib/email"; import { getPullRequestMergeState } from "@/lib/github-app"; @@ -149,6 +150,10 @@ describe("POST /api/gigs/[id]/invoice", () => { beforeEach(() => { vi.clearAllMocks(); (getConnectedCoinpayAccessToken as any).mockResolvedValue("coinpay-access-token"); + (getCoinpayLink as any).mockResolvedValue({ + state: "connected", + accessToken: "coinpay-access-token", + }); (getCoinpayGlobalWalletTokens as any).mockResolvedValue([ { currency: "sol", @@ -245,6 +250,82 @@ describe("POST /api/gigs/[id]/invoice", () => { expect(res.status).toBe(400); }); + /** + * "Connect" and "reconnect" are different instructions. + * + * A worker whose CoinPay link predates the wallet:read scope is connected and + * cannot invoice. Telling them to connect describes something they have + * already done, so they check, see a connection, and try again: a loop with + * no exit. #553 taught the connections page the difference; an agent calling + * this API never sees that page, so the sentence here is its only + * instruction. + */ + describe("when CoinPay cannot be used", () => { + const gig = { id: GIG_ID, title: "Test Gig", poster_id: POSTER_ID, payment_coin: "SOL" }; + const application = { + id: APP_ID, + applicant_id: WORKER_ID, + status: "accepted", + proposed_rate: 150, + }; + + function authAsWorker() { + const sb = mockSupabase({ + gigs: { + select: vi.fn().mockReturnThis(), + eq: vi.fn().mockReturnThis(), + single: vi.fn().mockResolvedValue({ data: gig, error: null }), + }, + applications: { + select: vi.fn().mockReturnThis(), + eq: vi.fn().mockReturnThis(), + single: vi.fn().mockResolvedValue({ data: application, error: null }), + }, + gig_invoices: mockInvoiceTable({ insertResult: null }), + }); + (getAuthContext as any).mockResolvedValue({ user: { id: WORKER_ID }, supabase: sb }); + } + + const body = () => + req({ + application_id: APP_ID, + amount: 150, + payment_currency: "sol", + merchant_wallet_address: "So11111111111111111111111111111111111111112", + }); + + it("says reconnect, not connect, for a link that exists but lacks the scope", async () => { + (getCoinpayLink as any).mockResolvedValue({ state: "needs_reconnect", accessToken: null }); + authAsWorker(); + + const res = await POST(body(), params); + expect(res.status).toBe(409); + + const json = await res.json(); + expect(json.coinpay_link_state).toBe("needs_reconnect"); + expect(json.error).toMatch(/reconnect/i); + // The exact sentence that sent the reporter in circles. + expect(json.error).not.toBe("Connect your CoinPay account before sending an invoice"); + // And the steps must lead with reconnecting, not with connecting. + expect(json.setup_instructions[0]).toMatch(/reconnect/i); + // Still an OAuth round trip, so a client offering the button keeps working. + expect(json.oauth_required).toBe(true); + }); + + it("still says connect when there is no link at all", async () => { + (getCoinpayLink as any).mockResolvedValue({ state: "none", accessToken: null }); + authAsWorker(); + + const res = await POST(body(), params); + expect(res.status).toBe(409); + + const json = await res.json(); + expect(json.coinpay_link_state).toBe("none"); + expect(json.error).toBe("Connect your CoinPay account before sending an invoice"); + expect(json.setup_instructions[0]).not.toMatch(/reconnect/i); + }); + }); + it("creates a pending invoice with the worker's CoinPay receiving wallet", async () => { const gig = { id: GIG_ID, title: "Test Gig", poster_id: POSTER_ID, payment_coin: "SOL" }; const application = { diff --git a/src/app/api/gigs/[id]/invoice/route.ts b/src/app/api/gigs/[id]/invoice/route.ts index 6d679368..89102f45 100644 --- a/src/app/api/gigs/[id]/invoice/route.ts +++ b/src/app/api/gigs/[id]/invoice/route.ts @@ -1,6 +1,6 @@ import { NextRequest, NextResponse } from "next/server"; import { createServiceClient, getAuthContext } from "@/lib/auth/get-user"; -import { getConnectedCoinpayAccessToken } from "@/lib/coinpay-oauth"; +import { getCoinpayLink } from "@/lib/coinpay-oauth"; import { findCoinpayGlobalWallet, getCoinpayGlobalWalletTokens, @@ -94,6 +94,20 @@ const COINPAY_WALLET_SETUP_INSTRUCTIONS = [ "Paste those addresses into Settings > Global Wallet Addresses in CoinPay, then refresh the invoice form.", ]; +/** + * The same steps, for a link that exists and is missing a permission. + * + * "Connect your CoinPay account" is the wrong instruction for somebody who + * already has: it describes something they have done, so they check, see a + * connection, and try again. The first step has to name reconnecting, and say + * why, or the loop has no exit. + */ +const COINPAY_RECONNECT_INSTRUCTIONS = [ + "Reconnect CoinPay from OAuth Connections. Your existing link was authorised before ugig needed permission to read your wallet addresses, so it cannot be used to invoice.", + "Reconnecting re-authorises the same account. Nothing else about it changes and your gigs are untouched.", + "Then check Settings > Global Wallet Addresses in CoinPay has an address for each coin you want to use.", +]; + function countSinglePullRequestLinks(links: string[]) { return new Set( links @@ -442,20 +456,35 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ ); } - const workerCoinpayToken = await getConnectedCoinpayAccessToken(workerId); - if (!workerCoinpayToken) { + const workerCoinpayLink = await getCoinpayLink(workerId); + if (workerCoinpayLink.accessToken === null) { + // "Connect" and "reconnect" are different instructions, and handing the + // wrong one to somebody who is already connected is a loop with no exit: + // they check, see a connection, and try again. See getCoinpayLink. + const reconnect = workerCoinpayLink.state === "needs_reconnect"; return NextResponse.json( { error: isWorker - ? "Connect your CoinPay account before sending an invoice" - : "The worker must connect CoinPay before this invoice can be created", + ? reconnect + ? "Reconnect your CoinPay account before sending an invoice. It is connected, but it was authorised before ugig needed permission to read your wallet addresses." + : "Connect your CoinPay account before sending an invoice" + : reconnect + ? "The worker must reconnect CoinPay before this invoice can be created. Their link predates the wallet permission ugig needs." + : "The worker must connect CoinPay before this invoice can be created", + // Still an OAuth round trip for the worker either way: reconnecting + // is the same authorise flow, so a client that keys on this flag to + // offer the button keeps working. oauth_required: isWorker, + coinpay_link_state: workerCoinpayLink.state, setup_required: true, - setup_instructions: COINPAY_WALLET_SETUP_INSTRUCTIONS, + setup_instructions: reconnect + ? COINPAY_RECONNECT_INSTRUCTIONS + : COINPAY_WALLET_SETUP_INSTRUCTIONS, }, { status: 409 } ); } + const workerCoinpayToken = workerCoinpayLink.accessToken; const workerWallets = await getCoinpayGlobalWalletTokens({ access_token: workerCoinpayToken }); if (workerWallets.length === 0) { diff --git a/src/lib/coinpay-oauth.ts b/src/lib/coinpay-oauth.ts index fa28af27..1fe7ddf0 100644 --- a/src/lib/coinpay-oauth.ts +++ b/src/lib/coinpay-oauth.ts @@ -84,7 +84,46 @@ async function refreshCoinpayToken( } } +/** + * Why a CoinPay link cannot be used, when it cannot. + * + * "none" and "needs_reconnect" are different problems with different fixes, + * and telling them apart is the whole point of this type. #553 taught the + * connections page the difference; every API that gates on CoinPay needs it + * too, because an agent calling the API never sees that page and the sentence + * it gets back is the only instruction it will ever have. + */ +export type CoinpayLinkState = "none" | "needs_reconnect" | "connected"; + +export interface CoinpayLink { + state: CoinpayLinkState; + /** Present only when state is "connected". */ + accessToken: string | null; +} + +/** + * The user's CoinPay link, and what is wrong with it. + * + * Deliberately reports "needs_reconnect" for a link that exists and cannot + * read wallets, rather than folding it into "not connected". That fold is what + * produced the bug: a user with a pre-wallet:read token was told to connect an + * account they had already connected, did nothing different because nothing + * looked wrong, and hit the same wall again. + */ +export async function getCoinpayLink(userId: string): Promise { + const token = await resolveCoinpayToken(userId); + return token.accessToken === null + ? { state: token.hadIdentity ? "needs_reconnect" : "none", accessToken: null } + : { state: "connected", accessToken: token.accessToken }; +} + export async function getConnectedCoinpayAccessToken(userId: string): Promise { + return (await resolveCoinpayToken(userId)).accessToken; +} + +async function resolveCoinpayToken( + userId: string +): Promise<{ accessToken: string | null; hadIdentity: boolean }> { const serviceSupabase = createServiceClient(); const { data } = await (serviceSupabase as any) .from("oauth_identities") @@ -95,14 +134,18 @@ export async function getConnectedCoinpayAccessToken(userId: string): Promise