From 07f4134d38e53bf6c9235f3ad18dd5b33e89d9ff Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 15 Oct 2026 11:02:34 +0200 Subject: feat(piggy): piggy list/add/remove (v0.6.0) New `piggy` group: - `piggy list` lists piggy banks with saved/target amounts and linked accounts. - `piggy add|remove [--account NAME]` changes the saved amount on a linked account via PUT /piggy-banks/{id}. Firefly syncs the whole account list on that PUT, so every linked account is resent (others without current_amount, which keeps their value) or it would be unlinked. Removing below zero is rejected before any write, and the saved amount is read back so a refused add fails loudly instead of returning 200 unchanged. add/remove currently hit an upstream bug on Firefly III <= 6.7.9: PiggyBankFactory::linkToAccountIds passes the pivot current_amount (an int from the DB) to bcsub() uncast, so any PUT with "accounts" 500s (firefly-iii/firefly-iii#13063). Documented in SKILL.md/README; the live integration test skips on that error and runs once upstream is fixed. Co-Authored-By: Claude Opus 5.5 --- README.md | 5 +++ SKILL.md | 22 +++++++++++ TODO.md | 3 +- completions/firefly.bash | 5 ++- firefly_cli/__init__.py | 2 +- firefly_cli/cli.py | 1 + firefly_cli/commands/__init__.py | 2 +- firefly_cli/commands/piggy.py | 81 +++++++++++++++++++++++++++++++++++++++ firefly_cli/resolver.py | 8 ++++ pyproject.toml | 2 +- tests/integration/test_live.py | 43 +++++++++++++++++++++ tests/unit/test_commands_piggy.py | 80 ++++++++++++++++++++++++++++++++++++++ tests/unit/test_resolver.py | 11 ++++++ 13 files changed, 260 insertions(+), 5 deletions(-) create mode 100644 firefly_cli/commands/piggy.py create mode 100644 tests/unit/test_commands_piggy.py diff --git a/README.md b/README.md index c6cdb80..ee575b0 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,9 @@ firefly budget disable mark a budget inactive firefly budget delete --yes firefly budget limit-list firefly budget limit-set --amount N [--start D] [--end D] +firefly piggy list +firefly piggy add [--account A] +firefly piggy remove [--account A] [--currency CODE] firefly category list @@ -102,6 +105,8 @@ The command set grows over time; see `CLAUDE.md` for how to add one. a period; `budget limit-set` sets a spending limit over a date range. `budget update` renames a budget or edits its auto-budget fields. `budget delete` and `tx delete` require `--yes`. +- Piggy banks: `piggy add/remove` change the saved amount on a linked account. + Blocked on Firefly III <= 6.7.9 by upstream bug firefly-iii/firefly-iii#13063. ## For agents (skill) diff --git a/SKILL.md b/SKILL.md index 87dcc94..7611e7d 100644 --- a/SKILL.md +++ b/SKILL.md @@ -83,6 +83,9 @@ firefly budget delete --yes # --yes required, no prompt firefly budget limit-list firefly budget limit-set --amount N [--start YYYY-MM-DD] [--end YYYY-MM-DD] [--currency CODE] # default period: current month +firefly piggy list +firefly piggy add [--account NAME] # --account only if linked to several +firefly piggy remove [--account NAME] ``` Global: `--human` (tables, do not use when parsing), `--url`/`--token` @@ -166,6 +169,25 @@ firefly budget disable Groceries # stop budgeting it (tog fields you pass (rename via `--name`, or edit the auto-budget fields). Note the leaf names are hyphenated (`limit-list`, `limit-set`), not `limit list`. +**Piggy banks.** A piggy bank earmarks money inside one or more linked asset +accounts; moving money in or out does NOT create a transaction or change any +account balance. `piggy list` shows each one with `current_amount`, +`target_amount` and its linked `accounts`. `piggy add/remove ` +changes the saved amount on the linked account (`--account NAME` when the +piggy bank has several). Removing more than is saved is a hard error before +any write; an add Firefly refuses (not enough unallocated money, target +exceeded) is a hard error too, never a silent no-op. +```bash +firefly piggy list +firefly piggy add "BBVA risparmi" 7.09 +firefly piggy remove "BBVA risparmi" 2 --account BBVA +``` +**Known upstream bug:** on Firefly III up to at least 6.7.9, `piggy add` and +`piggy remove` fail with `API error 500: ... bcsub(): Argument #2 ($num2) must +be of type string, int given` (firefly-iii/firefly-iii#13063). Nothing is +written. Until it is fixed, tell the user to move the amount in the Firefly +web UI; do not call the API directly. + **Check a balance:** ```bash firefly account balance test01 # -> {"id","name","current_balance"} diff --git a/TODO.md b/TODO.md index 3208c57..8c9a7de 100644 --- a/TODO.md +++ b/TODO.md @@ -8,7 +8,8 @@ bash completion). - [x] `budget` — list/create/delete/enable/disable, limit-list/limit-set, and `tx add --budget`. Shipped v0.4.0 (review follow-ups v0.4.1). - [ ] `bill` — list and inspect bills. -- [ ] `piggy` — piggy banks. +- [x] `piggy` — list/add/remove. Shipped v0.6.0. add/remove blocked upstream + by firefly-iii/firefly-iii#13063 (500 on PUT with accounts, <= 6.7.9). - [ ] `rule` — rules and rule groups. - [ ] `recurring` — recurring transactions. - [ ] `attachment` — list/download transaction attachments. diff --git a/completions/firefly.bash b/completions/firefly.bash index 34171f9..baca1db 100644 --- a/completions/firefly.bash +++ b/completions/firefly.bash @@ -16,7 +16,7 @@ _firefly() { } local global_opts="--human --url --token -h --help" - local groups="auth account category tag tx budget" + local groups="auth account category tag tx budget piggy" # Find the group and leaf among the words (skip the program name at 0). local group="" leaf="" i @@ -49,6 +49,8 @@ _firefly() { "budget limit-set") leaf_opts="--amount --currency --end --start";; "budget list") leaf_opts="--end --start";; "budget update") leaf_opts="--auto-budget-amount --auto-budget-period --auto-budget-type --currency --name";; + "piggy add") leaf_opts="--account";; + "piggy remove") leaf_opts="--account";; esac # Leaves per group. @@ -60,6 +62,7 @@ _firefly() { tag) leaves="list";; tx) leaves="add delete edit get list search";; budget) leaves="create delete disable enable limit-list limit-set list update";; + piggy) leaves="add list remove";; esac # Flag values: when the previous word is a flag with a fixed value set for diff --git a/firefly_cli/__init__.py b/firefly_cli/__init__.py index d4060af..1a475b7 100644 --- a/firefly_cli/__init__.py +++ b/firefly_cli/__init__.py @@ -2,4 +2,4 @@ # Copyright (C) 2026 Danilo M. # Licensed under the GNU General Public License v2.0 only. -__version__ = "0.5.0" +__version__ = "0.6.0" diff --git a/firefly_cli/cli.py b/firefly_cli/cli.py index 712d52d..7b624f2 100644 --- a/firefly_cli/cli.py +++ b/firefly_cli/cli.py @@ -20,6 +20,7 @@ _GROUP_HELP = { "tag": "list tags (tags auto-create when attached to a tx)", "tx": "record, list, and search transactions", "budget": "manage budgets and per-period spending limits", + "piggy": "list piggy banks and move money in or out of them", } def _build_parser(): diff --git a/firefly_cli/commands/__init__.py b/firefly_cli/commands/__init__.py index e87c623..99762a6 100644 --- a/firefly_cli/commands/__init__.py +++ b/firefly_cli/commands/__init__.py @@ -1,3 +1,3 @@ # Copyright (C) 2026 Danilo M. GPL-2.0-only # Importing each module runs its @registry.command decorators. -from firefly_cli.commands import auth, account, category, tag, transaction, budget # noqa: F401 +from firefly_cli.commands import auth, account, category, tag, transaction, budget, piggy # noqa: F401 diff --git a/firefly_cli/commands/piggy.py b/firefly_cli/commands/piggy.py new file mode 100644 index 0000000..9ade3c8 --- /dev/null +++ b/firefly_cli/commands/piggy.py @@ -0,0 +1,81 @@ +# Copyright (C) 2026 Danilo M. GPL-2.0-only +from decimal import Decimal, InvalidOperation +from firefly_cli import registry, output +from firefly_cli.errors import FireflyError + + +@registry.command("piggy list", help="list piggy banks with saved/target amounts and linked accounts") +def cmd_list(args, ctx): + resp = ctx.client.request("GET", "/api/v1/piggy-banks", params={"limit": 1000}) + output.emit(output.unwrap(resp), human=ctx.human) + return 0 + + +def _move_args(p): + p.add_argument("ref", help="piggy bank name or id") + p.add_argument("amount", help="amount to move (positive)") + p.add_argument("--account", default=None, + help="linked asset account name (required if the piggy bank has several)") + + +def _amount(raw): + try: + amt = Decimal(str(raw)) + except InvalidOperation: + amt = None + if amt is None or not amt.is_finite() or amt <= 0: + raise FireflyError(f'amount must be a positive number, got "{raw}"') + return amt + + +def _move(args, ctx, sign): + amt = _amount(args.amount) + pb = ctx.resolver.piggy(args.ref) + linked = pb.get("accounts") or [] + if args.account is None: + if len(linked) != 1: + raise FireflyError( + f'piggy bank "{pb.get("name")}" is linked to {len(linked)} accounts; ' + "pass --account: " + ", ".join(a.get("name", "") for a in linked)) + target = linked[0] + else: + hits = [a for a in linked if str(a.get("name", "")).lower() == args.account.lower()] + if len(hits) != 1: + raise FireflyError( + f'account "{args.account}" is not linked to piggy bank "{pb.get("name")}". ' + "Linked: " + (", ".join(a.get("name", "") for a in linked) or "(none)")) + target = hits[0] + + new = Decimal(str(target.get("current_amount") or "0")) + sign * amt + if new < 0: + raise FireflyError( + f'cannot remove {amt}: only {target.get("current_amount")} saved from {target.get("name")}') + new_s = f"{new:.2f}" + # Firefly syncs the whole account list on PUT, so every linked account must + # be resent or it gets unlinked. Omitting current_amount keeps its saved value. + body = {"accounts": [ + {"account_id": a["account_id"], "current_amount": new_s} if a is target + else {"account_id": a["account_id"]} + for a in linked]} + resp = ctx.client.request("PUT", f"/api/v1/piggy-banks/{pb['id']}", body=body) + saved = output.unwrap(resp) + # Validation normally 422s an unaffordable add, but PiggyBankFactory can + # still skip it and answer 200. Read back and fail loudly instead. + after = next((a for a in saved.get("accounts") or [] + if str(a.get("account_id")) == str(target["account_id"])), {}) + if Decimal(str(after.get("current_amount") or "0")) != new: + raise FireflyError( + f'Firefly did not apply the change (saved {after.get("current_amount")}, wanted {new_s}): ' + "not enough unallocated money on the account, or the target would be exceeded") + output.emit(saved, human=ctx.human) + return 0 + + +@registry.command("piggy add", help="put money into a piggy bank", args=_move_args) +def cmd_add(args, ctx): + return _move(args, ctx, 1) + + +@registry.command("piggy remove", help="take money out of a piggy bank", args=_move_args) +def cmd_remove(args, ctx): + return _move(args, ctx, -1) diff --git a/firefly_cli/resolver.py b/firefly_cli/resolver.py index cb7e9a8..1a5de11 100644 --- a/firefly_cli/resolver.py +++ b/firefly_cli/resolver.py @@ -59,3 +59,11 @@ class Resolver: def budget_by_id(self, budget_id): item = self.client.request("GET", f"/api/v1/budgets/{budget_id}")["data"] return {"id": item["id"], **item.get("attributes", {})} + + def piggy(self, name_or_id): + # Same name-or-id sniff as budget(), same ponytail caveat (a piggy bank + # literally named "7" is unreachable by name). + if str(name_or_id).isdigit(): + item = self.client.request("GET", f"/api/v1/piggy-banks/{name_or_id}")["data"] + return {"id": item["id"], **item.get("attributes", {})} + return self._match("piggy bank", self._list("/api/v1/piggy-banks"), name_or_id) diff --git a/pyproject.toml b/pyproject.toml index 409c096..24201ad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "firefly-iii-agent" -version = "0.5.0" +version = "0.6.0" description = "CLI tool for agent interaction with Firefly III" readme = "README.md" requires-python = ">=3.11" diff --git a/tests/integration/test_live.py b/tests/integration/test_live.py index a815b88..ec900d7 100644 --- a/tests/integration/test_live.py +++ b/tests/integration/test_live.py @@ -54,3 +54,46 @@ class TestLive(unittest.TestCase): self.assertEqual(got["data"]["id"], tx_id) finally: self.client.request("DELETE", f"/api/v1/transactions/{tx_id}") + + def test_piggy_add_remove_roundtrip(self): + # Create a piggy bank on the TEST account, drive it through the CLI + # handlers, then delete it. + import io, contextlib, json + from types import SimpleNamespace + from firefly_cli.commands import piggy + from firefly_cli.errors import FireflyError + accs = unwrap(self.client.request("GET", "/api/v1/accounts", + params={"type": "asset", "limit": 50})) + acc = next((a for a in accs if float(a.get("current_balance") or 0) >= 10), None) + if acc is None: + self.skipTest("test account has no asset account with >= 10 balance") + created = self.client.request("POST", "/api/v1/piggy-banks", body={ + "name": "firefly-cli integration piggy", "target_amount": "100", + "transaction_currency_code": acc.get("currency_code") or "EUR", + "start_date": "2026-01-01", + "accounts": [{"account_id": acc["id"], "current_amount": "0"}]}) + pb_id = created["data"]["id"] + ctx = SimpleNamespace(client=self.client, resolver=self.resolver, human=False) + + def run(fn, amount): + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + fn(SimpleNamespace(ref=pb_id, amount=amount, account=None), ctx) + return json.loads(buf.getvalue())["accounts"][0]["current_amount"] + try: + from firefly_cli.errors import ApiError + try: + first = run(piggy.cmd_add, "2.09") + except ApiError as e: + if "bcsub()" in str(e): + self.skipTest("firefly-iii/firefly-iii#13063: piggy PUT with accounts 500s") + raise + self.assertEqual(float(first), 2.09) + self.assertEqual(float(run(piggy.cmd_add, "0.91")), 3.00) + self.assertEqual(float(run(piggy.cmd_remove, "1")), 2.00) + with self.assertRaises(FireflyError): # more than the account holds + run(piggy.cmd_add, "99999999") + linked = unwrap(self.client.request("GET", f"/api/v1/piggy-banks/{pb_id}"))["accounts"] + self.assertEqual(len(linked), 1) # PUT did not unlink the account + finally: + self.client.request("DELETE", f"/api/v1/piggy-banks/{pb_id}") diff --git a/tests/unit/test_commands_piggy.py b/tests/unit/test_commands_piggy.py new file mode 100644 index 0000000..24565ac --- /dev/null +++ b/tests/unit/test_commands_piggy.py @@ -0,0 +1,80 @@ +# Copyright (C) 2026 Danilo M. GPL-2.0-only +import unittest +from unittest.mock import MagicMock +from types import SimpleNamespace +import firefly_cli.commands.piggy as piggy +from firefly_cli.errors import FireflyError + + +def _pb(accounts): + return {"id": "3", "name": "Round-ups", "accounts": accounts} + + +def _ctx(pb, put_accounts=None): + """Resolver returns `pb`; the PUT echoes `put_accounts` (what Firefly saved).""" + resolver = MagicMock() + resolver.piggy.return_value = pb + client = MagicMock() + client.request.return_value = {"data": {"id": "3", "attributes": { + "name": "Round-ups", "accounts": put_accounts or []}}} + return SimpleNamespace(client=client, resolver=resolver, human=False) + + +ONE = [{"account_id": "6", "name": "BBVA", "current_amount": "1.50"}] +TWO = ONE + [{"account_id": "8", "name": "Fineco", "current_amount": "0.00"}] + + +class TestPiggyAddRemove(unittest.TestCase): + def test_add_single_account_puts_new_total(self): + ctx = _ctx(_pb(ONE), [{"account_id": "6", "current_amount": "3.59"}]) + rc = piggy.cmd_add(SimpleNamespace(ref="Round-ups", amount="2.09", account=None), ctx) + self.assertEqual(rc, 0) + method, path = ctx.client.request.call_args[0][:2] + self.assertEqual((method, path), ("PUT", "/api/v1/piggy-banks/3")) + body = ctx.client.request.call_args[1]["body"] + self.assertEqual(body, {"accounts": [{"account_id": "6", "current_amount": "3.59"}]}) + + def test_remove_subtracts(self): + ctx = _ctx(_pb(ONE), [{"account_id": "6", "current_amount": "1.00"}]) + piggy.cmd_remove(SimpleNamespace(ref="Round-ups", amount="0.50", account=None), ctx) + body = ctx.client.request.call_args[1]["body"] + self.assertEqual(body["accounts"][0]["current_amount"], "1.00") + + def test_remove_below_zero_is_error_without_write(self): + ctx = _ctx(_pb(ONE)) + with self.assertRaises(FireflyError): + piggy.cmd_remove(SimpleNamespace(ref="Round-ups", amount="5", account=None), ctx) + ctx.client.request.assert_not_called() + + def test_multi_account_needs_account_flag(self): + ctx = _ctx(_pb(TWO)) + with self.assertRaises(FireflyError): + piggy.cmd_add(SimpleNamespace(ref="Round-ups", amount="1", account=None), ctx) + + def test_multi_account_resends_others_without_amount(self): + # Firefly syncs the account list: omitting an account would unlink it. + ctx = _ctx(_pb(TWO), [{"account_id": "8", "current_amount": "1.00"}]) + piggy.cmd_add(SimpleNamespace(ref="Round-ups", amount="1", account="fineco"), ctx) + body = ctx.client.request.call_args[1]["body"] + self.assertEqual(body["accounts"], [{"account_id": "6"}, + {"account_id": "8", "current_amount": "1.00"}]) + + def test_unknown_account_is_error(self): + ctx = _ctx(_pb(ONE)) + with self.assertRaises(FireflyError): + piggy.cmd_add(SimpleNamespace(ref="Round-ups", amount="1", account="Nope"), ctx) + + def test_silent_refusal_is_detected(self): + # Firefly skips an add it can't afford but still returns 200. + ctx = _ctx(_pb(ONE), [{"account_id": "6", "current_amount": "1.50"}]) + with self.assertRaises(FireflyError): + piggy.cmd_add(SimpleNamespace(ref="Round-ups", amount="2", account=None), ctx) + + def test_bad_amount(self): + for bad in ("0", "-1", "abc"): + with self.assertRaises(FireflyError): + piggy.cmd_add(SimpleNamespace(ref="x", amount=bad, account=None), _ctx(_pb(ONE))) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_resolver.py b/tests/unit/test_resolver.py index 8c93a84..d9a1dcd 100644 --- a/tests/unit/test_resolver.py +++ b/tests/unit/test_resolver.py @@ -84,3 +84,14 @@ class TestResolverBudget(unittest.TestCase): self.assertEqual(got["name"], "Fun") # Must hit the show endpoint, not list. client.request.assert_called_with("GET", "/api/v1/budgets/7") + +class TestPiggyResolver(unittest.TestCase): + def test_piggy_by_name(self): + c = client_returning([{"id": "1", "attrs": {"name": "BBVA risparmi"}}]) + self.assertEqual(Resolver(c).piggy("bbva risparmi")["id"], "1") + + def test_piggy_by_id_hits_show_endpoint(self): + c = MagicMock() + c.request.return_value = {"data": {"id": "1", "attributes": {"name": "X"}}} + Resolver(c).piggy("1") + self.assertEqual(c.request.call_args[0][1], "/api/v1/piggy-banks/1") -- cgit v1.2.3