Add service job provider runtime#9
Conversation
| process.kill(pid, "SIGTERM"); | ||
| stopped += 1; | ||
| } catch {} | ||
| break; |
There was a problem hiding this comment.
Unconditional break prevents fallback to legacy PID files
Medium Severity
The break statement in the stop command's PID file loop is unconditional — it sits outside the try/catch, so it always executes after the first existing PID file is found, even when process.kill() throws (e.g., stale PID). This prevents the loop from falling through to check the legacy PID file path, which is the entire reason the loop and pidFiles array exist.
Reviewed by Cursor Bugbot for commit 6eccf1e. Configure here.
| "package-lock.json", | ||
| "tsconfig.json", | ||
| ]) { | ||
| const source = resolve(process.cwd(), relativePath); |
There was a problem hiding this comment.
Deploy bundle copies from cwd instead of CLI source
Medium Severity
copyRuntimeBundle resolves CLI source directories (bin, src, serve, etc.) relative to process.cwd(), but process.cwd() is the user's working directory, not the CLI package's installation directory. The init command correctly uses import.meta.url to locate scaffold files, but deploy does not, so the bundle will silently miss CLI source files if the command is run from a directory other than the repo root.
Reviewed by Cursor Bugbot for commit 6eccf1e. Configure here.
| ), | ||
| opts.bundleDir, | ||
| ); | ||
| commands.push(result.command); |
There was a problem hiding this comment.
MPP secret key exposed in command-line arguments
Medium Severity
The MPP_SECRET_KEY is passed as a command-line argument to railway variable set via the envVars array, making it visible in process listings. The agent token is correctly protected by passing it via --stdin, but the MPP secret key (a cryptographic secret used for signing payment challenges) receives no such protection.
Reviewed by Cursor Bugbot for commit 3ceb483. Configure here.
| input: unknown; | ||
| }; | ||
| const originalEnv = process.env; | ||
| process.env = {}; |
There was a problem hiding this comment.
Sandbox environment clearing breaks handler module resolution
Medium Severity
Setting process.env = {} before import(handlerPath) clears the entire environment, which can break Node.js module resolution and any handler dependencies that read environment variables at import time (e.g., NODE_PATH, database connection strings). The finally block restores process.env only after execution completes, which is too late for import()-time env reads. Additionally, the sandbox provides minimal actual isolation since the handler can access workerData, the filesystem, and the network.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit be65840. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 6 total unresolved issues (including 4 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7338dc2. Configure here.
| const existing = getAgentToken(wallet); | ||
| if (existing && !isTokenExpired(existing)) return existing; | ||
|
|
||
| const chainId = Number(process.env.ACP_CHAIN_ID || "84532"); |
There was a problem hiding this comment.
Auth token chain ID defaults to wrong network
High Severity
getOrCreateAgentToken defaults chainId to "84532" (Base Sepolia testnet) when ACP_CHAIN_ID is not set, while getDefaultChainId() in the payment system defaults to base.id (8453, Base mainnet). This means the auth token is generated for a different chain than the one used for payment processing, likely causing authentication failures when no explicit ACP_CHAIN_ID env var is provided.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7338dc2. Configure here.
| const chainId = getDefaultChainId(); | ||
| const network = `eip155:${chainId}` as Network; | ||
| const asset = DEFAULT_STABLECOINS[network].address; | ||
| const decimals = DEFAULT_STABLECOINS[network].decimals; |
There was a problem hiding this comment.
Missing null check on stablecoin lookup in MPP
Medium Severity
DEFAULT_STABLECOINS[network] is accessed directly for .address and .decimals without checking if it's defined. If the network isn't supported, this throws an unhelpful TypeError. The equivalent code in x402.ts properly guards with if (!asset) throw new Error(...) before accessing properties.
Reviewed by Cursor Bugbot for commit 7338dc2. Configure here.


Summary
acp servecommands for service-job provider runtimes: init, start, endpoints, deploy bundle, stop, logs--settle-8183reserved but disabled for x402/MPP until ERC-8183 service-job support existsNotes
ACP_AGENT_TOKENand outbound access to BETests
npm run buildtsx bin/acp.ts serve endpoints --dir /tmp/acp-cli-smoke --jsonNote
High Risk
Adds a new provider runtime that verifies and settles x402/MPP payments (on-chain transactions) and introduces new auth-token issuance/storage and outbound Socket.IO relays, which are security- and funds-sensitive paths.
Overview
Adds a new
acp servecommand suite (init,start,endpoints,deploy,stop,logs) to scaffold and run a provider “service-job” runtime that executes a developerhandler.tsfor selected offerings.Introduces a
serve/runtime/server that connects outbound to the BE/service-jobsSocket.IO namespace, builds payment challenges, verifies and settles x402 (EIP-3009) and MPP (tempo viamppx) payments, and returns deliverables + protocol response headers back to BE;--settle-8183is explicitly reserved/blocked for relay-based protocols.Adds sandboxed handler execution support, offering/budget scaffolding templates, and a deployable bundle flow (Dockerfile + optional Railway CLI execution). Also updates config/auth plumbing to support
ACP_API_URLselection and persistedACP_AGENT_TOKENfetched via signed message (/auth/agent).Reviewed by Cursor Bugbot for commit 7338dc2. Bugbot is set up for automated code reviews on this repo. Configure here.