feat(expenses): add shared expense ledger MVP#83
Conversation
Add bearer-token enforcement, durable file-backed server storage, runtime request validation, and settlement invariant checks for the shared expense ledger MVP. Also tightens automated expense grants so scheduled grants cannot authorize manual settlements, and documents the server auth configuration.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53a3ee8c5b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .map((participant) => `${participant.chain}:${participant.agentId}`) | ||
| .sort() | ||
| .map((value) => value.replace(/[^a-zA-Z0-9]+/g, "_")); | ||
|
|
||
| return `expgrp_${normalized.join("_")}`; |
There was a problem hiding this comment.
The group ID derivation is lossy because it replaces every non-alphanumeric character with _ and then joins participant tokens with _, so distinct participant sets can collapse to the same groupId (e.g. different chain strings that only differ by punctuation placement). Since this groupId is used as the key for upsertGroup and expense/settlement lookups, a collision can merge unrelated ledgers and corrupt balances/history across groups.
Useful? React with 👍 / 👎.
| const existing = await this.store.findExpenseByIdempotencyKey(groupId, input.idempotencyKey); | ||
| if (existing) { | ||
| return existing; |
There was a problem hiding this comment.
In logExpense, idempotency is checked with findExpenseByIdempotencyKey before insertion, but the check and appendExpense happen in separate async operations without a shared lock/transaction. Under concurrent retries with the same idempotencyKey, two requests can both observe “not found” and both append, creating duplicate expenses and inflating group balances.
Useful? React with 👍 / 👎.
| const existing = await this.store.findSettlementByIdempotencyKey( | ||
| input.groupId, | ||
| input.idempotencyKey, | ||
| ); | ||
| if (existing) { |
There was a problem hiding this comment.
createSettlementIntent has the same non-atomic read-then-write pattern for idempotency: it checks findSettlementByIdempotencyKey and later appends the intent in a separate operation. Concurrent duplicate requests can race past the existence check and persist multiple pending intents for the same logical settlement, which can trigger duplicate payment attempts downstream.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 53a3ee8. Configure here.
| return whole.toString(); | ||
| } | ||
| return `${whole}.${fraction.toString().padStart(USDC_DECIMALS, "0").replace(/0+$/, "")}`; | ||
| } |
There was a problem hiding this comment.
formatUsdcMinor produces garbage output for negative values
Medium Severity
formatUsdcMinor is a public exported API accepting bigint | string but silently produces corrupt output for negative values. For -22500000n, BigInt modulo preserves sign so fraction is -500000n; its toString() is "-500000", which padStart(6, "0") leaves unchanged (length 7 > 6), producing "-22.-5" instead of "-22.5". Current callers in formatSignedUsdc work around this by negating before calling, but nothing prevents a future consumer from passing a negative value directly to this exported function and getting silently corrupted financial output.
Reviewed by Cursor Bugbot for commit 53a3ee8. Configure here.
| }; | ||
|
|
||
| return await this.store.appendExpense(expense); | ||
| } |
There was a problem hiding this comment.
Ledger idempotency check has TOCTOU race condition
Medium Severity
Both logExpense and createSettlementIntent have a time-of-check-to-time-of-use race between the idempotency key lookup and the append. Multiple await points separate the read from the write, so concurrent HTTP requests with the same idempotency key can both pass the check and both append, creating duplicate expenses or settlement intents. This breaks the claimed duplicate-settlement protection and could cause incorrect financial balances.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 53a3ee8. Configure here.
|
|
||
| export function settlementAmountDecimal(intent: ExpenseSettlementIntent): string { | ||
| return formatUsdcMinor(intent.amountMinor); | ||
| } |
There was a problem hiding this comment.
Exported settlementAmountDecimal function is never used
Low Severity
settlementAmountDecimal is exported from ledger.ts (and re-exported via index.ts) but has zero imports anywhere in the codebase. It's dead code that adds to the public API surface without any consumer.
Reviewed by Cursor Bugbot for commit 53a3ee8. Configure here.
1 participant
Fixes #37.
Adds the shared expense ledger path for agents that already hold stablecoins: agents can log group expenses, publish shared-expense grants, compute net obligations, and settle only the final balance in USDC instead of putting every expense on-chain. The ledger server is centralized but hardened for the MVP with bearer-token auth on ledger routes, file-backed persistence by default, runtime request validation, member/invariant checks, and duplicate-settlement protection. CLI setup now accepts an API token and the Trusted Agents skill documents the server configuration.
Validated with
bun run lint,bun run typecheck,bun run build, andbun run test.Note
Medium Risk
Introduces new ledger/business logic and a new HTTP service plus CLI commands that compute and persist balances/settlement intents; mistakes could lead to incorrect netting or settlement instructions, though actual on-chain transfers remain out of scope.
Overview
Adds a new shared-expense MVP that lets agents log off-chain expenses, compute net USDC balances, and generate a settlement intent rather than executing per-expense transfers.
Introduces
@trustedagents/app-expensesfor shared types, USDC amount parsing/formatting, equal-split math, deterministic group IDs, andexpense/settlegrant matching.Adds a new
trusted-agents-expense-serverNode HTTP service with request validation, optional bearer-token auth, in-memory/file-backed persistence, idempotent expense + settlement intent creation, and balance/history endpoints; wires it into the workspace build/typecheck/test flow.Extends the
tapCLI withtap expensescommands (setup,group create,log,balance,history,settle) including config/env handling for server URL, API token, and settlement address, and updates TAP skill/docs/specs accordingly.Reviewed by Cursor Bugbot for commit 53a3ee8. Bugbot is set up for automated code reviews on this repo. Configure here.