feat(support): brand support mail by the app the ticket was opened from (X-Client)

[joshuakrueger-dfx]

Problem

Support reply mails could arrive RealUnit-branded for DFX support answers (and vice-versa). Branding was sent without an explicit wallet, so resolveMailWallet's account-history override forced RealUnit onto any account ever linked to a RealUnit user.

Approach

Brand each support mail by the app the ticket was opened from, derived from the trusted per-request X-Client header — not from the user's persisted wallet (which is set once at signup and doesn't identify the calling app).

• src/shared/utils/request-client.ts — central client detection (X-Client: realunit-app, plus the legacy /v{n}/realunit/ path); the api-trace middleware now uses it too (single source of truth).
• SupportIssue.wallet — new nullable relation, eager-loaded; migration adds column + index + FK (TypeORM DefaultNamingStrategy names). Set at creation from X-Client: RealUnit app → RealUnit wallet, every other client → null.
• Branding rule (product decision): DFX is the default; only RealUnit-app tickets are RealUnit. null means DFX by design → walletId stays nullable, no backfill.
• newSupportMessage brands by issue.wallet ?? default(DFX); passing the wallet explicitly also bypasses the account-history override.
• Sibling limit-request mail migrated to the same issue source.

Prerequisites for RealUnit branding in prod

1. RealUnit app must send X-Client: realunit-app on the support-ticket request.
2. REALUNIT_MAIL_USER env must be set (else the RealUnit template is absent and it renders DFX).

Verification

Local (clean worktree off develop): tsc, prettier, eslint clean; full unit suite 1068 passed, 0 failed. Tests: X-Client detection (6), branding by issue source vs DFX default, no-mail; NotificationService resolveMailWallet safeguard. Migration is a new file (immutability check passes).

[TaprootFreak]

Re-reviewed the updated approach (issue-source-wallet branding via the new SupportIssue.wallet). This is a clean solution and resolves the concern from the earlier UserData.wallet-only version: branding now follows the wallet the ticket was opened from — the login User row via jwt.user (getUser(sourceUserId, { wallet: true })) — with origin-wallet and DFX-default fallbacks. SupportIssue.wallet being eager makes entity.issue.wallet reliably available on every path, including the bot job. jwt.account (userData id) + jwt.user (user/wallet id) are threaded correctly.

Verified locally off a clean clone (head 719a6bf): tsc --noEmit ✅, eslint ✅, prettier ✅, unit spec ✅ 5/5.

One should-fix before merge (migration index/FK names — inline) plus minor notes.

Two notes that don't map to a changed line:

• Reused issues keep their original source wallet. createIssueInternal reuses a matching open issue (const entity = existingIssue ?? save(newIssue)), so the freshly resolved sourceWallet is intentionally dropped when an issue is reused — branding follows the ticket's original source. Looks intended; flagging in case a continued ticket from a different app should re-evaluate the source.
• Same override still affects other USER_V2 mails (out of scope here): KYC, TFA, account-merge, user-data and unassigned-tx notifications still pass userData without input.wallet, so the RealUnit account-history override still applies there. This is now the second caller-side fix for the same heuristic (after the #3846 login-mail fix) — worth a follow-up, or hardening resolveMailWallet centrally.

[TaprootFreak]

Index and FK names don't match TypeORM's DefaultNamingStrategy — they look hand-written. The repo convention is that hand-written migrations use the exact names TypeORM would generate, so the live schema matches the entity metadata.

For support_issue.walletId the correct names are:

• index: IDX_f5224979beab23e21df3066a60
• FK: FK_f5224979beab23e21df3066a60a

Verified against this repo's TypeORM DefaultNamingStrategy; sanity-checked by reproducing the existing user_data.walletId names (IDX_dcf41efbd8bd1f2a80f369028b / FK_dcf41efbd8bd1f2a80f369028b2) exactly with the same method.

The current names are also structurally off: the FK reuses the same 26-char hash as the index, whereas a TypeORM FK name is one char longer than the index hash. With the wrong names, the next migration:generate / schema comparison will treat the entity's @Index()/FK as missing and emit a spurious drop+recreate (schema drift). Please update all four occurrences — up (lines 18–19) and down (lines 26–27).

[TaprootFreak]

Correction — I retract my earlier praise of this priority chain; it was wrong given the hard requirement for this feature.

Requirement (non-negotiable): every support ticket must be attributed to its originating application exactly — RealUnit (RealUnitCH/app) or DFX (DFXswiss/services, app.dfx.swiss) — and the mail template must be chosen solely from that exact attribution. Heuristics and fallbacks are forbidden. If the source cannot be determined exactly, the system must fail closed (no mail / explicit error), never guess a brand.

This line violates it. entity.issue.wallet ?? getUserData(...).wallet ?? getDefault() is a two-step heuristic fallback (account-origin wallet, then DFX default). It silently mis-brands whenever issue.wallet is null.

issue.wallet is null today on at least one path: createIssueBySupport → createIssue(+userDataId, input) (support-issue.controller.ts:69) passes no source user, so support-created tickets carry no exact source and fall into the heuristic.

Required instead:

1. Set SupportIssue.wallet to the exact originating client on every creation path, including support-created tickets (the agent/UI must specify the product, or derive it from an unambiguous signal — never guess).
2. Make the column NOT NULL (after backfill) so an unattributed ticket cannot exist.
3. Brand from only issue.wallet; drop the ?? origin ?? default chain. If it is ever unresolvable, fail closed.

Template mapping for reference (mail.factory.ts:152-153): branding keys off wallet.name via Config.mail.wallet — RealUnit → realunit; there is no explicit DFX entry, so DFX is the default template. For 100% exactness an explicit DFX wallet→template mapping is preferable to "anything not RealUnit = default".

[joshuakrueger-dfx]

You're right — the heuristic is gone. Fixed in bd674c37e.

Branding is now exact-or-fail-closed. newSupportMessage brands strictly from issue.wallet; if it's null the mail is skipped (logged), never guessed. Dropped the ?? origin ?? default chain and the UserDataService/WalletService deps with it.

On the three required items:

1. Exact source on every creation path

• Customer tickets: exact, server-derived from the login user (jwt.user → its wallet) — not client-supplied, so it can't be spoofed. ✓
• Transaction-request tickets: exact from the request's user wallet. ✓
• Support-created tickets (createIssueBySupport): no app source exists server-side. Per your note these need the agent/UI to specify the product — that's a SUPPORT-role-only signal (must not be client-trusted) and a new FE+API contract, so I've left it as a follow-up. Until then they fail closed (no mail), no guess.

2. NOT NULL column — can't be satisfied yet without breaking the no-guess rule on legacy rows. Existing tickets have no recorded source; transaction-linked ones are backfillable exactly from the transaction's user wallet, but non-transaction legacy tickets have no exact source, so backfilling them would itself be a guess. Proposal: backfill the exactly-derivable rows, decide a policy for the rest (or close them), then a follow-up migration flips NOT NULL. Needs a data/product call.

3. Explicit DFX template mapping — branding from issue.wallet is already exact: RealUnit → realunit, and a DFX-sourced ticket carries the DFX wallet whose name has no Config.mail.wallet entry → default = DFX template (mail.factory.ts:152-153). Happy to add an explicit DFX entry if you'd prefer it spelled out rather than relying on "default = DFX".

Net: no mis-branding is now possible. The open items (support-created attribution, NOT NULL backfill) are the ones needing a coordinated FE/data decision — can split them into a follow-up issue if you prefer to keep this PR to the safety fix.

[TaprootFreak]

Minor (coverage): since NotificationService is fully mocked and sendMail stubbed (line 56), resolveMailWallet never runs in any test — so the actual safeguard that fixes the bug (if (input.wallet) return, notification.service.ts:119) is never exercised. The unit boundary is reasonable, but if that line or the override changes later, this suite stays green while branding silently regresses. An integration-style test letting the real resolveMailWallet run (override skipped when input.wallet is set, applied when it isn't) would lock it in.

[joshuakrueger-dfx]

Thanks for the thorough re-review, @TaprootFreak — all addressed in 6f90a1f1c.

Should-fix — migration index/FK names (fixed). You were right; mine were hand-written and both wrong and structurally invalid (FK reused the 26-char index hash). Now using the DefaultNamingStrategy names, independently verified:

• printf %s 'support_issue_walletId' | shasum → f5224979beab23e21df3066a60a… ⇒ index IDX_f5224979beab23e21df3066a60 (26), FK FK_f5224979beab23e21df3066a60a (27)
• same method reproduces the existing user_data.walletId names (IDX_dcf41efbd8bd1f2a80f369028b / FK_dcf41efbd8bd1f2a80f369028b2) exactly.

(Editing the migration is safe vs. the immutability check — the file is new in this PR, so it diffs as added against develop.)

Coverage note (added). New notification.service.spec.ts lets the real resolveMailWallet run (mocking createMail→undefined to short-circuit sendMail right after it): asserts an explicitly set input.wallet skips the override (the if (input.wallet) return safeguard the support fix relies on) and that it falls back to the account wallet otherwise. The override-applies branch needs Config.mail.wallet.RealUnit (gated behind REALUNIT_MAIL_USER, unset in tests), so I didn't force that path — happy to add an env-gated case if you'd prefer.

Note 1 — reused issues keep their original source wallet: intended. A continued ticket stays branded by where it was first opened; re-evaluating the source on reuse from a different app would be a deliberate product change, not done here.

Note 2 — other USER_V2 mails (KYC/TFA/merge/…): agreed, out of scope. This is now the second caller-side fix for the same heuristic; I'd suggest a follow-up to harden resolveMailWallet centrally (e.g. brand by the user's origin wallet rather than the any-preferred-link override) so we stop patching it per caller. Can open an issue if useful.

[TaprootFreak]

Required: exact, deterministic source attribution for support-ticket mail branding (no heuristics)

Authoritative requirement for this PR. Supersedes my earlier notes that endorsed deriving the source from the user's wallet — see the hard constraint below.

Requirement (non-negotiable)

Every support ticket is created from exactly one application — RealUnit (RealUnitCH/app) or DFX (DFXswiss/services, app.dfx.swiss). Each ticket must be attributed to its source app 100% exactly, and the mail template chosen solely from that exact attribution. No heuristics, no fallback chains, no implicit defaults. If the source cannot be resolved exactly, fail closed (no mail / explicit error) — never guess a brand.

Hard constraint — forbidden sources

The attribution source must be an explicit, reliable signal of the creating application. The following are not reliable and are not permitted as the source:

• user.wallet / user.wallet.id (the authenticating user's wallet). It is set once at signup as data.wallet ?? getDefault() and never updated (user.service.ts:270), it is not 1:1 with the two apps (many DFX-ecosystem wallets all render as DFX default), and it is ambiguous for multi-wallet accounts and account/mail-login tokens.
• userData.wallet (account origin) — account-history inference; ambiguous for multi-wallet accounts.
• transactionRequest.user.wallet — same as user.wallet.
• getDefault() / "anything not RealUnit = default" — an implicit guess.

Consequently the current resolution is built entirely on forbidden sources: getUser(jwt.user).wallet (path a), transactionRequest.user.wallet (path b), and the ?? userData.wallet ?? getDefault() chain. Note: the API has no reliable app signal at the ticket boundary today (no Origin/client header handling; the DTO has no source field) — a correct solution must introduce one.

Blockers in the current PR (@6f90a1f1c) — verified against the code

1. Forbidden heuristic chain (support-issue-notification.service.ts:31-34): issue.wallet ?? getUserData(...).wallet ?? getDefault() — steps 2+3 are forbidden inference/default.
2. Fail-closed is structurally impossible today. Branding only ever recognizes RealUnit (resolveMailWallet filters on isPreferred; mail.factory.ts:152-153 looks up Config.mail.wallet[name] where only onchainlabs/RealUnit exist). There is no explicit DFX entry, so "source unknown" and "DFX" share the same catch-all default path — a correctly-attributed DFX ticket is indistinguishable from a guessed one, and "stop, unknown" cannot be expressed.
3. Source derived from forbidden user.wallet on every "exact" path (controller:52, controller:53).
4. Support-created tickets unattributed by design (controller:69 createIssue(+userDataId, input) — no source → issue.wallet = NULL).
5. Account/mail-login token path has no jwt.user: generateAccountToken sets account but not user → path (a) gets sourceUserId=undefined → NULL. A whole legitimate login type is never attributed.
6. getUser(...).then(u => u?.wallet) silently coerces not-found/null-wallet to undefined → NULL → heuristic, instead of failing.
7. Column nullable, no backfill, no NOT NULL/invariant — walletId=NULL is a permanent normal state for new tickets, not just legacy.
8. Dedup discards the freshly resolved source (const entity = existingIssue ?? save(newIssue)), so reused/legacy issues keep their old/NULL attribution.
9. Multi-wallet ambiguity — step-2 userData.wallet is the account's default-user wallet, not the ticket source; RealUnit registers per wallet address, so one account can hold both DFX and RealUnit users → wrong brand even without NULL.
10. Tests cement the forbidden behavior — the specs assert the origin- and default-fallbacks as "correct" (they would need rewriting under fail-closed), and createCustomWallet drops name, so every branding assertion only checks object identity; the actual template/brand selection is never tested.
11. Sibling support mail not migrated — limit-request-notification.service.ts:46 still brands/skips via userData.wallet?.name (origin); same violation class, same SupportIssue.
12. RealUnit branding is env-gated — Config.mail.wallet.RealUnit exists only when REALUNIT_MAIL_USER is set; without it even an exactly-attributed RealUnit ticket renders as DFX.

Fixed and verified (credit where due)

• ✅ Migration index/FK names now match TypeORM DefaultNamingStrategy (IDX_f5224979beab23e21df3066a60 / FK_f5224979beab23e21df3066a60a, all four occurrences) — independently re-verified.
• ✅ The resolveMailWallet safeguard test (if (input.wallet) return) is a solid addition.

What a correct solution requires

1. Introduce an explicit, server-trusted app/source identifier for support tickets (NOT derived from user.wallet/userData.wallet) — e.g. an authenticated client/app claim or a trusted, validated request field — set on every creation path, including support-created and mail-login sessions.
2. Persist it on the ticket and make it NOT NULL (backfill existing rows first).
3. Select the template only from that exact source; add an explicit DFX mapping so DFX is a positively-attributed state, not the absence of RealUnit.
4. Fail closed when the source is not exactly resolvable (no mail / explicit error), never default.
5. Migrate the sibling support mails (limit-request) to the same exact source.
6. Tests must assert the actual brand/template per source and the fail-closed path; fix the wallet mock to carry name.

Until 1–4 hold, the feature cannot meet the requirement. Note: the proposed follow-up "brand by the user's origin wallet" is itself one of the forbidden sources above and would not satisfy this.

[joshuakrueger-dfx]

Reworked per your authoritative requirement — the source is no longer wallet-derived. Pushed in 08302aa65.

Source = the trusted per-request X-Client signal (the one you noted exists), centralized in src/shared/utils/request-client.ts (single source of truth, the api-trace middleware now uses it too). Forbidden sources removed: dropped jwt.user/getUser().wallet, transactionRequest.user.wallet, and the ?? userData.wallet ?? getDefault() heuristic.

SupportIssue.wallet is set at creation from X-Client: realunit-app → RealUnit wallet; every other client → null.

One deliberate product decision from @joshuakrueger-dfx that diverges from your spec: DFX is the default brand; only RealUnit-app tickets are branded RealUnit. So null is a valid, intended state meaning DFX — not an unattributed error. Consequences, stated honestly:

• This is positive attribution for RealUnit (explicit X-Client), default for DFX — i.e. we are not requiring a positive DFX signal or fail-closed. That's the product call, not an oversight.
• Therefore walletId stays nullable by design (null = DFX); no NOT-NULL/backfill needed.
• No explicit DFX template entry is added — DFX intentionally uses the default template.

Addressed from your list regardless of the default decision:

• ✅ Source is the trusted client signal, not user.wallet (blockers 1, 3, 5, 6, 9).
• ✅ Set on customer + transaction-request paths; support-created tickets have no app client → DFX default (4).
• ✅ Dedup keeps the original ticket's source — intended for a continued ticket (8).
• ✅ Sibling limit-request mail migrated to the issue source instead of userData.wallet (11).
• ✅ createCustomWallet mock now carries name; branding tests assert the actual wallet.name per source + a unit spec for the detection (10).

Two real prerequisites for RealUnit branding to work in prod (flagging, not solved here):

1. The RealUnit app must send X-Client: realunit-app on the support-ticket request (it already sends the header generally; needs confirming for this endpoint — otherwise RealUnit tickets fall to the DFX default).
2. REALUNIT_MAIL_USER must be set in the environment, or even a correctly-attributed RealUnit ticket renders with the default template (12).

If you still require positive DFX attribution + fail-closed over the product's default-DFX decision, that's a call for you and @joshuakrueger-dfx — happy to switch to strict mode, but it would gate all DFX support mails on a new DFX-app header first.

Full suite: 1068 passed, 0 failed.

[davidleomay]

Blockers 1, 2, 12 from TaprootFreak's comment still apply here.

1. Heuristic default not removed. issue.wallet ?? walletService.getDefault() still defaults when source is unknown — exactly the implicit guess the requirement forbids.

2. No explicit DFX template. Config.mail.wallet has no DFX entry, so an exactly-attributed DFX ticket and a ticket with unknown source share the same catch-all path. "Unknown" cannot be expressed; fail-closed is structurally impossible.

12. Env-gated RealUnit template. Even for an exactly-attributed RealUnit ticket, the mail silently renders DFX when REALUNIT_MAIL_USER is unset (because Config.mail.wallet.RealUnit is conditional). PR description calls this a prereq; under strict attribution this should at minimum warn-log when source=RealUnit but template is absent.

To meet the requirement: add an explicit DFX entry in Config.mail.wallet, drop the ?? default, and skip-with-warn when issue.wallet is null.

[davidleomay]

Blocker 4 (TaprootFreak): support-staff path remains unattributed.

createIssueBySupport (just below, line 71) still calls createIssue(+userDataId, input) without forwarding any client/source, so every support-staff-opened ticket gets issue.wallet = NULL and falls through to the DFX default. Under strict attribution this is not allowed.

Options: (a) add an explicit source field to CreateSupportIssueSupportDto that the agent must pick (DFX/RealUnit), (b) forward X-Client on this endpoint too, or (c) explicitly document and encode that staff-created tickets are always DFX (requires the explicit DFX wallet entry from Blocker 2).

[davidleomay]

Blocker 7 (TaprootFreak): nullable column, no backfill, no NOT NULL invariant.

walletId stays nullable indefinitely. Combined with the support-staff path (Blocker 4) and dedup (Blocker 8), NULL is a permanent normal state for new tickets, not just legacy — so the attribution invariant is not enforced at the schema level.

The strict requirement is: backfill existing rows, then make the column NOT NULL. If the team accepts NULL=DFX as policy instead, that decision needs to be encoded explicitly (via the DFX wallet entry from Blocker 2), not left implicit.

[davidleomay]

Blocker 8 (TaprootFreak): dedup will discard the freshly resolved source.

The newly created newIssue carries sourceWallet, but at line 230 the code does const entity = existingIssue ?? (await this.supportIssueRepo.save(newIssue)). When an existing matching issue is reused (same userData/type/reason), its (possibly NULL or stale) wallet sticks and the freshly resolved sourceWallet is silently dropped.

Concrete failure: user opens a ticket from the DFX app, later re-opens from the RealUnit app under the same userData/type/reason — second request keeps DFX attribution. Mitigation: detect divergence (existingIssue.wallet?.id !== sourceWallet?.id) and either update the row or treat as a new issue.