fix: close the orphan-leg hole and add idempotency keys (frontier A1 + B3)
Two long-standing correctness gaps, both flagged in fin-research-frontier. A1 — POST /api/transactions could write half a linked pair TransactionCreate inherited the full six-value type Literal and accepted a free-standing linked_account_id, so any client could create a lone `transfer` row with no partner. That is exactly the balance corruption invariant #1 exists to prevent: the orphan counts toward SUM(amount - fee) and paired-delete can never find a partner to remove it with. TransactionCreate now narrows type to the single-leg kinds and rejects a caller-supplied linked_account_id. TransactionBase stays wide on purpose — reads must keep describing historical rows of every type. splitwise_pay deliberately stays allowed. The two-leg creator uses that type, but a standalone row carrying splitwise_partner_share is also a legitimate record, and is what `fin tx add --type splitwise_pay --partner-share` writes. Narrowing to expense/income would have broken it. B3 — no idempotency, so any retry double-booked Migration i9d0e1f2a3b4 adds a nullable client_ref to transactions. POST /api/transactions, /batch, /transfer and /settlement accept it, and crud.find_by_client_ref is checked before writing, so retrying a timed-out request replays the original rows instead of booking them again. NULL means "no idempotency requested" and never collides. Deviation from the frontier's sketch, which called for unique-when-not-null: the index is NOT unique. A transfer writes two rows and a batch writes many, all under one key, so uniqueness cannot live on the column. Suppression is a check-then-write inside the creators instead — sound here because SQLite serialises writers and the scheduler never writes transactions. Exposed on the MCP create_transaction/create_transfer/create_settlement tools, since a retrying agent is the case B3 was written about. The server instructions no longer tell agents that writes are non-idempotent. Tests: 60 passing, up from 47. Thirteen new cover retries collapsing to one write across all four endpoints, differing keys staying separate, NULL keys not colliding, and the orphan-leg guard refusing transfer/settlement/manual linkage while the four legitimate single-leg types still work. Migration verified both directions on a scratch DB seeded at the previous head: existing rows survive with NULL client_ref, index is created, and downgrade drops cleanly with rows intact. Verified live over HTTP that a transfer submitted three times with one client_ref yields one pair of legs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3qYLuu9jgXsCRXmJ18YGY
This commit is contained in:
1 parent
3eb5fdbe55
commit
c9a1375fb9
8 files changed
+313
-5
No files matched your search
@@ -83,6 +83,8 @@ cli/
|
|||||||
|
|
||||||
`Transaction` belongs to one account and has a `type`: `expense`, `income`, `transfer`, `splitwise_pay`, `splitwise_receive`, or `settlement`. Transfers and settlements always produce two linked rows — a debit on one account and a credit on another — connected via `linked_account_id`. The `fee` column is included in balance calculations (`amount - fee`). For Splitwise pay transactions, `splitwise_partner_share` records the partner's portion (used in spending calculations to show only the user's net share).
|
`Transaction` belongs to one account and has a `type`: `expense`, `income`, `transfer`, `splitwise_pay`, `splitwise_receive`, or `settlement`. Transfers and settlements always produce two linked rows — a debit on one account and a credit on another — connected via `linked_account_id`. The `fee` column is included in balance calculations (`amount - fee`). For Splitwise pay transactions, `splitwise_partner_share` records the partner's portion (used in spending calculations to show only the user's net share).
|
||||||
|
|
||||||
|
**Idempotency**: every write that moves money accepts an optional `client_ref` — `POST /api/transactions`, `/batch`, `/transfer` and `/settlement`. Every row of one write carries the same key, and the creators check `crud.find_by_client_ref` before writing, so retrying a timed-out request replays the original rows instead of double-booking. The index is deliberately **not** unique (a transfer writes two rows under one key); duplicate suppression is a check-then-write, which is sound because SQLite serialises writers and the scheduler never writes transactions. A NULL `client_ref` means "no idempotency requested" and never collides.
|
||||||
|
|
||||||
`POST /api/transactions/batch` writes several single-leg rows sharing one account, date and currency in a single commit — all rows land or none do. It is the one write endpoint whose items carry **positive** amounts: the sign is derived server-side from the batch `type`, which makes a mixed-sign batch unrepresentable. Its `type` is restricted to `expense`/`income` for the same reason the MCP `create_transaction` tool is — a batched `transfer` would emit orphan legs (invariant #1). The UI entry point is the `Batch Expenses` QuickAdd flow.
|
`POST /api/transactions/batch` writes several single-leg rows sharing one account, date and currency in a single commit — all rows land or none do. It is the one write endpoint whose items carry **positive** amounts: the sign is derived server-side from the batch `type`, which makes a mixed-sign batch unrepresentable. Its `type` is restricted to `expense`/`income` for the same reason the MCP `create_transaction` tool is — a batched `transfer` would emit orphan legs (invariant #1). The UI entry point is the `Batch Expenses` QuickAdd flow.
|
||||||
|
|
||||||
`StockTransaction` records buy/sell/rsu_vest events and drives `Holding` state. Holdings are never edited directly — they are recomputed from the full stock transaction history on any delete, and incrementally updated on insert.
|
`StockTransaction` records buy/sell/rsu_vest events and drives `Holding` state. Holdings are never edited directly — they are recomputed from the full stock transaction history on any delete, and incrementally updated on insert.
|
||||||
@@ -91,7 +93,7 @@ cli/
|
|||||||
|
|
||||||
## Key invariants — never break these
|
## Key invariants — never break these
|
||||||
|
|
||||||
1. **Atomic two-leg writes**: `create_transfer` and `create_settlement` in crud.py use `db.add_all([leg1, leg2]); db.commit()`. Both legs must be written in a single transaction. Deleting one leg of a transfer without the other corrupts balances.
|
1. **Atomic two-leg writes**: `create_transfer` and `create_settlement` in crud.py use `db.add_all([leg1, leg2]); db.commit()`. Both legs must be written in a single transaction. Deleting one leg of a transfer without the other corrupts balances. This is now enforced at the edge too: `TransactionCreate` rejects `type` of `transfer`/`settlement` and rejects a caller-supplied `linked_account_id`, so `POST /api/transactions` can no longer write half a pair. Keep `TransactionBase` wide — reads must still describe historical rows of every type.
|
||||||
2. **Holding recompute on delete**: `delete_stock_transaction` calls `recompute_holding_from_stock_transactions` after the delete, before `db.commit()`. Skipping this leaves holdings inconsistent with the transaction log.
|
2. **Holding recompute on delete**: `delete_stock_transaction` calls `recompute_holding_from_stock_transactions` after the delete, before `db.commit()`. Skipping this leaves holdings inconsistent with the transaction log.
|
||||||
3. **WAL mode**: `PRAGMA journal_mode=WAL` is applied on every new connection (both the main engine and the scheduler's private engine). The scheduler runs in a background thread and must not share the request-scoped session.
|
3. **WAL mode**: `PRAGMA journal_mode=WAL` is applied on every new connection (both the main engine and the scheduler's private engine). The scheduler runs in a background thread and must not share the request-scoped session.
|
||||||
4. **Non-root Docker user**: The API Dockerfile creates user `app` (uid 1000) and runs as that user. Do not revert to root.
|
4. **Non-root Docker user**: The API Dockerfile creates user `app` (uid 1000) and runs as that user. Do not revert to root.
|
||||||
|
|||||||
@@ -61,6 +61,11 @@ Use `find_account` / `find_category` to turn a name into the integer id the
|
|||||||
other tools take. Both lists are also published as the resources
|
other tools take. Both lists are also published as the resources
|
||||||
`fin://accounts` and `fin://categories`.
|
`fin://accounts` and `fin://categories`.
|
||||||
|
|
||||||
|
`create_transaction`, `create_transfer` and `create_settlement` accept an
|
||||||
|
optional `client_ref`. Retrying a failed call with the same key returns the
|
||||||
|
write that already happened instead of booking it twice — worth setting on any
|
||||||
|
write an agent might have to retry.
|
||||||
|
|
||||||
**Transports.** `stdio` (default) suits clients that spawn the process.
|
**Transports.** `stdio` (default) suits clients that spawn the process.
|
||||||
`streamable-http` runs it as a service — it binds `FIN_MCP_HOST:FIN_MCP_PORT`
|
`streamable-http` runs it as a service — it binds `FIN_MCP_HOST:FIN_MCP_PORT`
|
||||||
and serves MCP at `FIN_MCP_PATH`:
|
and serves MCP at `FIN_MCP_PATH`:
|
||||||
|
|||||||
@@ -0,0 +1,37 @@
|
|||||||
|
"""add client_ref idempotency key to transactions
|
||||||
|
|
||||||
|
Revision ID: i9d0e1f2a3b4
|
||||||
|
Revises: h8c9d0e1f2a3
|
||||||
|
Create Date: 2026-08-22
|
||||||
|
|
||||||
|
Adds a nullable, caller-supplied idempotency key. Every row written by one
|
||||||
|
request carries the same value, so a retry after an ambiguous failure returns
|
||||||
|
the existing rows instead of booking them twice.
|
||||||
|
|
||||||
|
The index is deliberately NOT unique: a transfer writes two legs and a batch
|
||||||
|
writes many, all sharing one key. Duplicate suppression happens as a
|
||||||
|
check-then-write inside the crud creators. Backward-safe — the column is
|
||||||
|
nullable and every existing row keeps NULL, which the creators treat as
|
||||||
|
"no idempotency requested".
|
||||||
|
"""
|
||||||
|
from typing import Sequence, Union
|
||||||
|
|
||||||
|
from alembic import op
|
||||||
|
import sqlalchemy as sa
|
||||||
|
|
||||||
|
revision: str = "i9d0e1f2a3b4"
|
||||||
|
down_revision: Union[str, None] = "h8c9d0e1f2a3"
|
||||||
|
branch_labels: Union[str, Sequence[str], None] = None
|
||||||
|
depends_on: Union[str, Sequence[str], None] = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
with op.batch_alter_table("transactions") as batch_op:
|
||||||
|
batch_op.add_column(sa.Column("client_ref", sa.Text(), nullable=True))
|
||||||
|
op.create_index("ix_transactions_client_ref", "transactions", ["client_ref"])
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
op.drop_index("ix_transactions_client_ref", table_name="transactions")
|
||||||
|
with op.batch_alter_table("transactions") as batch_op:
|
||||||
|
batch_op.drop_column("client_ref")
|
||||||
@@ -149,9 +149,29 @@ def get_transaction(db: Session, transaction_id: int) -> models.Transaction | No
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def find_by_client_ref(db: Session, client_ref: str | None) -> list[models.Transaction]:
|
||||||
|
"""Return the rows a previous request already wrote under this key.
|
||||||
|
|
||||||
|
Empty when no key was supplied or nothing matches. Every creator that
|
||||||
|
accepts a client_ref calls this first, so retrying an ambiguous request
|
||||||
|
replays the original result instead of double-booking.
|
||||||
|
"""
|
||||||
|
if not client_ref:
|
||||||
|
return []
|
||||||
|
return (
|
||||||
|
db.query(models.Transaction)
|
||||||
|
.filter(models.Transaction.client_ref == client_ref)
|
||||||
|
.order_by(models.Transaction.id)
|
||||||
|
.all()
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def create_transaction(
|
def create_transaction(
|
||||||
db: Session, transaction: schemas.TransactionCreate
|
db: Session, transaction: schemas.TransactionCreate
|
||||||
) -> models.Transaction:
|
) -> models.Transaction:
|
||||||
|
existing = find_by_client_ref(db, transaction.client_ref)
|
||||||
|
if existing:
|
||||||
|
return existing[0]
|
||||||
db_transaction = models.Transaction(**transaction.model_dump())
|
db_transaction = models.Transaction(**transaction.model_dump())
|
||||||
db.add(db_transaction)
|
db.add(db_transaction)
|
||||||
db.commit()
|
db.commit()
|
||||||
@@ -169,6 +189,9 @@ def create_transactions_batch(
|
|||||||
accepted (see BatchTransactionRequest), so this never produces the orphan
|
accepted (see BatchTransactionRequest), so this never produces the orphan
|
||||||
legs that invariant #1 exists to prevent.
|
legs that invariant #1 exists to prevent.
|
||||||
"""
|
"""
|
||||||
|
existing = find_by_client_ref(db, req.client_ref)
|
||||||
|
if existing:
|
||||||
|
return existing
|
||||||
sign = -1 if req.type == "expense" else 1
|
sign = -1 if req.type == "expense" else 1
|
||||||
rows = [
|
rows = [
|
||||||
models.Transaction(
|
models.Transaction(
|
||||||
@@ -180,6 +203,7 @@ def create_transactions_batch(
|
|||||||
category_id=item.category_id,
|
category_id=item.category_id,
|
||||||
description=item.description,
|
description=item.description,
|
||||||
fee=item.fee,
|
fee=item.fee,
|
||||||
|
client_ref=req.client_ref,
|
||||||
)
|
)
|
||||||
for item in req.items
|
for item in req.items
|
||||||
]
|
]
|
||||||
@@ -265,6 +289,9 @@ def create_transfer(
|
|||||||
db: Session, req: schemas.TransferRequest
|
db: Session, req: schemas.TransferRequest
|
||||||
) -> list[models.Transaction]:
|
) -> list[models.Transaction]:
|
||||||
"""Write both legs of a transfer atomically."""
|
"""Write both legs of a transfer atomically."""
|
||||||
|
existing = find_by_client_ref(db, req.client_ref)
|
||||||
|
if existing:
|
||||||
|
return existing
|
||||||
dest_amount = req.to_amount if req.to_amount is not None else req.amount
|
dest_amount = req.to_amount if req.to_amount is not None else req.amount
|
||||||
dest_currency = req.to_currency if req.to_currency is not None else req.currency
|
dest_currency = req.to_currency if req.to_currency is not None else req.currency
|
||||||
from_leg = models.Transaction(
|
from_leg = models.Transaction(
|
||||||
@@ -276,6 +303,7 @@ def create_transfer(
|
|||||||
description=req.description,
|
description=req.description,
|
||||||
linked_account_id=req.to_account_id,
|
linked_account_id=req.to_account_id,
|
||||||
fee=req.fee,
|
fee=req.fee,
|
||||||
|
client_ref=req.client_ref,
|
||||||
)
|
)
|
||||||
to_leg = models.Transaction(
|
to_leg = models.Transaction(
|
||||||
date=req.date,
|
date=req.date,
|
||||||
@@ -286,6 +314,7 @@ def create_transfer(
|
|||||||
description=req.description,
|
description=req.description,
|
||||||
linked_account_id=req.from_account_id,
|
linked_account_id=req.from_account_id,
|
||||||
fee=0,
|
fee=0,
|
||||||
|
client_ref=req.client_ref,
|
||||||
)
|
)
|
||||||
db.add_all([from_leg, to_leg])
|
db.add_all([from_leg, to_leg])
|
||||||
db.flush()
|
db.flush()
|
||||||
@@ -305,6 +334,9 @@ def create_settlement(
|
|||||||
direction='splitwise_to_bank': she paid me — bank +amount, splitwise -amount
|
direction='splitwise_to_bank': she paid me — bank +amount, splitwise -amount
|
||||||
direction='bank_to_splitwise': i paid her — bank -amount, splitwise +amount
|
direction='bank_to_splitwise': i paid her — bank -amount, splitwise +amount
|
||||||
"""
|
"""
|
||||||
|
existing = find_by_client_ref(db, req.client_ref)
|
||||||
|
if existing:
|
||||||
|
return existing
|
||||||
if req.direction == "splitwise_to_bank":
|
if req.direction == "splitwise_to_bank":
|
||||||
bank_amount = req.amount
|
bank_amount = req.amount
|
||||||
splitwise_amount = -req.amount
|
splitwise_amount = -req.amount
|
||||||
@@ -321,6 +353,7 @@ def create_settlement(
|
|||||||
description="Splitwise settlement",
|
description="Splitwise settlement",
|
||||||
linked_account_id=req.splitwise_account_id,
|
linked_account_id=req.splitwise_account_id,
|
||||||
fee=0,
|
fee=0,
|
||||||
|
client_ref=req.client_ref,
|
||||||
)
|
)
|
||||||
splitwise_leg = models.Transaction(
|
splitwise_leg = models.Transaction(
|
||||||
date=req.date,
|
date=req.date,
|
||||||
@@ -331,6 +364,7 @@ def create_settlement(
|
|||||||
description="Splitwise settlement",
|
description="Splitwise settlement",
|
||||||
linked_account_id=req.bank_account_id,
|
linked_account_id=req.bank_account_id,
|
||||||
fee=0,
|
fee=0,
|
||||||
|
client_ref=req.client_ref,
|
||||||
)
|
)
|
||||||
db.add_all([bank_leg, splitwise_leg])
|
db.add_all([bank_leg, splitwise_leg])
|
||||||
db.flush()
|
db.flush()
|
||||||
|
|||||||
@@ -68,6 +68,13 @@ class Transaction(Base):
|
|||||||
splitwise_partner_share = Column(Float, nullable=True)
|
splitwise_partner_share = Column(Float, nullable=True)
|
||||||
linked_account_id = Column(Integer, ForeignKey("accounts.id"), nullable=True)
|
linked_account_id = Column(Integer, ForeignKey("accounts.id"), nullable=True)
|
||||||
linked_transaction_id = Column(Integer, nullable=True)
|
linked_transaction_id = Column(Integer, nullable=True)
|
||||||
|
# Caller-supplied idempotency key. Shared by every row of one write, so a
|
||||||
|
# retried request returns the rows it already made instead of double-booking.
|
||||||
|
# Indexed but NOT unique: a two-leg or batched write puts the same value on
|
||||||
|
# several rows. Duplicate suppression is a check-then-write inside the
|
||||||
|
# creator, which is sound here because SQLite serialises writers and the
|
||||||
|
# scheduler never writes transactions.
|
||||||
|
client_ref = Column(Text, nullable=True, index=True)
|
||||||
created_at = Column(DateTime, nullable=False, default=datetime.utcnow)
|
created_at = Column(DateTime, nullable=False, default=datetime.utcnow)
|
||||||
|
|
||||||
account = relationship("Account", back_populates="transactions", foreign_keys=[account_id])
|
account = relationship("Account", back_populates="transactions", foreign_keys=[account_id])
|
||||||
|
|||||||
+27
-2
@@ -1,7 +1,7 @@
|
|||||||
from datetime import date, datetime
|
from datetime import date, datetime
|
||||||
from typing import Literal, Optional
|
from typing import Literal, Optional
|
||||||
|
|
||||||
from pydantic import BaseModel, ConfigDict, Field
|
from pydantic import BaseModel, ConfigDict, Field, model_validator
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -81,10 +81,32 @@ class TransactionBase(BaseModel):
|
|||||||
exchange_rate_used: Optional[float] = None
|
exchange_rate_used: Optional[float] = None
|
||||||
splitwise_partner_share: Optional[float] = None
|
splitwise_partner_share: Optional[float] = None
|
||||||
linked_account_id: Optional[int] = None
|
linked_account_id: Optional[int] = None
|
||||||
|
client_ref: Optional[str] = Field(default=None, max_length=100)
|
||||||
|
|
||||||
|
|
||||||
class TransactionCreate(TransactionBase):
|
class TransactionCreate(TransactionBase):
|
||||||
pass
|
"""A single, standalone transaction.
|
||||||
|
|
||||||
|
Narrower than TransactionBase, which must keep describing every historical
|
||||||
|
row for reads. `transfer` and `settlement` always come in linked pairs, so
|
||||||
|
one written here would be an orphan leg — the balance corruption invariant
|
||||||
|
#1 exists to prevent. Both have dedicated atomic endpoints. `splitwise_pay`
|
||||||
|
stays allowed: the two-leg creator uses that type, but a standalone row
|
||||||
|
carrying splitwise_partner_share is also a legitimate record and is what
|
||||||
|
`fin tx add --type splitwise_pay --partner-share` writes.
|
||||||
|
"""
|
||||||
|
|
||||||
|
type: Literal["expense", "income", "splitwise_pay", "splitwise_receive"]
|
||||||
|
|
||||||
|
@model_validator(mode="after")
|
||||||
|
def _reject_manual_linkage(self):
|
||||||
|
if self.linked_account_id is not None:
|
||||||
|
raise ValueError(
|
||||||
|
"linked_account_id cannot be set on a standalone transaction — "
|
||||||
|
"linked pairs are written by /transfer, /settlement, "
|
||||||
|
"/split-expense and /splitwise-i-paid, which set both sides atomically"
|
||||||
|
)
|
||||||
|
return self
|
||||||
|
|
||||||
|
|
||||||
class TransactionUpdate(BaseModel):
|
class TransactionUpdate(BaseModel):
|
||||||
@@ -95,6 +117,7 @@ class TransactionUpdate(BaseModel):
|
|||||||
|
|
||||||
|
|
||||||
class TransferRequest(BaseModel):
|
class TransferRequest(BaseModel):
|
||||||
|
client_ref: Optional[str] = Field(default=None, max_length=100)
|
||||||
from_account_id: int
|
from_account_id: int
|
||||||
to_account_id: int
|
to_account_id: int
|
||||||
amount: float
|
amount: float
|
||||||
@@ -107,6 +130,7 @@ class TransferRequest(BaseModel):
|
|||||||
|
|
||||||
|
|
||||||
class SettlementRequest(BaseModel):
|
class SettlementRequest(BaseModel):
|
||||||
|
client_ref: Optional[str] = Field(default=None, max_length=100)
|
||||||
bank_account_id: int
|
bank_account_id: int
|
||||||
splitwise_account_id: int
|
splitwise_account_id: int
|
||||||
amount: float
|
amount: float
|
||||||
@@ -159,6 +183,7 @@ class BatchTransactionRequest(BaseModel):
|
|||||||
date: date
|
date: date
|
||||||
currency: str = "EUR"
|
currency: str = "EUR"
|
||||||
type: Literal["expense", "income"] = "expense"
|
type: Literal["expense", "income"] = "expense"
|
||||||
|
client_ref: Optional[str] = Field(default=None, max_length=100)
|
||||||
items: list[BatchTransactionItem] = Field(min_length=1, max_length=100)
|
items: list[BatchTransactionItem] = Field(min_length=1, max_length=100)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+23
-2
@@ -86,8 +86,11 @@ def _instructions(allow_writes: bool) -> str:
|
|||||||
return base + (
|
return base + (
|
||||||
"\n\nWrite discipline (applies to every non-read tool):\n"
|
"\n\nWrite discipline (applies to every non-read tool):\n"
|
||||||
"- Confirm amounts, accounts and dates with the user before writing.\n"
|
"- Confirm amounts, accounts and dates with the user before writing.\n"
|
||||||
"- Writes are NOT idempotent. A retry after an ambiguous failure can "
|
"- Pass a unique client_ref on create_transaction, create_transfer and "
|
||||||
"double-book — list recent transactions and check for an identical row first.\n"
|
"create_settlement. If the call fails ambiguously, retry with the SAME "
|
||||||
|
"client_ref — the write is returned rather than repeated, so you cannot "
|
||||||
|
"double-book. Tools without a client_ref parameter are not yet protected: "
|
||||||
|
"there, list recent transactions and check for an identical row first.\n"
|
||||||
"- Accounts and categories are addressed by integer id, never by name. "
|
"- Accounts and categories are addressed by integer id, never by name. "
|
||||||
"Resolve the id with find_account / find_category first; if the lookup "
|
"Resolve the id with find_account / find_category first; if the lookup "
|
||||||
"returns nothing or more than one row, ask the user instead of guessing."
|
"returns nothing or more than one row, ask the user instead of guessing."
|
||||||
@@ -506,6 +509,7 @@ def create_server(
|
|||||||
description: str | None = None,
|
description: str | None = None,
|
||||||
fee: float = 0,
|
fee: float = 0,
|
||||||
exchange_rate_used: float | None = None,
|
exchange_rate_used: float | None = None,
|
||||||
|
client_ref: str | None = None,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Create a single expense or income transaction.
|
"""Create a single expense or income transaction.
|
||||||
|
|
||||||
@@ -525,6 +529,9 @@ def create_server(
|
|||||||
description: Optional description (max 500 chars).
|
description: Optional description (max 500 chars).
|
||||||
fee: Fee amount (included in balance calculation; default 0).
|
fee: Fee amount (included in balance calculation; default 0).
|
||||||
exchange_rate_used: Optional FX rate used for this transaction.
|
exchange_rate_used: Optional FX rate used for this transaction.
|
||||||
|
client_ref: Optional idempotency key. If a write with this key already
|
||||||
|
exists it is returned unchanged instead of booking a second time.
|
||||||
|
Set one before any write you might have to retry.
|
||||||
"""
|
"""
|
||||||
data: dict[str, Any] = {
|
data: dict[str, Any] = {
|
||||||
"date": date,
|
"date": date,
|
||||||
@@ -540,6 +547,8 @@ def create_server(
|
|||||||
data["description"] = description
|
data["description"] = description
|
||||||
if exchange_rate_used is not None:
|
if exchange_rate_used is not None:
|
||||||
data["exchange_rate_used"] = exchange_rate_used
|
data["exchange_rate_used"] = exchange_rate_used
|
||||||
|
if client_ref is not None:
|
||||||
|
data["client_ref"] = client_ref
|
||||||
return {"result": client.create_transaction(data=data)}
|
return {"result": client.create_transaction(data=data)}
|
||||||
|
|
||||||
@mcp.tool(
|
@mcp.tool(
|
||||||
@@ -608,6 +617,7 @@ def create_server(
|
|||||||
description: str = "Transfer",
|
description: str = "Transfer",
|
||||||
to_amount: float | None = None,
|
to_amount: float | None = None,
|
||||||
to_currency: str | None = None,
|
to_currency: str | None = None,
|
||||||
|
client_ref: str | None = None,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Create an atomic two-leg transfer between two accounts.
|
"""Create an atomic two-leg transfer between two accounts.
|
||||||
|
|
||||||
@@ -623,6 +633,9 @@ def create_server(
|
|||||||
description: Description for both legs (default "Transfer").
|
description: Description for both legs (default "Transfer").
|
||||||
to_amount: Amount received in destination currency (for FX transfers).
|
to_amount: Amount received in destination currency (for FX transfers).
|
||||||
to_currency: Destination currency code (for FX transfers).
|
to_currency: Destination currency code (for FX transfers).
|
||||||
|
client_ref: Optional idempotency key. If a write with this key already
|
||||||
|
exists it is returned unchanged instead of booking a second time.
|
||||||
|
Set one before any write you might have to retry.
|
||||||
"""
|
"""
|
||||||
data: dict[str, Any] = {
|
data: dict[str, Any] = {
|
||||||
"from_account_id": from_account_id,
|
"from_account_id": from_account_id,
|
||||||
@@ -637,6 +650,8 @@ def create_server(
|
|||||||
data["to_amount"] = to_amount
|
data["to_amount"] = to_amount
|
||||||
if to_currency is not None:
|
if to_currency is not None:
|
||||||
data["to_currency"] = to_currency
|
data["to_currency"] = to_currency
|
||||||
|
if client_ref is not None:
|
||||||
|
data["client_ref"] = client_ref
|
||||||
return {"result": client.create_transfer(data=data)}
|
return {"result": client.create_transfer(data=data)}
|
||||||
|
|
||||||
@mcp.tool(
|
@mcp.tool(
|
||||||
@@ -649,6 +664,7 @@ def create_server(
|
|||||||
date: str,
|
date: str,
|
||||||
direction: Literal["bank_to_splitwise", "splitwise_to_bank"],
|
direction: Literal["bank_to_splitwise", "splitwise_to_bank"],
|
||||||
currency: str = "EUR",
|
currency: str = "EUR",
|
||||||
|
client_ref: str | None = None,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Create an atomic Splitwise settlement between a bank account and a Splitwise account.
|
"""Create an atomic Splitwise settlement between a bank account and a Splitwise account.
|
||||||
|
|
||||||
@@ -660,6 +676,9 @@ def create_server(
|
|||||||
direction: "bank_to_splitwise" (you paid the Splitwise balance)
|
direction: "bank_to_splitwise" (you paid the Splitwise balance)
|
||||||
or "splitwise_to_bank" (Splitwise partner paid you).
|
or "splitwise_to_bank" (Splitwise partner paid you).
|
||||||
currency: Currency code (default EUR).
|
currency: Currency code (default EUR).
|
||||||
|
client_ref: Optional idempotency key. If a write with this key already
|
||||||
|
exists it is returned unchanged instead of booking a second time.
|
||||||
|
Set one before any write you might have to retry.
|
||||||
"""
|
"""
|
||||||
data: dict[str, Any] = {
|
data: dict[str, Any] = {
|
||||||
"bank_account_id": bank_account_id,
|
"bank_account_id": bank_account_id,
|
||||||
@@ -669,6 +688,8 @@ def create_server(
|
|||||||
"date": date,
|
"date": date,
|
||||||
"direction": direction,
|
"direction": direction,
|
||||||
}
|
}
|
||||||
|
if client_ref is not None:
|
||||||
|
data["client_ref"] = client_ref
|
||||||
return {"result": client.create_settlement(data=data)}
|
return {"result": client.create_settlement(data=data)}
|
||||||
|
|
||||||
@mcp.tool(
|
@mcp.tool(
|
||||||
|
|||||||
@@ -0,0 +1,177 @@
|
|||||||
|
"""Idempotency keys — retrying a write must not double-book.
|
||||||
|
|
||||||
|
Without these, an agent or a flaky network that retries a timed-out create
|
||||||
|
books the money twice and nothing detects it. A caller-supplied client_ref
|
||||||
|
makes the retry replay the original result instead.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from fin_cli.client import FinClientError
|
||||||
|
|
||||||
|
|
||||||
|
def _bank(client, name="Checking"):
|
||||||
|
return client.create_account({"name": name, "type": "bank", "currency": "EUR"})
|
||||||
|
|
||||||
|
|
||||||
|
def _expense(account_id, client_ref=None, amount=-12.40):
|
||||||
|
data = {
|
||||||
|
"account_id": account_id,
|
||||||
|
"date": "2026-08-22",
|
||||||
|
"amount": amount,
|
||||||
|
"currency": "EUR",
|
||||||
|
"type": "expense",
|
||||||
|
}
|
||||||
|
if client_ref is not None:
|
||||||
|
data["client_ref"] = client_ref
|
||||||
|
return data
|
||||||
|
|
||||||
|
|
||||||
|
class TestSingleCreate:
|
||||||
|
def test_repeat_with_same_key_writes_once(self, client):
|
||||||
|
acct = _bank(client)
|
||||||
|
|
||||||
|
first = client.create_transaction(_expense(acct["id"], "trip-001"))
|
||||||
|
second = client.create_transaction(_expense(acct["id"], "trip-001"))
|
||||||
|
|
||||||
|
assert first["id"] == second["id"]
|
||||||
|
assert len(client.list_transactions(account_id=acct["id"])) == 1
|
||||||
|
|
||||||
|
def test_retry_replays_the_original_row_not_the_new_payload(self, client):
|
||||||
|
"""The key identifies the operation; a differing body does not create a second row."""
|
||||||
|
acct = _bank(client)
|
||||||
|
|
||||||
|
first = client.create_transaction(_expense(acct["id"], "trip-002", amount=-12.40))
|
||||||
|
second = client.create_transaction(_expense(acct["id"], "trip-002", amount=-999.00))
|
||||||
|
|
||||||
|
assert second["id"] == first["id"]
|
||||||
|
assert second["amount"] == -12.40
|
||||||
|
assert len(client.list_transactions(account_id=acct["id"])) == 1
|
||||||
|
|
||||||
|
def test_different_keys_write_separately(self, client):
|
||||||
|
acct = _bank(client)
|
||||||
|
|
||||||
|
client.create_transaction(_expense(acct["id"], "trip-003"))
|
||||||
|
client.create_transaction(_expense(acct["id"], "trip-004"))
|
||||||
|
|
||||||
|
assert len(client.list_transactions(account_id=acct["id"])) == 2
|
||||||
|
|
||||||
|
def test_no_key_means_no_deduplication(self, client):
|
||||||
|
"""NULL client_ref must not collide with other NULLs."""
|
||||||
|
acct = _bank(client)
|
||||||
|
|
||||||
|
client.create_transaction(_expense(acct["id"]))
|
||||||
|
client.create_transaction(_expense(acct["id"]))
|
||||||
|
|
||||||
|
assert len(client.list_transactions(account_id=acct["id"])) == 2
|
||||||
|
|
||||||
|
def test_client_ref_is_returned_on_reads(self, client):
|
||||||
|
acct = _bank(client)
|
||||||
|
client.create_transaction(_expense(acct["id"], "trip-005"))
|
||||||
|
|
||||||
|
stored = client.list_transactions(account_id=acct["id"])[0]
|
||||||
|
assert stored["client_ref"] == "trip-005"
|
||||||
|
|
||||||
|
|
||||||
|
class TestMultiLegWrites:
|
||||||
|
def test_repeated_transfer_keeps_two_legs(self, client):
|
||||||
|
a = _bank(client, "A")
|
||||||
|
b = _bank(client, "B")
|
||||||
|
payload = {
|
||||||
|
"from_account_id": a["id"], "to_account_id": b["id"],
|
||||||
|
"amount": 40.0, "currency": "EUR", "date": "2026-08-22",
|
||||||
|
"client_ref": "xfer-001",
|
||||||
|
}
|
||||||
|
|
||||||
|
first = client.create_transfer(payload)
|
||||||
|
second = client.create_transfer(payload)
|
||||||
|
|
||||||
|
assert len(first) == 2 and len(second) == 2
|
||||||
|
assert [t["id"] for t in first] == [t["id"] for t in second]
|
||||||
|
assert len(client.list_transactions()) == 2, "retry created a second pair of legs"
|
||||||
|
|
||||||
|
def test_repeated_settlement_keeps_two_legs(self, client):
|
||||||
|
bank = _bank(client, "Bank")
|
||||||
|
sw = client.create_account({"name": "SW", "type": "splitwise", "currency": "EUR"})
|
||||||
|
payload = {
|
||||||
|
"bank_account_id": bank["id"], "splitwise_account_id": sw["id"],
|
||||||
|
"amount": 25.0, "currency": "EUR", "date": "2026-08-22",
|
||||||
|
"direction": "splitwise_to_bank", "client_ref": "settle-001",
|
||||||
|
}
|
||||||
|
|
||||||
|
first = client.create_settlement(payload)
|
||||||
|
second = client.create_settlement(payload)
|
||||||
|
|
||||||
|
assert [t["id"] for t in first] == [t["id"] for t in second]
|
||||||
|
assert len(client.list_transactions()) == 2
|
||||||
|
|
||||||
|
def test_repeated_batch_writes_rows_once(self, client):
|
||||||
|
acct = _bank(client)
|
||||||
|
payload = {
|
||||||
|
"account_id": acct["id"], "date": "2026-08-22", "client_ref": "batch-001",
|
||||||
|
"items": [{"amount": 12.40}, {"amount": 4.20}, {"amount": 31.00}],
|
||||||
|
}
|
||||||
|
|
||||||
|
first = client.create_transactions_batch(payload)
|
||||||
|
second = client.create_transactions_batch(payload)
|
||||||
|
|
||||||
|
assert len(first) == 3
|
||||||
|
assert [t["id"] for t in first] == [t["id"] for t in second]
|
||||||
|
assert len(client.list_transactions(account_id=acct["id"])) == 3
|
||||||
|
|
||||||
|
def test_every_row_of_a_batch_carries_the_key(self, client):
|
||||||
|
"""The whole group shares one key, which is how the retry finds all of it."""
|
||||||
|
acct = _bank(client)
|
||||||
|
client.create_transactions_batch({
|
||||||
|
"account_id": acct["id"], "date": "2026-08-22", "client_ref": "batch-002",
|
||||||
|
"items": [{"amount": 1.0}, {"amount": 2.0}],
|
||||||
|
})
|
||||||
|
|
||||||
|
stored = client.list_transactions(account_id=acct["id"])
|
||||||
|
assert all(t["client_ref"] == "batch-002" for t in stored)
|
||||||
|
|
||||||
|
|
||||||
|
class TestOrphanLegGuard:
|
||||||
|
"""A1 — the raw create endpoint must not be able to write half a pair."""
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("bad_type", ["transfer", "settlement"])
|
||||||
|
def test_two_leg_types_are_refused(self, client, bad_type):
|
||||||
|
acct = _bank(client)
|
||||||
|
|
||||||
|
with pytest.raises(FinClientError) as exc:
|
||||||
|
client.create_transaction({
|
||||||
|
"account_id": acct["id"], "date": "2026-08-22", "amount": -10.0,
|
||||||
|
"currency": "EUR", "type": bad_type,
|
||||||
|
})
|
||||||
|
|
||||||
|
assert exc.value.status_code == 422
|
||||||
|
assert client.list_transactions() == []
|
||||||
|
|
||||||
|
def test_single_leg_types_still_work(self, client):
|
||||||
|
"""Narrowing must not break the flows that legitimately post single rows."""
|
||||||
|
acct = _bank(client)
|
||||||
|
sw = client.create_account({"name": "SW", "type": "splitwise", "currency": "EUR"})
|
||||||
|
|
||||||
|
for account_id, t in [
|
||||||
|
(acct["id"], "expense"), (acct["id"], "income"),
|
||||||
|
(sw["id"], "splitwise_receive"), (sw["id"], "splitwise_pay"),
|
||||||
|
]:
|
||||||
|
created = client.create_transaction({
|
||||||
|
"account_id": account_id, "date": "2026-08-22", "amount": -5.0,
|
||||||
|
"currency": "EUR", "type": t,
|
||||||
|
})
|
||||||
|
assert created["type"] == t
|
||||||
|
|
||||||
|
def test_manual_linkage_is_refused(self, client):
|
||||||
|
"""A free-standing linked_account_id would fake half of a linked pair."""
|
||||||
|
a = _bank(client, "A")
|
||||||
|
b = _bank(client, "B")
|
||||||
|
|
||||||
|
with pytest.raises(FinClientError) as exc:
|
||||||
|
client.create_transaction({
|
||||||
|
"account_id": a["id"], "date": "2026-08-22", "amount": -10.0,
|
||||||
|
"currency": "EUR", "type": "expense", "linked_account_id": b["id"],
|
||||||
|
})
|
||||||
|
|
||||||
|
assert exc.value.status_code == 422
|
||||||
|
assert client.list_transactions() == []
|
||||||
Reference in new issue
Block a user