[BTAPI-75] Swap the Wealthsimple activity fetch for wealthsimple-api calls, behind the cutover flag #92

Merged

chris merged 16 commits from feature/BTAPI-75 into main

2026-09-14 10:31:24 -06:00

chris commented

2026-09-14 09:15:01 -06:00

Owner

Ticket

BTAPI-75 Swap in-repo Wealthsimple client for wealthsimple-api service calls

Part of the "Wealthsimple API — shared service for Budget Tracker and Home Assistant" epic. This cuts budget-tracker-api's transaction-sync data-fetching over to the standalone wealthsimple-api service. The upsert/dedupe engine in server/sync/ is untouched.

Summary

Flag-gated, so the default is byte-identical to today. Config.wealthsimpleApiEnabled (WS_API_ENABLED, default false, added by BTAPI-81) selects the implementation. fetchCreditCardActivities(since) is now a two-line dispatcher: false → the pre-existing GraphQL path, renamed fetchCreditCardActivitiesDirect and otherwise unchanged (the rename was verified textually identical); true → fetchCreditCardActivitiesViaWealthsimpleApi, which reads the service's GET /activity-feed through a new thin client (lib/wealthsimple/activity-feed-client) modelled on the existing trigger-client.
Nothing is deleted. client/, queries.ts, account discovery and fetchCreditCardBalance all stay — they are the false branch and the ws:activities CLI's balance read. Deleting them is BTAPI-78's job once the flag retires. Stating it explicitly so they aren't read as dead code.
New config / compose: WS_API_URL / WS_API_KEY in the wealthsimple-api client block of .env.example; Config.assertWealthsimpleApiClientConfig(), which fails loud at boot only when the flag is on; parseServiceUrl normalizing the URL; and sync joining the external api-shared network.
One implementation detail the plan asked to surface: the shared classification helpers (isPurchase, toActivity, logSkipped, toSpendAmount, purchaseNodeSchema) are typed to ActivityNode — the loose, all-optional type activityNodeSchema infers — which is also, structurally, everything a RawActivityItem from the client promises. So both branches share one set of rules about what counts as spend, and no parallel helper set was added; nothing needed widening.
Docs: server/lib/wealthsimple/CLAUDE.md documents the branch point (and the interface it shares with BTAPI-76/77); root CLAUDE.md, docs/DEV-ENVIRONMENT.md and .env.example record the networking and the listener acceptance.

Two decisions worth a reviewer's attention

Joining api-shared widens the sync trigger listener's reach — a conscious acceptance, and not free. The listener has no auth by design: its access control is the set of networks sync is attached to, and it binds 0.0.0.0 within them. Adding api-shared therefore lets every first-party container on that network ring the doorbell unauthenticated — spending WS_MAX_SYNCS_PER_HOUR headroom and arming the retry ladder. Accepted on the basis that all api-shared members are first-party, and documented in trigger-listener/index.ts (the full reasoning, plus what would invalidate it), root CLAUDE.md, docs/DEV-ENVIRONMENT.md and .env.example. No auth was added. The original plan assumed this was free; that was wrong.
WS_CREDIT_CARD_ACCOUNT_ID stays an optional post-fetch filter on the new branch, unlike the hard stop it is on the direct branch (which still throws rather than guessing between two open cards). Because items the feed leaves unattributed are kept — an absent accountId means "the feed did not say", not "a different account" — a feed spanning more than one account is aggregated into one budget as one owner. So a warning is logged whenever the feed names more than one account, with or without a pin, naming the count and the distinct ids; with a pin set it also states that the other accounts' items were filtered out. The count is always taken from the whole feed, since one taken after the pin filter could never reach two.

Notable fixes from review

The since window is applied to the raw timestamp before mapping. Applying it after toActivity — which throws on a purchase missing a field the schema requires — let one malformed legacy purchase kill every run from then on, since this branch fetches the whole cached history unfiltered. An item whose timestamp is absent or unreadable is now dropped and named in a warning; an in-window item with a bad field still throws, exactly as on the direct branch.
The internal hop has its own error type. WealthsimpleApiClientError (mirroring SyncTriggerUnavailableError) is kept out of the engine's 401/429 refusal cooldown: a 401 from wealthsimple-api means WS_API_KEY ≠ that stack's INTERNAL_API_KEY, not a refused credential — there is no session to refresh, and a cooldown would only delay the fix its message names. It still counts toward the consecutive-failure streak and the ordinary outage alert.
The boot assert runs at module scope in sync/app.ts, outside the boot IIFE's try, so a missing key exits non-zero rather than being caught and restart-looping under restart: unless-stopped.
lib/logger's pretty-print selection now excludes the test runner regardless of NODE_ENV or TTY. Bun's runner only sets NODE_ENV=test when the environment doesn't already set it, and the dev container exports NODE_ENV=development — so under a TTY the documented local test command had 5 pre-existing failures across 4 files, not just this ticket's own tests. Production output and normal dev-container use are unchanged.

Tests

Whole suite green: 831 pass, 0 fail (832 tests, 58 files). The 1 skip is the pre-existing curl-impersonate integration test, which self-skips when the binary isn't present on a plain checkout. bun --cwd=./server run verify passes all five checks (type-check, lint, format, format:check, test), and the suite is green both in CI's non-TTY environment and under a pty with NODE_ENV=development.

Coverage added: the new client (URL, bearer header, 200 parse, non-2xx with status, unreachable and real-timeout without status, malformed body), the new branch on the activities module (mapping, non-purchase exclusion, wsId derivation, sign/rounding, the client-side window, the optional account pin including unattributed items, single non-paginated request, malformed-purchase throw, 503 with no fallback), both dispatcher directions, the multi-account warning decision and its call site (asserted on the pino records the module actually emits), the engine-level internal-hop 401, and the logger's selection logic.

Operator steps this branch deliberately does NOT perform

The sync container is live production, so these were left to a human. Both are required before the flag is flipped:

Set WS_API_KEY to the same value as wealthsimple-api's INTERNAL_API_KEY. They live in two separate .env files and neither codebase can verify the match — assertWealthsimpleApiClientConfig catches only an empty key. A mismatch surfaces as a 401 on the first flagged sync.
Run the flag-on comparison — ws:activities against a real wealthsimple-api with the flag on, confirming the returned activities match its GET /activity-feed cache, filtered and mapped identically to the direct path.

Flipping the flag also needs the container recreated (docker compose up -d --force-recreate <service>), not merely restarted: compose resolves env_file: .env at creation, so a plain docker restart silently keeps the old value.

Follow-up worth filing (WSAPI, not this ticket)

Reviewers noticed in wealthsimple-api's own source that isCardPurchase has no dead-status gate while isMoreAuthoritative prefers non-pending — so a dead node sharing a derived id with its live authorized twin could win the cache. Traced through from this side the consequence is bounded and low-severity: the purchase then simply isn't in the feed, and the only local effect is that the vanished-pending check may raise its (accurate) alert for that purchase's previously-synced pending row, deleting nothing. wealthsimple-api was not touched by this branch; a WSAPI ticket could tighten it if you want it closed.

🤖 Generated with Claude Code

## Ticket [BTAPI-75](http://192.168.2.100:7123/home/browse/BTAPI-75/) Swap in-repo Wealthsimple client for wealthsimple-api service calls Part of the "Wealthsimple API — shared service for Budget Tracker and Home Assistant" epic. This cuts budget-tracker-api's transaction-sync **data-fetching** over to the standalone `wealthsimple-api` service. The upsert/dedupe engine in `server/sync/` is untouched. ## Summary - **Flag-gated, so the default is byte-identical to today.** `Config.wealthsimpleApiEnabled` (`WS_API_ENABLED`, default **false**, added by BTAPI-81) selects the implementation. `fetchCreditCardActivities(since)` is now a two-line dispatcher: `false` → the pre-existing GraphQL path, renamed `fetchCreditCardActivitiesDirect` and otherwise unchanged (the rename was verified textually identical); `true` → `fetchCreditCardActivitiesViaWealthsimpleApi`, which reads the service's `GET /activity-feed` through a new thin client (`lib/wealthsimple/activity-feed-client`) modelled on the existing `trigger-client`. - **Nothing is deleted.** `client/`, `queries.ts`, account discovery and `fetchCreditCardBalance` all stay — they are the `false` branch and the `ws:activities` CLI's balance read. Deleting them is BTAPI-78's job once the flag retires. Stating it explicitly so they aren't read as dead code. - **New config / compose:** `WS_API_URL` / `WS_API_KEY` in the `wealthsimple-api client` block of `.env.example`; `Config.assertWealthsimpleApiClientConfig()`, which fails loud at boot **only when the flag is on**; `parseServiceUrl` normalizing the URL; and `sync` joining the external `api-shared` network. - **One implementation detail the plan asked to surface:** the shared classification helpers (`isPurchase`, `toActivity`, `logSkipped`, `toSpendAmount`, `purchaseNodeSchema`) are typed to `ActivityNode` — the loose, all-optional type `activityNodeSchema` infers — which is also, structurally, everything a `RawActivityItem` from the client promises. So both branches share one set of rules about what counts as spend, and **no parallel helper set was added**; nothing needed widening. - **Docs:** `server/lib/wealthsimple/CLAUDE.md` documents the branch point (and the interface it shares with BTAPI-76/77); root `CLAUDE.md`, `docs/DEV-ENVIRONMENT.md` and `.env.example` record the networking and the listener acceptance. ## Two decisions worth a reviewer's attention 1. **Joining `api-shared` widens the sync trigger listener's reach — a conscious acceptance, and not free.** The listener has no auth by design: its access control *is* the set of networks `sync` is attached to, and it binds `0.0.0.0` within them. Adding `api-shared` therefore lets every first-party container on that network ring the doorbell unauthenticated — spending `WS_MAX_SYNCS_PER_HOUR` headroom and arming the retry ladder. Accepted on the basis that all `api-shared` members are first-party, and documented in `trigger-listener/index.ts` (the full reasoning, plus what would invalidate it), root `CLAUDE.md`, `docs/DEV-ENVIRONMENT.md` and `.env.example`. **No auth was added.** The original plan assumed this was free; that was wrong. 2. **`WS_CREDIT_CARD_ACCOUNT_ID` stays an optional post-fetch filter** on the new branch, unlike the hard stop it is on the direct branch (which still throws rather than guessing between two open cards). Because items the feed leaves *unattributed* are kept — an absent `accountId` means "the feed did not say", not "a different account" — a feed spanning more than one account is aggregated into one budget as one owner. So a **warning is logged whenever the feed names more than one account, with or without a pin**, naming the count and the distinct ids; with a pin set it also states that the other accounts' items were filtered out. The count is always taken from the whole feed, since one taken after the pin filter could never reach two. ## Notable fixes from review - **The `since` window is applied to the raw timestamp *before* mapping.** Applying it after `toActivity` — which throws on a purchase missing a field the schema requires — let one malformed legacy purchase kill every run from then on, since this branch fetches the whole cached history unfiltered. An item whose timestamp is absent or unreadable is now dropped and named in a warning; an in-window item with a bad field still throws, exactly as on the direct branch. - **The internal hop has its own error type.** `WealthsimpleApiClientError` (mirroring `SyncTriggerUnavailableError`) is kept out of the engine's 401/429 refusal cooldown: a 401 from `wealthsimple-api` means `WS_API_KEY` ≠ that stack's `INTERNAL_API_KEY`, not a refused credential — there is no session to refresh, and a cooldown would only delay the fix its message names. It still counts toward the consecutive-failure streak and the ordinary outage alert. - **The boot assert runs at module scope** in `sync/app.ts`, outside the boot IIFE's `try`, so a missing key exits non-zero rather than being caught and restart-looping under `restart: unless-stopped`. - **`lib/logger`'s pretty-print selection** now excludes the test runner regardless of `NODE_ENV` or TTY. Bun's runner only sets `NODE_ENV=test` when the environment doesn't already set it, and the dev container exports `NODE_ENV=development` — so under a TTY the documented local test command had **5 pre-existing failures across 4 files**, not just this ticket's own tests. Production output and normal dev-container use are unchanged. ## Tests Whole suite green: **831 pass, 0 fail** (832 tests, 58 files). The 1 skip is the pre-existing curl-impersonate integration test, which self-skips when the binary isn't present on a plain checkout. `bun --cwd=./server run verify` passes all five checks (type-check, lint, format, format:check, test), and the suite is green both in CI's non-TTY environment and under a pty with `NODE_ENV=development`. Coverage added: the new client (URL, bearer header, 200 parse, non-2xx with status, unreachable and real-timeout without status, malformed body), the new branch on the activities module (mapping, non-purchase exclusion, `wsId` derivation, sign/rounding, the client-side window, the optional account pin including unattributed items, single non-paginated request, malformed-purchase throw, 503 with no fallback), both dispatcher directions, the multi-account warning decision *and* its call site (asserted on the pino records the module actually emits), the engine-level internal-hop 401, and the logger's selection logic. ## Operator steps this branch deliberately does NOT perform The `sync` container is live production, so these were left to a human. Both are required before the flag is flipped: 1. **Set `WS_API_KEY` to the same value as `wealthsimple-api`'s `INTERNAL_API_KEY`.** They live in two separate `.env` files and neither codebase can verify the match — `assertWealthsimpleApiClientConfig` catches only an empty key. A mismatch surfaces as a 401 on the first flagged sync. 2. **Run the flag-on comparison** — `ws:activities` against a real `wealthsimple-api` with the flag on, confirming the returned activities match its `GET /activity-feed` cache, filtered and mapped identically to the direct path. Flipping the flag also needs the container **recreated** (`docker compose up -d --force-recreate <service>`), not merely restarted: compose resolves `env_file: .env` at creation, so a plain `docker restart` silently keeps the old value. ## Follow-up worth filing (WSAPI, not this ticket) Reviewers noticed in `wealthsimple-api`'s own source that `isCardPurchase` has no dead-status gate while `isMoreAuthoritative` prefers non-pending — so a dead node sharing a derived id with its live authorized twin could win the cache. Traced through from this side the consequence is bounded and low-severity: the purchase then simply isn't in the feed, and the only local effect is that the vanished-pending check may raise its (accurate) alert for that purchase's previously-synced pending row, deleting nothing. `wealthsimple-api` was not touched by this branch; a WSAPI ticket could tighten it if you want it closed. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

chris added 16 commits

2026-09-14 09:15:01 -06:00

[BTAPI-75] Add wealthsimple-api client config and wire sync to api-shared. e777a15504

[BTAPI-75] Add a thin wealthsimple-api activity-feed client. 71ed922557

[BTAPI-75] Dispatch the activity fetch between Wealthsimple and wealthsimple-api. 173b912674

[BTAPI-75] Document the two activity-fetch implementations behind the cutover flag. 4f764edf21

[BTAPI-75] Fix the wealthsimple-api branch's window, unattributed items, and multi-account visibility. e5c2304893

[BTAPI-75] Give the wealthsimple-api hop its own error type, out of the refusal cooldown. 3f5e05f4a9

[BTAPI-75] Normalize the service URL and move the boot assert outside the try. ef1d1e79bd

[BTAPI-75] Record the widened listener reach and the corrected branch contract in the docs. 983dc86898

[BTAPI-75] Measure the multi-account warning on the whole feed, not the pinned one. cafa081a47

[BTAPI-75] Cover the engine side of the internal-hop 401 and tighten its status contract. d7e933b565