fix(accounts): recover ledger-only account instead of blanket 409 (libra-#50)

When add_account reported the Open already existed, the endpoint raised
409 before the DB-mirror step — so an account present in the ledger but
missing from libra's DB (a prior sync failure with no cross-DB atomicity,
or an out-of-band open) was stranded: invisible to permissions with no
recovery path. Now 409 only when the account is already in the DB too;
otherwise sync it and return success. Adds a recovery test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

crud.py

@@ -250,9 +250,13 @@ async def get_or_create_user_account(
                 if not fava_account_exists:
                     # Create account in Fava/Beancount via Open directive
                     logger.info(f"[FAVA CREATE] Creating account in Fava: {account_name}")
+                    # Unconstrained Open: a per-user receivable/payable legitimately
+                    # holds arbitrary fiat (CAD/GBP/JPY/…). Constraining it to
+                    # EUR/SATS/USD made any posting in another currency fail
+                    # bean-check (the errors this account path originally exhibited).
                     await fava.add_account(
                         account_name=account_name,
-                        currencies=["EUR", "SATS", "USD"],  # Support common currencies
+                        currencies=None,
                         metadata={
                             "user_id": user_id,
                             "description": f"User-specific {account_type.value} account"

fava_client.py

@@ -60,6 +60,30 @@ def _escape_beancount_string(value: str) -> str:
     )
 
 
+def _open_directive_exists(source: str, account_name: str) -> bool:
+    """Return True if `source` already contains an Open directive for exactly
+    `account_name`.
+
+    Anchored to a real `YYYY-MM-DD open <account>` directive line (re.MULTILINE)
+    so the account name can't match text inside another account's description
+    metadata or a comment (false positive → spurious 409). The trailing
+    negative-lookahead `(?![\\w:-])` requires the next char not to be an
+    account-continuation char, so:
+      - a prefix (Expenses:Gas) does not match a longer sibling
+        (Expenses:GasStation / Expenses:Gas:Vehicle), and
+      - a real directive with an inline comment and no space
+        (`open Expenses:Gas;legacy`) is still detected (`;` ends the name),
+        which the previous `(?:\\s|$)` boundary missed → duplicate write.
+    """
+    return bool(
+        re.search(
+            rf"^\d{{4}}-\d{{2}}-\d{{2}} open {re.escape(account_name)}(?![\w:-])",
+            source,
+            re.MULTILINE,
+        )
+    )
+
+
 class FavaClient:
     """
     Async client for Fava REST API.
@@ -1644,15 +1668,9 @@ class FavaClient:
                         source = source_data["source"]
 
                         # Step 2: Check if account already exists (may have been
-                        # created by a concurrent request). Anchor on the Open
-                        # directive and require the account to be followed by
-                        # whitespace/end-of-line so a prefix (Expenses:Gas) does
-                        # not match a longer sibling (Expenses:GasStation).
-                        if re.search(
-                            rf"open {re.escape(account_name)}(?:\s|$)",
-                            source,
-                            re.MULTILINE,
-                        ):
+                        # created by a concurrent request). See
+                        # _open_directive_exists for the anchoring rationale.
+                        if _open_directive_exists(source, account_name):
                             logger.info(f"Account {account_name} already exists in {target_file}")
                             return {
                                 "data": sha256sum,

tests/test_admin_chart_accounts_api.py

@@ -93,6 +93,32 @@ async def test_add_chart_account_duplicate_returns_409(
     assert "already exists" in second.json().get("detail", "").lower()
 
 
+@pytest.mark.anyio
+async def test_add_chart_account_recovers_ledger_only_account(
+    client, super_user_headers,
+):
+    """An account present in the ledger but absent from libra's DB (prior sync
+    failure / out-of-band edit) is recovered (synced), not 409'd — otherwise it
+    would be permanently un-grantable with no path back.
+
+    Reproduce the ledger-only state by creating normally (so Fava parses the
+    Open) then deleting only the libra-DB row — appending to the ledger file
+    directly would race Fava's parse cache."""
+    from ..crud import db  # the same singleton the app uses
+
+    name = _unique("Expenses:Recover")
+    first = await add_chart_account(client, super_user_headers=super_user_headers, name=name)
+    assert first.status_code == 201, f"setup create failed: {first.status_code} {first.text}"
+
+    await db.execute("DELETE FROM accounts WHERE name = :name", {"name": name})
+
+    r = await add_chart_account(client, super_user_headers=super_user_headers, name=name)
+    assert r.status_code == 201, f"expected 201 recovery, got {r.status_code}: {r.text}"
+    body = r.json()
+    assert body.get("already_existed") is True, body
+    assert body["synced_to_libra_db"] is True, body
+
+
 @pytest.mark.anyio
 async def test_add_chart_account_invalid_prefix_returns_400(
     client, super_user_headers, fava_ledger_path,

tests/test_unit.py

@@ -31,9 +31,59 @@ bf = _module("beancount_format")
 au = _module("account_utils")
 val = _module("core.validation")
 mdl = _module("models")
+fc = _module("fava_client")
 AccountType = mdl.AccountType
 
 
+# ---------------------------------------------------------------------------
+# fava_client._open_directive_exists — duplicate-account detection
+# ---------------------------------------------------------------------------
+
+
+def test_open_directive_exists_matches_real_directive():
+    src = "2020-01-01 open Expenses:Vehicle:Gas\n"
+    assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True
+
+
+def test_open_directive_exists_matches_currency_constrained_and_metadata():
+    src = (
+        "2020-01-01 open Expenses:Vehicle:Gas EUR, SATS\n"
+        '  added_by: "abc"\n'
+    )
+    assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True
+
+
+def test_open_directive_exists_matches_inline_comment_without_space():
+    # Valid Beancount: the account token ends at ';'. The old (?:\\s|$) boundary
+    # missed this → duplicate Open written → bean-check breaks.
+    src = "2020-01-01 open Expenses:Vehicle:Gas;legacy-import\n"
+    assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True
+
+
+def test_open_directive_exists_ignores_name_inside_description():
+    # The name appears only inside another account's description metadata.
+    src = (
+        "2020-01-01 open Expenses:Notes\n"
+        '  description: "remember to open Expenses:Vehicle:Gas next month"\n'
+    )
+    assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False
+
+
+def test_open_directive_exists_ignores_comment_line():
+    src = "; TODO: open Expenses:Vehicle:Gas eventually\n"
+    assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False
+
+
+def test_open_directive_exists_does_not_match_longer_sibling():
+    src = "2020-01-01 open Expenses:Vehicle:GasStation\n"
+    assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False
+
+
+def test_open_directive_exists_does_not_match_deeper_child():
+    src = "2020-01-01 open Expenses:Vehicle:Gas:Premium\n"
+    assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False
+
+
 # ---------------------------------------------------------------------------
 # beancount_format.sanitize_link
 # ---------------------------------------------------------------------------

views_api.py

@@ -3750,14 +3750,31 @@ async def api_admin_add_chart_account(
         metadata=metadata,
     )
 
+    from .account_sync import sync_single_account_from_beancount
+
     if result.get("already_existed"):
+        # The Open directive is already in the ledger. If it's also already
+        # mirrored into libra's DB, it's a true duplicate → 409. If not (a prior
+        # sync failed — there's no cross-DB atomicity — or it was opened out of
+        # band), mirror it now so it becomes grantable instead of being stranded
+        # with no recovery path.
+        from .crud import get_account_by_name
+
+        if await get_account_by_name(payload.name) is not None:
             raise HTTPException(
                 status_code=HTTPStatus.CONFLICT,
                 detail=f"Account {payload.name} already exists",
             )
 
+        synced = await sync_single_account_from_beancount(payload.name)
+        return {
+            "success": True,
+            "account_name": payload.name,
+            "synced_to_libra_db": synced,
+            "already_existed": True,
+        }
+
     # Mirror into libra DB so permissions / metadata layer sees it.
-    from .account_sync import sync_single_account_from_beancount
     synced = await sync_single_account_from_beancount(payload.name)
 
     return {