feat: interactive onboarding setup script#111
Open
JonasBaeumer wants to merge 20 commits into
Open
Conversation
Scaffold for the interactive onboarding setup script. Adds @clack/prompts (v0.11.0, CJS-compatible) as a devDependency, a `setup` npm script entry point, and a dedicated tsconfig for the scripts/ directory that extends the root config.
Shared types (StepResult, SetupContext), ASCII header banner, and utility functions: shell exec wrapper, env file reader/writer with template preservation, health-check polling, OS detection, and random hex generation.
Thin bootstrap script that verifies Node.js 18+ and npm are present, offers to install via brew/nodesource if missing, runs npm install, then execs into the TypeScript setup script. Supports --non-interactive flag for CI environments.
Phase 1 (prerequisites): checks Docker, Compose, Stripe CLI, ngrok with auto-install offers via brew/apt. Phase 2 (environment): bootstraps .env from template, prompts for Stripe key and optional Telegram config, offers defaults with opt-in per-value customization for experienced devs. Phase 3 (infrastructure): starts Docker containers and polls pg_isready / redis-cli ping until healthy. Phase 4 (database): runs prisma generate, migrate, and seed with API key capture and display.
Phase 5 (stripe): validates API key, checks Issuing enabled and balance funded, optionally spawns stripe listen and captures webhook secret into .env. Phase 6 (telegram): validates bot token via getMe API call, reminds about ngrok + webhook registration. Phase 7 (verification): spawns dev server to test health endpoint, offers to run unit and integration tests. Phase 8 (summary): prints color-coded status card with all results and actionable next steps. Main orchestrator (setup.ts) wires all phases together with early bail-outs if Docker or infra checks fail.
JonasBaeumer
requested review from
Hajuj,
aleksandr-gorbunov and
georgyia
as code owners
Add one-command setup instructions at the top of Quick Start, collapse the manual steps into a details block as a fallback.
Group/channel chat IDs are negative (e.g. -5129659862). The validation regex was rejecting them. Updated to allow an optional leading minus and clarified the prompt text.
When docker compose fails due to a port already in use, the script now detects the conflicting process (via lsof) and offers three options: kill the process, remap to the next free port (creates a docker-compose.override.yml and updates .env), or skip. Previously it just printed the error and bailed.
Port conflict resolution now distinguishes between: - Docker Desktop internal processes (never killable) - Docker containers from other projects (offer docker stop) - Regular processes (offer kill) Detects Docker ownership via process name patterns and docker ps --filter to identify specific containers. Prevents accidentally killing com.docker.backend or docker-proxy.
lsof misses native Postgres and some macOS services. Replaced with net.Socket connect probe that catches everything. Also: when docker compose up itself fails with a port conflict, the script now offers the same remap/kill/skip options instead of just bailing. Extracted helpers for port remap logic and override file generation.
Existing containers retain their old port bindings even after writing docker-compose.override.yml. Now runs docker compose rm to remove stale containers before restarting with --force-recreate, ensuring the new port mappings are actually applied.
Replace docker-compose.override.yml approach (which appends ports instead of replacing them) with POSTGRES_HOST_PORT / REDIS_HOST_PORT env vars in docker-compose.yml with defaults. The setup script passes these to docker compose via the process environment, ensuring clean port remapping without override file merge issues.
…ntegration tests Progress bar: - Detect test suite count at runtime via --listTests with identical args to the actual run (no hardcoded counts) - Unit tests exclude integration suites via --testPathIgnorePatterns - Auto-adjust total upward if more suites appear than expected - Final bar uses actual completed count Stripe CLI: - Pass --api-key explicitly to stripe listen so it doesn't rely on a potentially expired stripe login session - Listen on both stdout and stderr for the whsec_ secret - Detect auth failures early to avoid 20s timeout on bad keys Integration tests: - Removed from setup verification (they depend on ngrok + webhook registration which hasn't happened yet) - Added to Next Steps in the summary
New Phase 8 that offers to start dev services (server, worker, Stripe CLI, ngrok) as background processes or wait for manual startup: - For each service, user picks: auto-start, manual, or skip - Live dashboard polls health checks and shows real-time status: ✓ Dev server running (PID 12345) — port 3000 ⠋ Stub worker starting... ✓ ngrok tunnel https://abc123.ngrok-free.app - Auto-detects manually started services (HTTP health check, ngrok API, Telegram getWebhookInfo) - Auto-registers Telegram webhook once ngrok tunnel is detected - Press 's' to skip remaining, or waits until all services are up - Prints cleanup command with all background PIDs at the end - Stripe CLI listener moved from Phase 5 to services phase to avoid duplicate prompts
georgyia
requested changes
Apr 14, 2026
georgyia
left a comment
georgyia
left a comment
Collaborator
There was a problem hiding this comment.
Three things to address before merge:
-
Split PR scope — this branch includes #107's
TELEGRAM_TEST_CHANNEL_IDchanges alongside the onboarding script. Merge #107 first, then rebase this branch so review and rollback stay clean. -
Declare
picocolors—services.ts,verification.ts,summary.ts, andascii.tsimportpicocolorsbut it's not inpackage.json. It only works today because it's a transitive dep. Add it as a devDependency. -
Replace
nc -zwith Node TCP probe —infrastructure.tsusesnc -zto find a free port, butncisn't guaranteed on all machines. You already have a workingisPortListening()TCP probe in the same file — reuse that instead.
Unit tests pass (355/355). The onboarding flow itself is solid — nice work on the port-conflict handling and the live services dashboard.
8 tasks
- Use singleton getStripeClient() in reconciliation and authorizationFlow tests so they inherit maxNetworkRetries (was new Stripe() with retries=0) - Bump Stripe maxNetworkRetries 3 -> 5 to survive transient test-mode connection errors - Raise hook timeouts in cardLifecycle (60s -> 90s), checkoutSimulator (60s -> 120s) and telegramApprovalCheckout step 4 (30s -> 120s) to absorb Stripe API latency variance - Switch checkoutSimulator and telegramApprovalCheckout to testHelpers.transactions.createForceCapture, bypassing the real-time authorization webhook that interferes when stripe listen is running - Fix reconciliation afterAll cleanup order so FK-dependent rows (virtualCard, ledgerEntry, pot) are deleted before purchaseIntent/user - Use Math.abs on Stripe Issuing capture amounts in reconciliationService (Stripe returns negative amounts for debits) - Gitignore .env.* (excluding .env.example) to prevent accidental commits of test credentials
…oard polish - Add Phase 9 'integration tests' to setup orchestrator (npm run setup) using runIntegrationSuite from verification.ts; renumber summary to Phase 10 - Replace single-select service launcher with multiselect: developers can now pick which services (dev server, worker, stripe listen, telegram) to launch in one pass; track launchedServices on SetupContext - Switch verification phase functions to return SubStep results that render through a shared logSubSteps helper, replacing ad-hoc inline spinners - Tighten prerequisites and database phase output (substep formatting, fewer redundant log lines) - Add SubStep utilities (subStepPass/Fail/Warn/Skip, logSubSteps) in utils.ts and a SubStep type in types.ts
This was referenced May 1, 2026
# Conflicts: # src/payments/providers/stripe/reconciliationService.ts # tests/integration/e2e/telegramApprovalCheckout.test.ts # tests/integration/stripe/reconciliation.test.ts
The bot UX previously created a dual-channel feeling: setup/test pings mingled with live notifications, and the seeded demo user was auto-linked to a dev chat. The recently added TELEGRAM_TEST_CHANNEL_ID addressed this by routing test traffic to a separate group, but the goal of #163 is the opposite — keep one chat per user and make setup messages disappear after onboarding completes. Behaviour changes: - Track every bot-sent and user-sent message_id in the signup session. - On successful signup, after the API-key message, bulk-delete all tracked messages and land the user on the persistent main menu. - Same cleanup runs when a stale session is replaced by a new /start. - Emit TELEGRAM_SETUP_CLEANED audit event with the deleted count. Other changes: - Drop TELEGRAM_TEST_CHAT_ID from env.ts/seed.ts; demo user is no longer auto-linked. Drop TELEGRAM_TEST_CHANNEL_ID and its docs. - Live tests still read process.env.TELEGRAM_TEST_CHAT_ID as a local-dev escape hatch (skips when unset). - Fix live-test bug: callback_query filter compared from.id (user) against the configured chat ID, dropping every tap in a group chat. Now compares message.chat.id. Same fix in menuHandler.live. - Wrong-button alert in live tests no longer leaks raw callback_data. - Add deleteMessage to mockBot; tests for cleanup, stale-session cleanup, and best-effort deletion. - docs/telegram-setup.md rewritten around a single onboarding path.
Contributor
📝 WalkthroughWalkthroughThis PR introduces a comprehensive development setup automation system with multi-phase configuration orchestration, refactors Telegram onboarding to track and clean up ephemeral signup messages with audit event recording, consolidates Stripe client usage across tests, improves Telegram webhook reliability in integration tests, and updates documentation to reflect the new setup and pairing flows. ChangesDevelopment Setup Automation
Telegram Onboarding with Message Cleanup
Documentation & Configuration
Stripe Client Consolidation & Test Reliability
Telegram Signup Handler Tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/integration/e2e/checkoutSimulator.test.ts (1)
122-126:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard cleanup against undefined IDs to prevent full-table deletes.
If
beforeAllfails before assigningintentIdoruserId, Prisma'sdeleteMany({ where: { intentId: undefined } })will match all rows and delete the entire table. This is a known risk with Prisma.🛡️ Proposed fix to guard cleanup
afterAll(async () => { - await prisma.virtualCard.deleteMany({ where: { intentId } }); - await prisma.purchaseIntent.deleteMany({ where: { id: intentId } }); - await prisma.user.deleteMany({ where: { id: userId } }); + if (intentId) { + await prisma.virtualCard.deleteMany({ where: { intentId } }); + await prisma.purchaseIntent.deleteMany({ where: { id: intentId } }); + } + if (userId) { + await prisma.user.deleteMany({ where: { id: userId } }); + } });Based on learnings: "In Prisma queries, do not pass
undefinedas a value inside awhereclause... Prisma will treat it as 'no filter,' which can trigger full-table operations."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/e2e/checkoutSimulator.test.ts` around lines 122 - 126, The cleanup in afterAll calls prisma.virtualCard.deleteMany, prisma.purchaseIntent.deleteMany and prisma.user.deleteMany using intentId and userId which may be undefined if beforeAll failed; guard these calls by checking intentId and userId are defined before invoking the respective deleteMany (or only include a where clause when the ID is present) to avoid accidental full-table deletes — update the afterAll to conditionally call prisma.virtualCard.deleteMany({ where: { intentId } }) when intentId is not undefined and similarly guard prisma.purchaseIntent.deleteMany({ where: { id: intentId } }) and prisma.user.deleteMany({ where: { id: userId } }).tests/integration/e2e/telegramApprovalCheckout.test.ts (1)
85-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard cleanup against undefined IDs to prevent full-table deletes.
userIdandintentIdare assigned insideit()blocks (lines 105 and 130). If early tests fail before assignment, these remainundefined, and Prisma'sdeleteMany({ where: { intentId: undefined } })matches all rows.🛡️ Proposed fix to guard cleanup
afterAll(async () => { - await prisma.virtualCard.deleteMany({ where: { intentId } }); - await prisma.ledgerEntry.deleteMany({ where: { intentId } }); - await prisma.pot.deleteMany({ where: { intentId } }); - await prisma.approvalDecision.deleteMany({ where: { intentId } }); - await prisma.auditEvent.deleteMany({ where: { intentId } }); - await prisma.purchaseIntent.deleteMany({ where: { id: intentId } }); - await prisma.user.deleteMany({ where: { id: userId } }); + if (intentId) { + await prisma.virtualCard.deleteMany({ where: { intentId } }); + await prisma.ledgerEntry.deleteMany({ where: { intentId } }); + await prisma.pot.deleteMany({ where: { intentId } }); + await prisma.approvalDecision.deleteMany({ where: { intentId } }); + await prisma.auditEvent.deleteMany({ where: { intentId } }); + await prisma.purchaseIntent.deleteMany({ where: { id: intentId } }); + } + if (userId) { + await prisma.user.deleteMany({ where: { id: userId } }); + } });Based on learnings: "In Prisma queries, do not pass
undefinedas a value inside awhereclause... Prisma will treat it as 'no filter,' which can trigger full-table operations."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/e2e/telegramApprovalCheckout.test.ts` around lines 85 - 92, The afterAll cleanup calls (prisma.virtualCard.deleteMany, prisma.ledgerEntry.deleteMany, prisma.pot.deleteMany, prisma.approvalDecision.deleteMany, prisma.auditEvent.deleteMany, prisma.purchaseIntent.deleteMany, prisma.user.deleteMany) use intentId and userId that are set only inside it() blocks; guard each deleteMany so it only runs when the corresponding id is defined (e.g., if (intentId) { prisma.virtualCard.deleteMany({ where: { intentId } }) } and similarly for other intentId-based deletes, and if (userId) { prisma.user.deleteMany({ where: { id: userId } }) }), preventing passing undefined into where and avoiding accidental full-table deletes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/telegram-setup.md`:
- Around line 120-123: The doc currently implies `npm run seed` is required for
the `/start <code>` pairing flow, which is incorrect; clarify or remove that
prerequisite by stating that `npm run seed` is optional and only needed if you
want the pre-created demo user (demo@agentpay.dev), otherwise you can directly
call POST /v1/agent/register to obtain a pairing code and complete the same
`/start <code>` flow—update the paragraph referencing `npm run seed`,
`demo@agentpay.dev`, `POST /v1/agent/register`, and `/start <code>` to reflect
this optionality (or delete the seed instruction if you prefer to avoid
confusion).
In `@README.md`:
- Around line 262-266: Update the README's quick-start clone command to replace
the placeholder "your-org/trustedpaymentinfrastructureforagents" with the actual
repository location; edit the code block containing the lines starting with "git
clone https://github.com/your-org/trustedpaymentinfrastructureforagents" so it
points to the real GitHub org/repo (or a generic "git clone <repo-url>"
placeholder) and verify the subsequent "cd
trustedpaymentinfrastructureforagents" and "npm run setup" commands match the
chosen repo name.
In `@scripts/setup.ts`:
- Around line 80-95: The integration test phase is always executed even when the
user declines to launch services; update the logic so runIntegrationSuite(ctx)
only runs if services will be available: either wrap the call in the existing
launchNow condition (i.e. call runIntegrationSuite(ctx) inside the if
(launchNow) block after await launchServices(ctx)) or add a service-availability
check (e.g. isServicesRunning(ctx) or similar) and only call
runIntegrationSuite(ctx) when that check passes; reference launchNow,
launchServices(ctx), runIntegrationSuite(ctx), confirm and isCancel to locate
and modify the decision point.
In `@scripts/setup/database.ts`:
- Around line 48-59: The script always runs exec('npm run seed') which re-seeds
the demo user and rotates the API key; modify the flow in the database setup
routine to first detect whether the demo user already exists (e.g., query your
DB or check ctx/results) and only call exec('npm run seed 2>&1') when the demo
user is absent or when an explicit reset flag is passed; update the branches
that call steps.push(subStepFail(...))/steps.push(subStepPass(...)) and
ctx.results accordingly so they reflect "skipped" vs "created", and apply the
same conditional logic to the other seed invocation later in the file that
mirrors this block (the one that currently uses subStepPass/subStepFail and
s.message/s.stop/logSubSteps).
- Around line 32-34: Replace the conditional selection of migrate command so the
script always uses the safe, non-interactive deploy flow: change the logic that
sets migrateCmd (currently using ctx.nonInteractive and 'npx prisma migrate dev'
vs 'npx prisma migrate deploy') to always assign 'npx prisma migrate deploy',
and ensure the subsequent call to execStreaming('sh', ['-c', migrateCmd], {
timeout: 60_000 }) uses that constant; update any related comments or variable
names (e.g., migrateCmd, ctx.nonInteractive, execStreaming invocation) to
reflect the always-deploy behavior.
In `@scripts/setup/environment.ts`:
- Around line 93-106: The non-interactive branch currently sets ctx.skipTelegram
using process.env.TELEGRAM_BOT_TOKEN which ignores tokens loaded into envVars
from the merged .env; change the logic in the non-interactive branch (where
ctx.nonInteractive is true) to base ctx.skipTelegram on
envVars.TELEGRAM_BOT_TOKEN (the merged state) instead of process.env, and keep
using process.env only as a fallback when filling placeholders later (e.g., in
the block that assigns envVars.TELEGRAM_BOT_TOKEN when isPlaceholder(...) is
true); update the condition that sets ctx.skipTelegram so it reflects the merged
envVars value.
In `@scripts/setup/infrastructure.ts`:
- Around line 172-178: The code calling exec(`docker stop
${processInfo.containerName}`) when action === 'stop-container' risks shell
injection via processInfo.containerName; fix by validating/sanitizing
processInfo.containerName (e.g., allow only [-_.A-Za-z0-9]+) or, better, avoid
shell interpolation by using a safe API (child_process.execFile or spawn) to
pass the container name as an argument; update the block around the action
'stop-container' and the exec call so it either rejects invalid names or calls
execFile('docker', ['stop', processInfo.containerName'], ...) and preserve
existing timeout/exit handling.
- Around line 211-228: The current applyPortRemap implementation uses naive
string.replace on ctx.envVars.DATABASE_URL and ctx.envVars.REDIS_URL which can
accidentally replace occurrences of the port number in user, password, or
database names; update applyPortRemap to parse the connection URLs and set the
port explicitly: in applyPortRemap, for composeService === 'postgres' and
'redis', construct a URL object from ctx.envVars.DATABASE_URL /
ctx.envVars.REDIS_URL (adding a scheme prefix if missing), set url.port =
String(newPort), then assign ctx.envVars.DATABASE_URL = url.toString() (and
ctx.envVars.REDIS_URL similarly); include a small try/catch and only fall back
to the current string.replace logic if URL parsing throws so behavior is
resilient.
In `@scripts/setup/prerequisites.ts`:
- Around line 20-31: The code currently treats ctx.nonInteractive as "install
without asking"; change offer logic so non-interactive mode does not
auto-install optional tools: initialize shouldInstall = false (not
ctx.nonInteractive), only set shouldInstall = answer when confirm(...) runs in
interactive mode, and add support for an explicit opt-in flag (e.g.,
ctx.autoInstall or a new parameter) if callers really want automatic installs in
non-interactive runs; also ensure when skipping in non-interactive mode you
log/warn that install was skipped instead of running exec(cmd), and keep the
final exec(cmd) path unchanged (use exec(...) only when shouldInstall is true).
In `@scripts/setup/services.ts`:
- Around line 166-170: The Stripe secret is leaked via command-line arg: stop
passing ctx.envVars.STRIPE_SECRET_KEY as '--api-key' to the spawned 'stripe
listen' process; instead set STRIPE_API_KEY in the child process environment and
remove the '--api-key' arg. Update the call that uses spawnBackground('stripe',
[...]) (and adjust spawnBackground usage if needed) to pass an env object that
merges process.env with { STRIPE_API_KEY: ctx.envVars.STRIPE_SECRET_KEY } and
remove the '--api-key' entry from the argument array so the CLI reads the key
from the environment.
In `@scripts/setup/telegram.ts`:
- Around line 6-26: The telegramGetMe promise can hang if the Telegram API
stalls; modify telegramGetMe to implement a request timeout (e.g., 5–10s) that
aborts the HTTPS request and resolves { ok: false } when exceeded. Locate the
telegramGetMe function and the https.get(...) call, capture the returned
ClientRequest (const req = https.get(...)), start a timer when the request is
made, and on timeout call req.abort() (or req.destroy()), clear the timer on
'end' and 'error', and ensure the timeout path resolves { ok: false } so the
awaited call won't hang the spinner; also clear the timer in the normal JSON
parse path to avoid leaks.
In `@scripts/setup/utils.ts`:
- Around line 72-86: The parser in readEnvFile currently leaves surrounding
quotes on values (e.g., KEY="value") causing mismatches; update readEnvFile to,
after computing value in the loop, detect if value starts and ends with matching
single or double quotes and if so remove those outer quotes, then unescape
common escape sequences (e.g., \" -> ", \' -> ', \\n -> newline, \\r, \\t, \\)
before assigning into vars[key]; make these changes inside the same function
where value is defined so vars[key] receives the unquoted/unescaped string.
- Around line 167-178: The spawn call in execStreaming currently sets shell:
true while passing cmd and args as an array which defeats the
security/arg-escaping benefits of using an args array; either remove the shell
option to keep using cmd+args safely (in function execStreaming where spawn(...)
is called) or, if you actually need shell features, switch to passing a single
command string (join/escape cmd+args) and keep shell: true; update the spawn
invocation in execStreaming accordingly so it uses one consistent mode (array+no
shell or single string+shell).
In `@scripts/setup/verification.ts`:
- Around line 236-244: The child process is not started as a process group
leader, so process.kill(-serverProcess.pid, 'SIGTERM') will not reliably
terminate its process tree; update the spawn(...) call that assigns to
serverProcess to include { detached: true } (and appropriate stdio settings),
call serverProcess.unref() after spawning, and keep the existing finally block
so the primary shutdown uses process.kill(-serverProcess.pid, 'SIGTERM') with
the current fallback to serverProcess.kill('SIGTERM') if the group kill fails.
- Around line 371-376: The spinner start/stop around unit tests is redundant:
remove the immediate s.start('Running unit tests...') and s.stop('Running unit
tests...') pair and instead start the spinner only when ctx.nonInteractive is
false immediately before invoking runUnitTests(ctx), then stop the spinner after
runUnitTests completes (and also ensure it is stopped in error/cleanup paths);
update the block that checks shouldRunTests to reference the same s.start/s.stop
around the async call to runUnitTests and avoid the flicker caused by the
current immediate start/stop.
- Around line 413-416: The spinner is being started and immediately stopped
without performing any work (the redundant start/stop pattern using spinner()
assigned to s), so remove the unnecessary calls: either delete the
s.start('Running integration tests...') and s.stop('Running integration
tests...') lines or wrap them properly around the actual integration test
invocation so the spinner runs during work (i.e., call s.start(...) immediately
before running the tests and s.stop(...) after the tests complete using the
existing spinner() instance `s`). Ensure you update any error/exit paths to stop
the spinner before returning/throwing.
In `@setup.sh`:
- Around line 72-86: The non-interactive path currently calls install_node when
check_node fails and SETUP_NON_INTERACTIVE is set, which attempts to mutate the
machine; change this so that when SETUP_NON_INTERACTIVE is "1" the script fails
fast with a clear actionable error instead of calling install_node. Update the
logic around check_node and the SETUP_NON_INTERACTIVE branch (referencing the
check_node function and install_node invocation and the SETUP_NON_INTERACTIVE
variable) to print a helpful error message instructing users to install Node.js
or run an explicit auto-install flag, and exit with a non-zero status; keep
install_node only reachable from the interactive prompt or a new explicit
--auto-install flag handler.
In `@tests/integration/stripe/reconciliation.test.ts`:
- Around line 117-123: The teardown in afterAll calls prisma.deleteMany/delete
with intentId and prisma.delete with userId without checking they were assigned,
which can pass undefined and remove all rows; update the afterAll to guard each
cleanup by checking the identifiers (intentId and userId) exist before calling
prisma.virtualCard.deleteMany, prisma.ledgerEntry.deleteMany,
prisma.pot.deleteMany, prisma.purchaseIntent.delete, and prisma.user.delete
(e.g., only call the intent-related deleteMany/delete when intentId is truthy
and only call the user delete when userId is truthy) so no deletion runs when
IDs were never set.
In `@tests/integration/telegram/preferences.live.test.ts`:
- Around line 145-147: The test helper currently only filters by chat.id
(cb.message?.chat?.id) but drops the original sender id, causing forwarded
webhook payloads to reuse TEST_CHAT_ID as the synthetic sender; modify the wait
helpers that return callbacks/messages (the code paths referencing cb and msg in
these helpers) to preserve and return the original from.id (e.g., cb.from.id or
msg.from.id) alongside the message, and update the forwarding logic to use that
preserved sender id instead of TEST_CHAT_ID when constructing the synthetic
sender for forwarded updates so group sender identity is maintained.
---
Outside diff comments:
In `@tests/integration/e2e/checkoutSimulator.test.ts`:
- Around line 122-126: The cleanup in afterAll calls
prisma.virtualCard.deleteMany, prisma.purchaseIntent.deleteMany and
prisma.user.deleteMany using intentId and userId which may be undefined if
beforeAll failed; guard these calls by checking intentId and userId are defined
before invoking the respective deleteMany (or only include a where clause when
the ID is present) to avoid accidental full-table deletes — update the afterAll
to conditionally call prisma.virtualCard.deleteMany({ where: { intentId } })
when intentId is not undefined and similarly guard
prisma.purchaseIntent.deleteMany({ where: { id: intentId } }) and
prisma.user.deleteMany({ where: { id: userId } }).
In `@tests/integration/e2e/telegramApprovalCheckout.test.ts`:
- Around line 85-92: The afterAll cleanup calls (prisma.virtualCard.deleteMany,
prisma.ledgerEntry.deleteMany, prisma.pot.deleteMany,
prisma.approvalDecision.deleteMany, prisma.auditEvent.deleteMany,
prisma.purchaseIntent.deleteMany, prisma.user.deleteMany) use intentId and
userId that are set only inside it() blocks; guard each deleteMany so it only
runs when the corresponding id is defined (e.g., if (intentId) {
prisma.virtualCard.deleteMany({ where: { intentId } }) } and similarly for other
intentId-based deletes, and if (userId) { prisma.user.deleteMany({ where: { id:
userId } }) }), preventing passing undefined into where and avoiding accidental
full-table deletes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b68ccdc4-7778-44a3-b6d9-4b5f7a46064e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (44)
.env.example.gitignoreCLAUDE.mdREADME.mddocker-compose.ymldocs/telegram-setup.mdpackage.jsonscripts/setup.tsscripts/setup/ascii.tsscripts/setup/database.tsscripts/setup/environment.tsscripts/setup/infrastructure.tsscripts/setup/prerequisites.tsscripts/setup/services.tsscripts/setup/stripe.tsscripts/setup/summary.tsscripts/setup/telegram.tsscripts/setup/types.tsscripts/setup/utils.tsscripts/setup/verification.tsscripts/tsconfig.jsonsetup.shsrc/config/env.tssrc/contracts/audit.tssrc/db/seed.tssrc/payments/providers/stripe/stripeClient.tssrc/telegram/mockBot.tssrc/telegram/sessionStore.tssrc/telegram/signupHandler.tstests/integration/e2e/cardLifecycle.test.tstests/integration/e2e/checkoutSimulator.test.tstests/integration/e2e/onboarding.test.tstests/integration/e2e/stripeWebhook.test.tstests/integration/e2e/telegramApprovalCheckout.test.tstests/integration/stripe/authorizationFlow.test.tstests/integration/stripe/reconciliation.test.tstests/integration/telegram/connectionFlow.test.tstests/integration/telegram/menuHandler.live.test.tstests/integration/telegram/preferences.live.test.tstests/unit/api/rateLimit.test.tstests/unit/api/telegram.test.tstests/unit/telegram/callbackHandler.test.tstests/unit/telegram/notificationService.test.tstests/unit/telegram/signupHandler.test.ts
💤 Files with no reviewable changes (6)
- .env.example
- tests/unit/api/telegram.test.ts
- src/config/env.ts
- tests/unit/telegram/notificationService.test.ts
- tests/unit/telegram/callbackHandler.test.ts
- tests/unit/api/rateLimit.test.ts
Comment on lines
+120
to
+123
| The pairing code is valid for 30 minutes. If it expires before the user signs up, OpenClaw calls `POST /v1/agent/register` again with `{ "agentId": "ag_abc123" }` to get a fresh code. | ||
|
|
||
| ### Step 6A — Explore the menu | ||
| > **Local testing without OpenClaw?** Run `npm run seed` to create the `demo@agentpay.dev` user, then call `POST /v1/agent/register` directly with `curl` to get a pairing code and complete the same `/start <code>` flow above. | ||
|
|
Contributor
There was a problem hiding this comment.
Clarify or remove the npm run seed prerequisite here.
This fallback immediately switches into the same pairing flow, which already creates the account and API key. As written, npm run seed looks required for /start <code> even though it appears unrelated to those steps.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/telegram-setup.md` around lines 120 - 123, The doc currently implies
`npm run seed` is required for the `/start <code>` pairing flow, which is
incorrect; clarify or remove that prerequisite by stating that `npm run seed` is
optional and only needed if you want the pre-created demo user
(demo@agentpay.dev), otherwise you can directly call POST /v1/agent/register to
obtain a pairing code and complete the same `/start <code>` flow—update the
paragraph referencing `npm run seed`, `demo@agentpay.dev`, `POST
/v1/agent/register`, and `/start <code>` to reflect this optionality (or delete
the seed instruction if you prefer to avoid confusion).
Comment on lines
+262
to
+266
| ```bash | ||
| git clone https://github.com/your-org/trustedpaymentinfrastructureforagents | ||
| cd trustedpaymentinfrastructureforagents | ||
| npm run setup | ||
| ``` |
Contributor
There was a problem hiding this comment.
Replace the placeholder clone target.
These commands still point at your-org/trustedpaymentinfrastructureforagents, so the very first quick-start step fails for anyone copying from this repo.
📌 Suggested fix
-git clone https://github.com/your-org/trustedpaymentinfrastructureforagents
-cd trustedpaymentinfrastructureforagents
+git clone https://github.com/JonasBaeumer/AgentWallet
+cd AgentWallet🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 262 - 266, Update the README's quick-start clone
command to replace the placeholder
"your-org/trustedpaymentinfrastructureforagents" with the actual repository
location; edit the code block containing the lines starting with "git clone
https://github.com/your-org/trustedpaymentinfrastructureforagents" so it points
to the real GitHub org/repo (or a generic "git clone <repo-url>" placeholder)
and verify the subsequent "cd trustedpaymentinfrastructureforagents" and "npm
run setup" commands match the chosen repo name.
Comment on lines
+80
to
+95
| // Phase 8: Launch services | ||
| let launchNow = true; | ||
| if (!ctx.nonInteractive) { | ||
| const answer = await confirm({ | ||
| message: 'Launch dev services now? (server, worker, webhooks)', | ||
| initialValue: true, | ||
| }); | ||
| launchNow = !isCancel(answer) && answer; | ||
| } | ||
|
|
||
| if (launchNow) { | ||
| await launchServices(ctx); | ||
| } | ||
|
|
||
| // Phase 9: Integration tests (requires services running) | ||
| await runIntegrationSuite(ctx); |
Contributor
There was a problem hiding this comment.
Skip phase 9 when services were not launched.
The prompt makes service launch optional, but runIntegrationSuite(ctx) still executes unconditionally. If the user answers “No”, this path immediately runs tests that depend on those services and turns the opt-out into a guaranteed failure. Gate phase 9 behind launchNow, or detect that the required services are already running before starting the suite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/setup.ts` around lines 80 - 95, The integration test phase is always
executed even when the user declines to launch services; update the logic so
runIntegrationSuite(ctx) only runs if services will be available: either wrap
the call in the existing launchNow condition (i.e. call runIntegrationSuite(ctx)
inside the if (launchNow) block after await launchServices(ctx)) or add a
service-availability check (e.g. isServicesRunning(ctx) or similar) and only
call runIntegrationSuite(ctx) when that check passes; reference launchNow,
launchServices(ctx), runIntegrationSuite(ctx), confirm and isCancel to locate
and modify the decision point.
Comment on lines
+32
to
+34
| s.stop('Running database migrations...'); | ||
| const migrateCmd = ctx.nonInteractive ? 'npx prisma migrate deploy' : 'npx prisma migrate dev'; | ||
| const migrate = await execStreaming('sh', ['-c', migrateCmd], { timeout: 60_000 }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "database.ts" --path "*scripts/setup*"Repository: JonasBaeumer/AgentWallet
Length of output: 302
🏁 Script executed:
cat -n scripts/setup/database.ts | head -100Repository: JonasBaeumer/AgentWallet
Length of output: 3963
🌐 Web query:
Prisma migrate dev vs migrate deploy difference idempotent setup onboarding
💡 Result:
Difference: prisma migrate dev is for development; it detects schema drift using a shadow database, generates new migrations from schema changes, applies migrations to the dev database, and updates the migration history; it should never be used in production. prisma migrate deploy is for non-development (staging/production); it only applies existing pending migrations from your migration history to the target database, does not detect drift, does not generate artifacts, and does not reset the database. Idempotent setup / onboarding guidance (common production-safe pattern) 1) Initialize / first-time onboarding (existing DB): you must baseline the production database once, using Prisma Migrate’s baselining/resolve flow; after that, CI/CD can deploy consistently via migrate deploy. [1][2] 2) Development workflow: use prisma migrate dev locally to create new migration files and update your local migration history, then commit the generated /prisma/migrations and prisma/schema.prisma so other environments can apply the same migration history. [3][4] 3) Production (CI/CD) workflow: run prisma migrate deploy as part of an automated CI/CD pipeline to apply pending migrations. [5][3] 4) Why this is “idempotent” for onboarding/ops: migrate deploy compares the migration history recorded in _prisma_migrations with the migration files in the prisma/migrations directory and only applies migrations that are pending; it won’t re-apply already-applied migrations, and it won’t reset or generate artifacts. [4][6] Practical checklist for repeatable, safe deployment - Commit migration artifacts: ensure your migration history folder is in source control and your Prisma schema is synced. [5][4] - CI step: run npx prisma migrate deploy (with DATABASE_URL set in CI). [5][6] - Concurrency: Prisma Migrate uses advisory locking for production commands like migrate deploy to avoid catastrophic errors when multiple processes run concurrently; if it times out, you need to rerun. [6] Key gotchas - Never run prisma migrate dev in production. [3] - migrate deploy does not detect schema drift and does not reset the DB; if the production DB has diverged (e.g., hotfix/manual changes), you need to reconcile using migration tooling (e.g., migrate resolve / diff-based recovery). [6][2] - On MongoDB, migrate deploy isn’t supported; use db push instead. [7][5]
Citations:
- 1: https://www.prisma.io/docs/orm/prisma-client/deployment/deploy-migrations-from-a-local-environment
- 2: https://www.prisma.io/docs/cli/migrate/resolve
- 3: https://www.prisma.io/docs/orm/prisma-migrate/workflows/development-and-production
- 4: https://www.prisma.io/docs/orm/prisma-migrate/understanding-prisma-migrate/mental-model
- 5: https://www.prisma.io/docs/orm/prisma-client/deployment/deploy-database-changes-with-prisma-migrate
- 6: https://prisma.io/docs/orm/prisma-migrate/workflows/development-and-production
- 7: https://www.prisma.io/docs/cli/migrate/deploy
Always use npx prisma migrate deploy for this setup script.
The current code uses npx prisma migrate dev in interactive mode, which is unsafe for a setup script. prisma migrate dev detects schema drift, can prompt for drift resolution, reset the database, and generate new migration files—all behaviors that violate idempotence expectations for automated setup. npx prisma migrate deploy is the correct choice for both interactive and non-interactive modes: it only applies existing pending migrations from source control without resetting, detecting drift, or generating artifacts. This ensures setup reruns remain predictable.
Suggested change
- const migrateCmd = ctx.nonInteractive ? 'npx prisma migrate deploy' : 'npx prisma migrate dev';
+ const migrateCmd = 'npx prisma migrate deploy';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/setup/database.ts` around lines 32 - 34, Replace the conditional
selection of migrate command so the script always uses the safe, non-interactive
deploy flow: change the logic that sets migrateCmd (currently using
ctx.nonInteractive and 'npx prisma migrate dev' vs 'npx prisma migrate deploy')
to always assign 'npx prisma migrate deploy', and ensure the subsequent call to
execStreaming('sh', ['-c', migrateCmd], { timeout: 60_000 }) uses that constant;
update any related comments or variable names (e.g., migrateCmd,
ctx.nonInteractive, execStreaming invocation) to reflect the always-deploy
behavior.
Comment on lines
+48
to
+59
| // Seed demo user | ||
| s.message('Seeding demo user...'); | ||
| const seed = exec('npm run seed 2>&1', { timeout: 30_000 }); | ||
| if (seed.code !== 0) { | ||
| steps.push(subStepFail('Demo user seed', seed.stderr || seed.stdout || 'Seed script failed')); | ||
| ctx.results.push({ name: 'Demo user seed', status: 'fail', message: 'Seed script failed' }); | ||
| s.stop('Database setup completed with errors'); | ||
| logSubSteps(steps); | ||
| return; | ||
| } | ||
| steps.push(subStepPass('Demo user seed', 'demo@agentpay.dev')); | ||
| ctx.results.push({ name: 'Demo user seed', status: 'pass', message: 'demo@agentpay.dev created' }); |
Contributor
There was a problem hiding this comment.
Always reseeding the demo user breaks the advertised idempotent rerun.
This phase reruns npm run seed every time, and the note explicitly says that rerunning rotates the API key. That means a harmless setup rerun invalidates previously issued credentials and mutates local state. Either skip reseeding when the demo user already exists, or change the seed path to preserve the existing key unless the user explicitly requests a reset.
Also applies to: 70-77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/setup/database.ts` around lines 48 - 59, The script always runs
exec('npm run seed') which re-seeds the demo user and rotates the API key;
modify the flow in the database setup routine to first detect whether the demo
user already exists (e.g., query your DB or check ctx/results) and only call
exec('npm run seed 2>&1') when the demo user is absent or when an explicit reset
flag is passed; update the branches that call
steps.push(subStepFail(...))/steps.push(subStepPass(...)) and ctx.results
accordingly so they reflect "skipped" vs "created", and apply the same
conditional logic to the other seed invocation later in the file that mirrors
this block (the one that currently uses subStepPass/subStepFail and
s.message/s.stop/logSubSteps).
Comment on lines
+371
to
+376
| if (shouldRunTests) { | ||
| if (!ctx.nonInteractive) { | ||
| s.start('Running unit tests...'); | ||
| } | ||
| s.stop('Running unit tests...'); | ||
| const testStep = await runUnitTests(ctx); |
Contributor
There was a problem hiding this comment.
Redundant spinner start/stop sequence.
Lines 372-375 start a spinner and immediately stop it, which creates visual flicker without providing useful feedback. This appears to be leftover debug code or a logic error.
🐛 Proposed fix
if (shouldRunTests) {
- if (!ctx.nonInteractive) {
- s.start('Running unit tests...');
- }
- s.stop('Running unit tests...');
+ log.info('Running unit tests...');
const testStep = await runUnitTests(ctx);
steps.push(testStep);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/setup/verification.ts` around lines 371 - 376, The spinner start/stop
around unit tests is redundant: remove the immediate s.start('Running unit
tests...') and s.stop('Running unit tests...') pair and instead start the
spinner only when ctx.nonInteractive is false immediately before invoking
runUnitTests(ctx), then stop the spinner after runUnitTests completes (and also
ensure it is stopped in error/cleanup paths); update the block that checks
shouldRunTests to reference the same s.start/s.stop around the async call to
runUnitTests and avoid the flicker caused by the current immediate start/stop.
Comment on lines
+413
to
+416
| const s = spinner(); | ||
| s.start('Running integration tests...'); | ||
| s.stop('Running integration tests...'); | ||
|
|
Contributor
There was a problem hiding this comment.
Same redundant spinner pattern.
Lines 414-415 also have the start/stop without useful work pattern.
🐛 Proposed fix
- const s = spinner();
- s.start('Running integration tests...');
- s.stop('Running integration tests...');
+ log.info('Running integration tests...');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const s = spinner(); | |
| s.start('Running integration tests...'); | |
| s.stop('Running integration tests...'); | |
| log.info('Running integration tests...'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/setup/verification.ts` around lines 413 - 416, The spinner is being
started and immediately stopped without performing any work (the redundant
start/stop pattern using spinner() assigned to s), so remove the unnecessary
calls: either delete the s.start('Running integration tests...') and
s.stop('Running integration tests...') lines or wrap them properly around the
actual integration test invocation so the spinner runs during work (i.e., call
s.start(...) immediately before running the tests and s.stop(...) after the
tests complete using the existing spinner() instance `s`). Ensure you update any
error/exit paths to stop the spinner before returning/throwing.
Comment on lines
+72
to
+86
| if ! check_node; then | ||
| if [ "${SETUP_NON_INTERACTIVE:-}" = "1" ]; then | ||
| install_node | ||
| else | ||
| read -rp "Install Node.js 20? [y/N] " yn | ||
| case "$yn" in | ||
| [Yy]*) install_node ;; | ||
| *) echo "Aborting — Node.js 18+ is required."; exit 1 ;; | ||
| esac | ||
| fi | ||
|
|
||
| if ! check_node; then | ||
| echo -e "${RED}Node.js installation failed. Please install manually: https://nodejs.org${NC}" | ||
| exit 1 | ||
| fi |
Contributor
There was a problem hiding this comment.
Don't auto-install Node in --non-interactive mode.
The headless path currently tries to mutate the machine with Homebrew/apt/sudo, which is brittle for CI and other locked-down environments. In non-interactive mode this should fail fast with an actionable error instead; keep installation as an interactive opt-in or a separate explicit --auto-install flag.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@setup.sh` around lines 72 - 86, The non-interactive path currently calls
install_node when check_node fails and SETUP_NON_INTERACTIVE is set, which
attempts to mutate the machine; change this so that when SETUP_NON_INTERACTIVE
is "1" the script fails fast with a clear actionable error instead of calling
install_node. Update the logic around check_node and the SETUP_NON_INTERACTIVE
branch (referencing the check_node function and install_node invocation and the
SETUP_NON_INTERACTIVE variable) to print a helpful error message instructing
users to install Node.js or run an explicit auto-install flag, and exit with a
non-zero status; keep install_node only reachable from the interactive prompt or
a new explicit --auto-install flag handler.
Comment on lines
117
to
+123
| afterAll(async () => { | ||
| // Delete in FK-dependency order: virtualCard/ledger/pot -> intent -> user | ||
| await prisma.virtualCard.deleteMany({ where: { intentId } }).catch(() => {}); | ||
| await prisma.ledgerEntry.deleteMany({ where: { intentId } }).catch(() => {}); | ||
| await prisma.pot.deleteMany({ where: { intentId } }).catch(() => {}); | ||
| await prisma.virtualCard.deleteMany({ where: { intentId } }).catch(() => {}); | ||
| await prisma.purchaseIntent.deleteMany({ where: { id: intentId } }).catch(() => {}); | ||
| await prisma.user.deleteMany({ where: { id: userId } }).catch(() => {}); | ||
| await prisma.purchaseIntent.delete({ where: { id: intentId } }).catch(() => {}); | ||
| await prisma.user.delete({ where: { id: userId } }).catch(() => {}); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd tests/integration/stripe && wc -l reconciliation.test.tsRepository: JonasBaeumer/AgentWallet
Length of output: 95
🏁 Script executed:
cd tests/integration/stripe && head -n 150 reconciliation.test.ts | tail -n +1Repository: JonasBaeumer/AgentWallet
Length of output: 4945
🏁 Script executed:
cd tests/integration/stripe && sed -n '1,150p' reconciliation.test.tsRepository: JonasBaeumer/AgentWallet
Length of output: 4945
🏁 Script executed:
cd tests/integration/stripe && grep -n "intentId\|userId" reconciliation.test.ts | head -n 20Repository: JonasBaeumer/AgentWallet
Length of output: 1070
🏁 Script executed:
cd tests/integration/stripe && sed -n '1,120p' reconciliation.test.tsRepository: JonasBaeumer/AgentWallet
Length of output: 3875
🌐 Web query:
Prisma undefined where clause behavior deleteMany
💡 Result:
In Prisma Client, passing undefined inside a where filter (including on a destructive operation like deleteMany) causes Prisma to omit that filter predicate (it behaves as if you didn’t provide that key), which can unintentionally broaden the query (potentially deleting all rows if the rest of the where becomes empty/absent) [1][2]. What this means for deleteMany 1) If you effectively end up with an empty where, deleteMany will delete all rows for that model/table. Prisma documents that deleteMany({}) deletes all records, and deleteMany() without filters is the “delete all” form [3][4]. 2) If you build a where object dynamically and one of its properties becomes undefined, Prisma will drop that condition, so your deleteMany may turn into “delete by nothing” (broader than intended). This footgun is explicitly called out in Prisma’s docs warning and has been reported in practice (e.g., where: { shop: undefined } deleting the entire table) [1][5]. Safety / mitigation - Enable Prisma’s strictUndefinedChecks preview feature so explicitly setting a query field to undefined becomes a runtime error instead of being silently omitted [1][2]. - Use Prisma.skip instead of undefined when you want Prisma to ignore a part of the query [2]. Example - Risky (predicate dropped when value is undefined): prisma.user.deleteMany({ where: { id } }) - Safer (explicit skip): prisma.user.deleteMany({ where: { id: id?? Prisma.skip } }) [2]
Citations:
- 1: updateMany and deleteMany don't delete more than one record. prisma/prisma#6732
- 2: Every condition on a deleteMany call is satisfied if none of the records meet the criteria prisma/prisma#18193
- 3:
deleteManydoes not actually delete data for specific schemas prisma/prisma#18185 - 4:
where: { shop: undefined }: The entire table data was accidentally deleted prisma/prisma#20169 - 5: https://www.prisma.io/docs/orm/prisma-client/special-fields-and-types/null-and-undefined
🏁 Script executed:
find . -name "tsconfig.json" -type f | head -5 | xargs catRepository: JonasBaeumer/AgentWallet
Length of output: 4146
🏁 Script executed:
find . -path ./node_modules -prune -o -name "prisma.json" -o -name "schema.prisma" -type f | grep -v node_modules | head -10Repository: JonasBaeumer/AgentWallet
Length of output: 91
🏁 Script executed:
cat prisma/schema.prisma | head -50Repository: JonasBaeumer/AgentWallet
Length of output: 620
Guard teardown deletes until the IDs exist.
If beforeAll fails before intentId or userId is assigned, these deleteMany calls pass undefined to Prisma, which treats it as a filter omission and can delete all rows from those tables.
Suggested fix
afterAll(async () => {
// Delete in FK-dependency order: virtualCard/ledger/pot -> intent -> user
- await prisma.virtualCard.deleteMany({ where: { intentId } }).catch(() => {});
- await prisma.ledgerEntry.deleteMany({ where: { intentId } }).catch(() => {});
- await prisma.pot.deleteMany({ where: { intentId } }).catch(() => {});
- await prisma.purchaseIntent.delete({ where: { id: intentId } }).catch(() => {});
- await prisma.user.delete({ where: { id: userId } }).catch(() => {});
+ if (intentId) {
+ await prisma.virtualCard.deleteMany({ where: { intentId } }).catch(() => {});
+ await prisma.ledgerEntry.deleteMany({ where: { intentId } }).catch(() => {});
+ await prisma.pot.deleteMany({ where: { intentId } }).catch(() => {});
+ await prisma.purchaseIntent.delete({ where: { id: intentId } }).catch(() => {});
+ }
+ if (userId) {
+ await prisma.user.delete({ where: { id: userId } }).catch(() => {});
+ }
await prisma.$disconnect();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| afterAll(async () => { | |
| // Delete in FK-dependency order: virtualCard/ledger/pot -> intent -> user | |
| await prisma.virtualCard.deleteMany({ where: { intentId } }).catch(() => {}); | |
| await prisma.ledgerEntry.deleteMany({ where: { intentId } }).catch(() => {}); | |
| await prisma.pot.deleteMany({ where: { intentId } }).catch(() => {}); | |
| await prisma.virtualCard.deleteMany({ where: { intentId } }).catch(() => {}); | |
| await prisma.purchaseIntent.deleteMany({ where: { id: intentId } }).catch(() => {}); | |
| await prisma.user.deleteMany({ where: { id: userId } }).catch(() => {}); | |
| await prisma.purchaseIntent.delete({ where: { id: intentId } }).catch(() => {}); | |
| await prisma.user.delete({ where: { id: userId } }).catch(() => {}); | |
| afterAll(async () => { | |
| // Delete in FK-dependency order: virtualCard/ledger/pot -> intent -> user | |
| if (intentId) { | |
| await prisma.virtualCard.deleteMany({ where: { intentId } }).catch(() => {}); | |
| await prisma.ledgerEntry.deleteMany({ where: { intentId } }).catch(() => {}); | |
| await prisma.pot.deleteMany({ where: { intentId } }).catch(() => {}); | |
| await prisma.purchaseIntent.delete({ where: { id: intentId } }).catch(() => {}); | |
| } | |
| if (userId) { | |
| await prisma.user.delete({ where: { id: userId } }).catch(() => {}); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/stripe/reconciliation.test.ts` around lines 117 - 123, The
teardown in afterAll calls prisma.deleteMany/delete with intentId and
prisma.delete with userId without checking they were assigned, which can pass
undefined and remove all rows; update the afterAll to guard each cleanup by
checking the identifiers (intentId and userId) exist before calling
prisma.virtualCard.deleteMany, prisma.ledgerEntry.deleteMany,
prisma.pot.deleteMany, prisma.purchaseIntent.delete, and prisma.user.delete
(e.g., only call the intent-related deleteMany/delete when intentId is truthy
and only call the user delete when userId is truthy) so no deletion runs when
IDs were never set.
Comment on lines
+145
to
+147
| // Filter by chat.id (not from.id) — in a group chat, from.id is the | ||
| // tapping user's personal ID, while chat.id is the group ID we expect. | ||
| if (!cb || cb.message?.chat?.id !== TEST_CHAT_ID) continue; |
Contributor
There was a problem hiding this comment.
The group-chat fix is incomplete unless you carry the real sender ID through.
Filtering by chat.id is the right first step, but these helpers still discard from.id, and the forwarded webhook payload later reuses TEST_CHAT_ID as the synthetic sender. In a real group update, chat.id and from.id are different, so this suite can still miss group-chat regressions while appearing to support them. Return the original cb.from.id / msg.from.id from the wait helpers and pass that through when forwarding the update.
Also applies to: 215-215
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/telegram/preferences.live.test.ts` around lines 145 - 147,
The test helper currently only filters by chat.id (cb.message?.chat?.id) but
drops the original sender id, causing forwarded webhook payloads to reuse
TEST_CHAT_ID as the synthetic sender; modify the wait helpers that return
callbacks/messages (the code paths referencing cb and msg in these helpers) to
preserve and return the original from.id (e.g., cb.from.id or msg.from.id)
alongside the message, and update the forwarding logic to use that preserved
sender id instead of TEST_CHAT_ID when constructing the synthetic sender for
forwarded updates so group sender identity is maintained.
Collaborator
|
Re-checked after the May 7 merge commits: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a hybrid shell + TypeScript onboarding script (
npm run setup) that guides new developers through the complete local setup — from prerequisites to a running, verified environment.Type of change
Module(s) affected
Checklist
npm testpasses locallysrc/contracts/if shared across modules.env.exampleupdated if new env vars are introducedHow to test
.envand stop Docker, then runnpm run setup— should walk through all 8 phases interactivelynpm run setupagain immediately — should detect existing state and skip completed stepsnpm run setup -- --non-interactive— should complete without promptsnpm run setup— should fail gracefully at Phase 1 with a clear messageNotes for reviewer
@clack/prompts@0.11.0(not latest 1.x) because 1.x dropped CommonJS support which our ts-node setup requiresscripts/andsetup.sh— no changes tosrc/modulesaccounts.retrieve,issuing.cards.list,balance.retrieve)Follow-ups
scripts/setup/*(acknowledged as intentional follow-up; the script is interactive/side-effectful and needs a mocking strategy)connectionFlowCI-friendly, decide what to do with interactive walkthroughsSummary by CodeRabbit
New Features
Documentation
Bug Fixes