feat: add agent payment history endpoint#241
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Payment-history endpoint and policy route updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
CI StatusSame setup-phase infra failures as PRs #240 and #242 — unrelated to this PR.
None of these jobs reached the test execution step. The payment history endpoint changes are unaffected. |
- Resolve merge conflict in backend/src/routes/agents.js: keep both POST /update-policy and PUT /policy routes - Add payment-history tests from PR branch - Fix orphaned braces in test/agents.test.js - Resolve agent/agent.js merge conflicts (take upstream/main)
- Remove duplicate SERVICE_MAX_TTL export in contract.js - Remove duplicate annotateTtlWarning in registry.js - Remove duplicate mock/SERVICE_MAX_TTL declarations in registry.test.js - Fix creditPayment() in services.js: call recordPaymentOnChain - Use vi.hoisted() for mock variables in test/agents.test.js - Resolve merge conflict in agents.js (keep both POST + PUT policy routes)
- Remove cache:npm + cache-dependency-path (package-lock.json is gitignored) - Replace npm ci with npm install --ignore-scripts - Download pre-built stellar-cli binary instead of cargo install - All 4 CI jobs should now pass without requiring lockfiles
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@agent/agent.js`:
- Around line 26-28: Normalize the configured category list before it is used in
agent/agent.js: the ALLOWED_CATS initialization currently preserves whitespace
and empty values from process.env.ALLOWED_CATEGORIES. Update the parsing logic
around ALLOWED_CATS to trim each split entry and filter out blanks before
registration, so values like "weather, search," become clean category names
only.
- Around line 322-328: When `agent/agent.js` hits the `catch (err)` path in the
service attempt loop, it only logs and retries, so thrown payment/fetch failures
never flow through the same failure handling as the non-OK branch. Update the
`catch` block to record a failed outcome and apply the same reputation/negative
scoring logic used for rejected responses, using the existing `selected`,
`category`, `failed`, and `logger` flow so exceptions are treated as failed
service attempts too.
- Line 251: The runTask function now requires a valid httpClient because it
calls httpClient.fetch unconditionally, so update the remaining runTask call
sites in agent/agent.test.js to pass a compatible HTTP client stub/mock. Use the
runTask export in agent/agent.js as the reference and make sure every invocation
supplies the fourth argument so the tests don’t fail with an undefined fetch
call.
In `@backend/src/routes/services.js`:
- Around line 45-50: Add an idempotency guard before the background on-chain
write in the services payment flow. The issue is in the payment-credited path
that logs `txHash` but calls `recordPaymentOnChain(agentAddress, serviceId,
priceStroops, true)` without using it, so retries can double-record the same
payment. Update this flow in the relevant handler to dedupe by `txHash` (or pass
`txHash` through the `recordPaymentOnChain`/contract dedupe path) and only
enqueue the on-chain write once per transaction, while keeping the existing
`logger.info` and error handling intact.
In `@backend/test/agents.test.js`:
- Around line 125-127: The invalid-pagination test is leaving an unused one-shot
mock queued because getActivityFeed() is never reached on that path. Remove the
mockReturnValueOnce([]) setup from the test, or reset mockGetActivityFeed in
beforeEach alongside vi.clearAllMocks() in agents.test.js so later tests do not
inherit stale mock state.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c9ea6297-cb65-43d3-b1c1-a0cfb4d6324e
📒 Files selected for processing (7)
agent/agent.jsbackend/src/lib/contract.jsbackend/src/routes/agents.jsbackend/src/routes/registry.jsbackend/src/routes/registry.test.jsbackend/src/routes/services.jsbackend/test/agents.test.js
✅ Files skipped from review due to trivial changes (3)
- backend/src/routes/registry.test.js
- backend/src/routes/registry.js
- backend/src/lib/contract.js
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/routes/agents.js
| logger.info({ agentAddress, txHash, serviceId, priceInStroops: priceStroops.toString() }, `${serviceLabel} payment credited to registered agent`); | ||
|
|
||
| // Record the payment on-chain in the background — never blocks the response. | ||
| recordPaymentOnChain(agentAddress, serviceId, priceStroops, true).catch((err) => | ||
| logger.error({ err, agentAddress, serviceId }, `Failed to record ${serviceLabel} payment on-chain`) | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Add idempotency before recording payments on-chain.
Line 45 has txHash, but Line 48 records only (agentAddress, serviceId, amount, success). If the same paid request is retried or replayed with the same transaction hash, this path can double-count the payment on-chain. Use a durable idempotency guard keyed by txHash, or include the transaction hash in the contract-level dedupe path.
🤖 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 `@backend/src/routes/services.js` around lines 45 - 50, Add an idempotency
guard before the background on-chain write in the services payment flow. The
issue is in the payment-credited path that logs `txHash` but calls
`recordPaymentOnChain(agentAddress, serviceId, priceStroops, true)` without
using it, so retries can double-record the same payment. Update this flow in the
relevant handler to dedupe by `txHash` (or pass `txHash` through the
`recordPaymentOnChain`/contract dedupe path) and only enqueue the on-chain write
once per transaction, while keeping the existing `logger.info` and error
handling intact.
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'clearAllMocks|mockReset|mockReturnValueOnce|mockReturnValue' backend/test/agents.test.js backend/src/routes/agents.test.jsRepository: Stellar-Ecosystem/lodestar
Length of output: 8122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/test/agents.test.js uses =="
rg -n -C2 'mockGetActivityFeed|mockParseActivityPagination|beforeEach\(\)' backend/test/agents.test.js
echo
echo "== backend/src/routes/agents.test.js uses =="
rg -n -C2 'mockGetActivityFeed|mockParseActivityPagination|beforeEach\(\)' backend/src/routes/agents.test.jsRepository: Stellar-Ecosystem/lodestar
Length of output: 4237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/test/agents.test.js file size =="
wc -l backend/test/agents.test.js
echo
echo "== backend/src/routes/agents.test.js file size =="
wc -l backend/src/routes/agents.test.js
echo
echo "== backend/test/agents.test.js around payment-history block =="
sed -n '115,205p' backend/test/agents.test.jsRepository: Stellar-Ecosystem/lodestar
Length of output: 3453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/test/agents.test.js: relevant lines =="
sed -n '1,260p' backend/test/agents.test.js
echo
echo "== backend/test/agents.test.js: all payment-history tests =="
rg -n 'payment-history|mockGetActivityFeed|mockParseActivityPagination' backend/test/agents.test.jsRepository: Stellar-Ecosystem/lodestar
Length of output: 8319
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l backend/test/agents.test.js
printf '\n== tail ==\n'
tail -n 40 backend/test/agents.test.js
printf '\n== all mockGetActivityFeed / mockParseActivityPagination uses ==\n'
rg -n 'mockGetActivityFeed|mockParseActivityPagination|beforeEach\(\)' backend/test/agents.test.jsRepository: Stellar-Ecosystem/lodestar
Length of output: 2716
Remove the unused one-shot mock here. getActivityFeed() isn’t called on the invalid-pagination path, so mockReturnValueOnce([]) stays queued and can affect a later test. Delete that line, or reset mockGetActivityFeed in beforeEach if you want stricter isolation.
🤖 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 `@backend/test/agents.test.js` around lines 125 - 127, The invalid-pagination
test is leaving an unused one-shot mock queued because getActivityFeed() is
never reached on that path. Remove the mockReturnValueOnce([]) setup from the
test, or reset mockGetActivityFeed in beforeEach alongside vi.clearAllMocks() in
agents.test.js so later tests do not inherit stale mock state.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/ci.yml:
- Around line 47-51: The Install Stellar CLI step downloads and installs a
release binary without verifying its integrity, so update this workflow to
validate the Stellar CLI asset before moving it into place. In the “Install
Stellar CLI” step, add checksum and/or signature verification for the file
fetched by gh release download before the tar extraction and sudo mv into
/usr/local/bin/stellar, and only proceed with installation after validation
succeeds.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a2e93c40-460b-40dc-a46c-fcb78fabb154
📒 Files selected for processing (1)
.github/workflows/ci.yml
…se wget for stellar-cli
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/tsconfig.json`:
- Line 20: Remove the __tests__ exclusion from the frontend tsconfig so the
CI-used TypeScript config still type-checks frontend/__tests__ TS/TSX files; if
tests need different settings, create a separate test-specific tsconfig instead
of changing the main config. Update the exclude array in tsconfig.json
accordingly and keep the main config aligned with npx tsc --noEmit.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b7445244-cda8-4281-9ad8-b0ec26cd83c2
📒 Files selected for processing (3)
.github/workflows/ci.ymlagent/agent.test.jsfrontend/tsconfig.json
| }, | ||
| "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"], | ||
| "exclude": ["node_modules"] | ||
| "exclude": ["node_modules", "__tests__"] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== frontend tsconfig files =="
fd -t f 'tsconfig.*\.json$' frontend
echo
echo "== TypeScript files under frontend/__tests__ =="
fd -t f -e ts -e tsx . frontend/__tests__ || true
echo
echo "== CI typecheck command =="
sed -n '100,115p' .github/workflows/ci.ymlRepository: Stellar-Ecosystem/lodestar
Length of output: 1282
Don't exclude __tests__ from the only TypeScript config CI uses.
frontend/__tests__ contains TS/TSX files, and frontend CI runs npx tsc --noEmit against this config. Excluding __tests__ removes those tests from type-checking; if the build needs a narrower config, split it into a separate tsconfig instead.
Minimal fix
- "exclude": ["node_modules", "__tests__"]
+ "exclude": ["node_modules"]📝 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.
| "exclude": ["node_modules", "__tests__"] | |
| "exclude": ["node_modules"] |
🤖 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 `@frontend/tsconfig.json` at line 20, Remove the __tests__ exclusion from the
frontend tsconfig so the CI-used TypeScript config still type-checks
frontend/__tests__ TS/TSX files; if tests need different settings, create a
separate test-specific tsconfig instead of changing the main config. Update the
exclude array in tsconfig.json accordingly and keep the main config aligned with
npx tsc --noEmit.
…est expectations Restores structured logging (serviceName, priceUsdc, servicesFound, attempt fields), encodePaymentSignatureHeader, minReputation threshold, weighted fallback retry, and payment_failed error details that agent.test.js now expects (updated in PR Stellar-Ecosystem#240).
2 participants
Closes #83
What changed:
GET /api/agents/:address/payment-historyinbackend/src/routes/agents.js— filters the activity feed by agent address and returns a paginated list of payment events.activityFeedmodule (getActivityFeed,parseActivityPagination) — no new dependencies.backend/src/routes/agents.test.js.Response shape:
{ "payments": [{"agent": "GA…", "txHash": "…", "service": "…", "amount": "…"}], "pagination": {"total": 5, "limit": 20, "offset": 0, "hasMore": false} }Tests pass locally (14/14 tests in agents.test.js). Pre-existing failures in
test/agents.test.jsare unrelated to this PR.Summary by CodeRabbit
New Features
Bug Fixes