chris/wealthsimple-api
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[WSAPI-7] Session health alerting #6

Merged
chris merged 8 commits from feature/WSAPI-7 into main 2026-09-13 15:46:49 -06:00
Conversation 0 Commits 8 Files changed 9 +546 -32
chris commented 2026-09-13 11:08:40 -06:00
Owner
Copy link

Ticket

WSAPI-7 Session health alerting

A dead Wealthsimple session previously only logged, so a missed or undelivered alert read identically to
"nothing happened" — the gap BTAPI-62/BTAPP-53 closed on budget-tracker-api's side, now closed here.

What this does

  • lib/alert-webhook (new): sendAlert({ kind, body }) — one POST to Config.alertWebhookUrl, bounded by AbortSignal.timeout(5000), and it never rejects. Every failure mode (unset target, transport error, timeout, non-2xx) is logged and swallowed, because a push that replaced the SessionDeadError would leave sync running against a session that is already dead.
  • killSession calls it on the transition from live to dead, gated on the existing markDead() transition boolean — so it fires exactly once per transition, not once per failed call. That guarantee is structural rather than a counter kept in the caller: assertUsable() short-circuits every call once the document reads dead, so killSession only ever runs once anyway.
  • Config.alertWebhookUrl is optional and deliberately un-asserted. Unset is the expected state until the receiver exists, and the service boots normally with it empty — alerting degrades to the server log line plus GET /status.
  • kind is a closed const-array-derived union (ALERT_KINDS → AlertKind, with SESSION_DEAD_KIND as the one shared value), mirroring lib/sync-status's JOBS/Job, with the wire value pinned by a test: it is the routing key a consumer in another repo binds to, so a silent change would break that consumer rather than fail a test.

Scope notes for the reviewer

  • The ticket named budget-tracker-app's existing Expo alert path as the default target. That is not reachable over HTTP — budget-tracker-api calls Notifications.sendAlert in-process, so there is nothing to call — so this ships the generic, configurable caller with the target left unset and documented. BTAPI-80 ("Add alert-webhook receiver so wealthsimple-api can push dead-session alerts") is blocked-by this ticket and builds the receiver. The target is a plain URL rather than a consumer-specific integration so home-assistant-api can point the same variable at its own notify route instead.
  • The alert body is deliberately condition-only — Wealthsimple sync is paused — reconnection needed. — and names no app. The sender cannot know which consumer is listening, so the per-app call to action belongs to the receiver, which knows what it is and routes on kind. The ticket explicitly required not hardcoding to one consumer.
  • The polling half of the ticket (sessionStatus, per-job lastSyncAt via GET /status) was already satisfied by WSAPI-5/6, so there is no route or shape change here.

Security

The webhook URL is never logged. Webhook endpoints routinely embed a secret in their path (Slack, Discord, HA notify targets), and Bun's connection-level fetch errors carry an own enumerable path holding the full request URL — which pino's error serializer copies straight through, so logging err as-is would print the credential-bearing URL at warn on the most likely real-world failure (host down, DNS, unreachable). Errors are therefore rebuilt field-by-field by an exported describeError, which also cannot throw on anything handed to it, since it is called on the way to a halt. Only the URL's origin reaches the log.

Verification

  • bun run type-check, bun run lint, bun run format:check — all clean.
  • bun test — 311 pass, 1 fail, 312 tests across 35 files.
  • The single failure is pre-existing and unrelated: lib/config/.test.ts > the Wealthsimple credentials default to empty strings, never undefined, caused by WS_EMAIL being set in this machine's gitignored .env (Bun auto-loads it). It was proven to fail identically on a main worktree with the same .env symlinked in, and lib/config/ is untouched by this branch.
  • Also run and passing: ALERT_WEBHOOK_URL=https://example.test/hook bun test. That env-set run is worth running: the alert is a fetch like any other, so any test file whose stubbed request counting could be perturbed by it pins Config.alertWebhookUrl explicitly — and this proves that holds with the variable genuinely set.
  • New coverage: lib/alert-webhook/.test.ts (one POST with the exact payload; origin trimming including secret-bearing URLs; describeError against a real path-carrying connection error and five hostile inputs; abort, transport and non-2xx swallowing; the wire-value pin) and three new cases in lib/wealthsimple/auth/.test.ts (exactly once with the exact payload, no request when unset, the halt preserved when delivery fails) plus the simultaneous-failure case now asserted with the alert live.

🤖 Generated with Claude Code

## Ticket [WSAPI-7](http://192.168.2.100:7123/home/browse/WSAPI-7/) Session health alerting A dead Wealthsimple session previously only logged, so a missed or undelivered alert read identically to "nothing happened" — the gap BTAPI-62/BTAPP-53 closed on budget-tracker-api's side, now closed here. ## What this does - **`lib/alert-webhook`** (new): `sendAlert({ kind, body })` — one POST to `Config.alertWebhookUrl`, bounded by `AbortSignal.timeout(5000)`, and it never rejects. Every failure mode (unset target, transport error, timeout, non-2xx) is logged and swallowed, because a push that replaced the `SessionDeadError` would leave sync running against a session that is already dead. - **`killSession`** calls it on the transition from live to dead, gated on the existing `markDead()` transition boolean — so it fires exactly once per transition, not once per failed call. That guarantee is structural rather than a counter kept in the caller: `assertUsable()` short-circuits every call once the document reads `dead`, so `killSession` only ever runs once anyway. - **`Config.alertWebhookUrl`** is optional and deliberately un-asserted. **Unset is the expected state** until the receiver exists, and the service boots normally with it empty — alerting degrades to the server log line plus `GET /status`. - **`kind`** is a closed const-array-derived union (`ALERT_KINDS` → `AlertKind`, with `SESSION_DEAD_KIND` as the one shared value), mirroring `lib/sync-status`'s `JOBS`/`Job`, with the wire value pinned by a test: it is the routing key a consumer in another repo binds to, so a silent change would break that consumer rather than fail a test. ## Scope notes for the reviewer - The ticket named budget-tracker-app's existing Expo alert path as the default target. That is not reachable over HTTP — budget-tracker-api calls `Notifications.sendAlert` **in-process**, so there is nothing to call — so this ships the generic, configurable caller with the target left unset and documented. [BTAPI-80](http://192.168.2.100:7123/home/browse/BTAPI-80/) ("Add alert-webhook receiver so wealthsimple-api can push dead-session alerts") is blocked-by this ticket and builds the receiver. The target is a plain URL rather than a consumer-specific integration so home-assistant-api can point the same variable at its own `notify` route instead. - The alert `body` is deliberately condition-only — `Wealthsimple sync is paused — reconnection needed.` — and names no app. The sender cannot know which consumer is listening, so the per-app call to action belongs to the receiver, which knows what it is and routes on `kind`. The ticket explicitly required not hardcoding to one consumer. - The polling half of the ticket (`sessionStatus`, per-job `lastSyncAt` via `GET /status`) was already satisfied by WSAPI-5/6, so there is **no route or shape change** here. ## Security The webhook URL is never logged. Webhook endpoints routinely embed a secret in their path (Slack, Discord, HA `notify` targets), and Bun's connection-level `fetch` errors carry an own **enumerable** `path` holding the full request URL — which pino's error serializer copies straight through, so logging `err` as-is would print the credential-bearing URL at `warn` on the most likely real-world failure (host down, DNS, unreachable). Errors are therefore rebuilt field-by-field by an exported `describeError`, which also cannot throw on anything handed to it, since it is called on the way to a halt. Only the URL's origin reaches the log. ## Verification - `bun run type-check`, `bun run lint`, `bun run format:check` — all clean. - `bun test` — **311 pass, 1 fail**, 312 tests across 35 files. - The single failure is **pre-existing and unrelated**: `lib/config/.test.ts > the Wealthsimple credentials default to empty strings, never undefined`, caused by `WS_EMAIL` being set in this machine's gitignored `.env` (Bun auto-loads it). It was proven to fail identically on a `main` worktree with the same `.env` symlinked in, and `lib/config/` is untouched by this branch. - Also run and passing: `ALERT_WEBHOOK_URL=https://example.test/hook bun test`. That env-set run is worth running: the alert is a `fetch` like any other, so any test file whose stubbed request counting could be perturbed by it pins `Config.alertWebhookUrl` explicitly — and this proves that holds with the variable genuinely set. - New coverage: `lib/alert-webhook/.test.ts` (one POST with the exact payload; origin trimming including secret-bearing URLs; `describeError` against a real `path`-carrying connection error and five hostile inputs; abort, transport and non-2xx swallowing; the wire-value pin) and three new cases in `lib/wealthsimple/auth/.test.ts` (exactly once with the exact payload, no request when unset, the halt preserved when delivery fails) plus the simultaneous-failure case now asserted with the alert live. 🤖 Generated with [Claude Code](https://claude.com/claude-code) 
Optional, un-asserted config for the dead-session alert target (WSAPI-7):
unset degrades to the log line plus GET /status rather than refusing to boot.

Co-Authored-By: Claude Code <noreply@anthropic.com>
POSTs { kind, body } as JSON to ALERT_WEBHOOK_URL with a bounded timeout. Never
rejects: an unset target, a transport failure, a timeout or a non-2xx response
are all logged and swallowed, so a failed push can never mask the halt.

Co-Authored-By: Claude Code <noreply@anthropic.com>
killSession now pushes { kind: wealthsimpleSessionDead, body } through
lib/alert-webhook, gated on markDead()'s transition boolean so it fires exactly
once per transition rather than once per failed call. Tests restore the
push-alert assertions the port's header comment noted were dropped.

Co-Authored-By: Claude Code <noreply@anthropic.com>
lib/wealthsimple/CLAUDE.md's now-singular scope-trims section drops the 'no
push-alert on session death' bullet and gains a section covering the alert, the
once-per-transition guarantee's dependence on markDead(), the never-masks-the-
halt contract and the unset-is-supported state. Root CLAUDE.md's architecture
tree lists the new module.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Pin Config.alertWebhookUrl in every test file that can reach sendAlert, so the
alert's fetch can't silently join a request count (client/.test.ts failed on any
machine with ALERT_WEBHOOK_URL set). Make the alert body condition-only — naming
one consumer told the other's user to leave the app. Log only the webhook's
origin, since these URLs carry secrets in their path. Type kind as a closed union
with one shared value, assert the abort path behaviorally, and cover the
simultaneous-failure case with the alert live.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Trimming the URL field was not enough: Bun's connection-level fetch failure carries
an own enumerable path holding the complete request URL, which pino copies straight
through — so the common failure (host down, DNS) printed a credential-bearing
webhook URL at warn. Log a rebuilt error instead, via an exported describeError
that also can't throw on anything handed to it. Pin the wire value consumers in
other repos route on, cover the origin trimming directly, and report a scheme with
no origin as such rather than the literal string 'null'.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
compress CLAUDE.md
All checks were successful
server / check (pull_request) Successful in 25s
Details
5a9e2afbdf 
Trim the WSAPI-7 additions in lib/wealthsimple/CLAUDE.md: drop the edit-history
paragraph about the section's former second scope trim, and tighten the
copy-decision paragraph, whose reasoning already lives in .env.example. The three
load-bearing alerting properties are deliberately left intact.

Co-Authored-By: Claude Code <noreply@anthropic.com>
chris merged commit 2a5a31710c into main 2026-09-13 15:46:49 -06:00
chris deleted branch feature/WSAPI-7 2026-09-13 15:46:49 -06:00
chris referenced this pull request from a commit 2026-09-13 15:46:50 -06:00
[WSAPI-7] Session health alerting (#6)
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
chris/wealthsimple-api!6
No description provided.