fix: Bootstrap w/ API for sending payment#5
fix: Bootstrap w/ API for sending payment#5SyedHannanMehdi wants to merge 30 commits intotscircuit:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR bootstraps a mock “Algora-like” API server to support bounty listing/creation and sending payments, aligning with Issue #1 (“Bootstrap w/ API for sending payment”).
Changes:
- Replaces the prior project setup with a TypeScript + Express server (entrypoint + app + routers).
- Adds an in-memory
Storewith seeded bounty data plus/bountiesand/paymentsroutes. - Adds initial Vitest/Supertest coverage for
/bountiesand updates TS/build configuration.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Switches to a Node/TS build output (dist) targeting ES2020/commonjs with declarations & sourcemaps. |
| package.json | Reworks package metadata and scripts for Express runtime + TypeScript build + Vitest tests. |
| src/app.ts | Creates the Express app, JSON middleware, routes, and 404 handler. |
| src/index.ts | Adds the server listener and startup logging. |
| src/types.ts | Introduces shared types for bounties, payments, and API shapes. |
| src/store.ts | Implements an in-memory store with seed data and CRUD helpers. |
| src/routes/bounties.ts | Adds /bounties list/get/create endpoints with Zod validation. |
| src/routes/payments.ts | Adds /payments list/get and /payments/send payment creation endpoint. |
| src/routes/bounties.test.ts | Adds Vitest/Supertest tests for the /bounties endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/routes/payments.ts
Outdated
| if (!result.success) { | ||
| return res | ||
| .status(400) | ||
| .json({ error: "Invalid request body", details: result.error.format() }); |
There was a problem hiding this comment.
This endpoint returns details: result.error.format() on validation failure, but ApiError.details is typed as string in src/types.ts. Either change the response to a string (e.g. result.error.message) or widen the type of details to match what the API actually returns.
| .json({ error: "Invalid request body", details: result.error.format() }); | |
| .json({ error: "Invalid request body", details: result.error.message }); |
src/routes/payments.ts
Outdated
| const { bounty_id, recipient_username, amount_usd, currency } = result.data; | ||
|
|
||
| // Validate the bounty exists | ||
| const bounty = store.getBounty(bounty_id); | ||
| if (!bounty) { | ||
| return res | ||
| .status(404) | ||
| .json({ error: "Bounty not found", details: `No bounty with id "${bounty_id}"` }); | ||
| } | ||
|
|
||
| // Guard: don't allow double-payment | ||
| if (bounty.status === "paid") { | ||
| return res.status(409).json({ | ||
| error: "Bounty already paid", | ||
| details: `Bounty "${bounty_id}" has already been marked as paid`, | ||
| }); | ||
| } | ||
|
|
||
| // Create payment record (simulate async processing → immediately complete) | ||
| const payment = store.createPayment({ | ||
| bounty_id, | ||
| recipient_username, | ||
| amount_usd, | ||
| currency, | ||
| status: "completed", | ||
| transaction_id: `txn_${uuidv4().replace(/-/g, "").slice(0, 16)}`, | ||
| }); |
There was a problem hiding this comment.
amount_usd, currency, and recipient_username are accepted from the request body even though the bounty already has amount/currency (and possibly an assigned recipient). This can create inconsistent records (paying a bounty for the wrong amount/currency/recipient). Consider deriving these fields from the bounty (or at least validating they match) before creating the payment and marking the bounty as paid.
src/routes/bounties.ts
Outdated
| if (!result.success) { | ||
| return res | ||
| .status(400) | ||
| .json({ error: "Invalid request body", details: result.error.format() }); |
There was a problem hiding this comment.
This route also returns details: result.error.format() (an object), which doesn't match the ApiError.details?: string type in src/types.ts. Align the runtime response shape and the exported types (either return a string message or widen the details type).
| .json({ error: "Invalid request body", details: result.error.format() }); | |
| .json({ | |
| error: "Invalid request body", | |
| details: JSON.stringify(result.error.format()), | |
| }); |
src/routes/bounties.test.ts
Outdated
| it("returns seeded bounties when store is populated", async () => { | ||
| store.createBounty({ | ||
| issue_number: 1, | ||
| repo: "tscircuit/fake-algora", | ||
| amount_usd: 10, | ||
| currency: "USD", | ||
| status: "open", | ||
| recipient_username: null, | ||
| }); |
There was a problem hiding this comment.
Test name is misleading: the store is reset in beforeEach, so this test isn't using seeded bounties; it's creating one explicitly. Rename the test (or adjust setup) so the description matches the behavior.
src/routes/payments.ts
Outdated
| router.post("/send", (req, res) => { | ||
| const result = SendPaymentSchema.safeParse(req.body); | ||
| if (!result.success) { | ||
| return res | ||
| .status(400) | ||
| .json({ error: "Invalid request body", details: result.error.format() }); | ||
| } | ||
|
|
||
| const { bounty_id, recipient_username, amount_usd, currency } = result.data; | ||
|
|
||
| // Validate the bounty exists | ||
| const bounty = store.getBounty(bounty_id); | ||
| if (!bounty) { | ||
| return res | ||
| .status(404) | ||
| .json({ error: "Bounty not found", details: `No bounty with id "${bounty_id}"` }); | ||
| } | ||
|
|
||
| // Guard: don't allow double-payment | ||
| if (bounty.status === "paid") { | ||
| return res.status(409).json({ | ||
| error: "Bounty already paid", | ||
| details: `Bounty "${bounty_id}" has already been marked as paid`, | ||
| }); | ||
| } | ||
|
|
||
| // Create payment record (simulate async processing → immediately complete) | ||
| const payment = store.createPayment({ | ||
| bounty_id, | ||
| recipient_username, | ||
| amount_usd, | ||
| currency, | ||
| status: "completed", | ||
| transaction_id: `txn_${uuidv4().replace(/-/g, "").slice(0, 16)}`, | ||
| }); | ||
|
|
||
| // Update bounty status | ||
| store.updateBounty(bounty_id, { | ||
| status: "paid", | ||
| recipient_username, | ||
| }); | ||
|
|
||
| res.status(201).json({ | ||
| payment, | ||
| message: `Payment of $${amount_usd} ${currency} sent to @${recipient_username} for bounty ${bounty_id}`, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
POST /payments/send introduces new behavior (payment creation, double-payment guard, bounty status update) but there are no corresponding Vitest/Supertest tests. Add tests for success, invalid body (400), unknown bounty (404), and already-paid bounty (409) to prevent regressions.
src/index.ts
Outdated
| @@ -0,0 +1,14 @@ | |||
| import app from "./app"; | |||
|
|
|||
| const PORT = process.env.PORT ?? 3000; | |||
There was a problem hiding this comment.
process.env.PORT is a string when set; passing it directly to app.listen can make Node treat it as a pipe name (e.g. "3000") instead of port 3000. Parse/coerce to a number (and validate it) before calling listen to avoid binding to the wrong endpoint.
| const PORT = process.env.PORT ?? 3000; | |
| const PORT = Number(process.env.PORT) || 3000; |
| "test": "vitest run", | ||
| "test:watch": "vitest" |
There was a problem hiding this comment.
The test script runs vitest, but the repo still contains existing tests/**/*.test.ts that import bun:test (and other Bun-only tooling). With the current setup, vitest run will try to execute those files and fail. Either migrate/replace the Bun tests, or configure Vitest's include/exclude patterns so it only runs the intended Vitest/Supertest tests.
| "test": "vitest run", | |
| "test:watch": "vitest" | |
| "test": "vitest run --exclude tests/**/*.test.ts", | |
| "test:watch": "vitest --exclude tests/**/*.test.ts" |
…uards, use string details
…ApiError.details type
…uble-pay & missing recipient
…d, POST /payments/send
Automated fix for #1 — implemented by CashClaw agent.
Closes #1