fix(kyc): stop KycStep.update from clearing omitted fields

[joshuakrueger-dfx]

Why

KycStep.update() built its payload straight from its parameters:

const update: Partial<KycStep> = { status, result: this.setResult(result), comment: this.addComment(comment), sequenceNumber };
Object.assign(this, update);

Object.assign copies undefined, and both callers that omit an argument use undefined to mean
leave this field alone. So the entity lost the field in memory. Two consequences, both live:

1. PUT /kyc/data/financial/:id returned a response missing two required fields.
updateFinancialData calls kycStep.update(undefined, data.responses). The step lost its status,
and KycStepMapper.toStepBase maps StepMap[undefined] → undefined, which drops out of the JSON.
An incomplete draft submit therefore answered 200 without status and without sequenceNumber,
although KycStepBase declares both as @ApiProperty (required).

What this does not fix. It is not the cause of the "status does not change after submitting the
financial information" report. Both frontends evaluate the submit response through
isStepDone(result) in @dfx.swiss/core, which checks
[IN_REVIEW, ON_HOLD, COMPLETED].includes(result.status). That was false with the field absent and
is false with inProgress present, so the user-visible behaviour is unchanged. That report is
still open and is being diagnosed separately; this PR is the contract violation and the merge branch
below, nothing more.

2. An AML branch in mergeUserData has been dead since 2024-11-11.
The cancel loop passes status: undefined for every slave step whose status is not in the cancel
list — COMPLETED included. Because the status was then wiped, the check further down the same
method could never be true:

if (slave.kycSteps.some((k) => (k.type === VIDEO || k.type === SUMSUB_VIDEO) && k.isCompleted)) {
  master.identificationType = KycIdentificationType.VIDEO_ID;
  master.bankTransactionVerification = CheckStatus.UNNECESSARY;
}

That rule was added deliberately in #1424 (2024-07-09) and worked; 6fb9d18536
([NOTASK] fix userData merge, 2024-11-11) moved the loop from direct assignment onto update()
and disabled it as a side effect. There is no revert, no ticket and no comment anywhere that says
this was wanted. This PR makes that branch fire again, which means a merge can now set
bankTransactionVerification = UNNECESSARY where it previously did not. That is the intended
behaviour, but it is a change in an AML-relevant outcome and it is the reason this fix is its own PR
rather than a line in a larger one — please look at it explicitly.

What changed

• update() drops entries whose value is undefined before assigning and before returning the partial.
• status becomes optional in the signature, matching the two call sites that pass undefined.
  It only compiled before because the project has no strictNullChecks.
• addComment returns string instead of string | undefined — Array.prototype.join always
  returns a string, and update() in the same file is its only caller.

Blast radius

update() has 11 production call sites. The change is in-memory only: TypeORM already strips
undefined from the update statement (UpdateQueryBuilder), so the SQL sent for every caller is
byte-for-byte what it was. Of the 11, two pass undefined:

Call site	Before	After
kyc.service.ts updateFinancialData	status wiped → response lost status/sequenceNumber	preserved
user-data.service.ts mergeUserData	status of every non-cancelled slave step wiped → VIDEO branch dead	preserved, branch fires
updateFileData, updateKycStepAndLog	sequenceNumber wiped → absent from the response	present
remaining 7	sequenceNumber wiped in memory, never read afterwards	no observable change

No caller passes undefined in order to clear a field; clearing is done with explicit null in
this repo (e.g. reminderSentDate: null in pause()).

How much damage the dead branch actually did — measured, not assumed

The fix is forward-only, so the question is whether the accounts merged while the branch was inert
need a backfill. Measured against production via the read-only /gs/debug query endpoint:

Accounts owning a completed Video / SumsubVideo ident step	791
of those, identificationType = VideoId	764
of those, something else (OnlineId 15, null 11, Manual 1)	27
of those 27, ident step last updated on/after 2024-11-11 (the regression)	2

So the upper bound for accounts this regression could have affected is two (userData 191940
and 284664), and it is genuinely an upper bound: the step's updated timestamp is not the merge
date, and a mismatch has other possible causes — an account that completed an online ident before
the video one legitimately carries OnlineId.

No backfill is proposed. Two possible cases do not justify a migration over user_data; if the
two turn out to be real, they are a manual correction. The number is recorded here so the decision
is visible rather than implied.

(Method, reproducible: kyc_step where name = Ident, type IN (Video, SumsubVideo),
status = Completed, grouped by userDataId → 791 ids; then user_data for those ids in batches of
100, which is the endpoint's IN cap. A first attempt used batches of 200, was rejected with
IN list size must be 1..100, and the rejected batches parsed as zero rows — the numbers above are
from the corrected run, with all 8 batches confirmed to have returned 791 rows and no error object.)

Tests

Six new tests. Every one was checked twice — once with the fix removed, once with the filter
mutated — so that "removing the fragment turns it red" is not the only evidence:

Probe	Result
Fix removed (kyc-step.entity.ts reset to develop)	5 of 108 fail, 3 of 3 suites red
Mutation v !== undefined → v !== null	5 of 108 fail, the same five

The five are: the entity omit case, the financial draft response, the updateFileData
sequenceNumber, and both merge cases (Video and SumsubVideo). The sixth test — an explicit
status change still being applied — stays green under both probes on purpose: it is the
counter-direction guard that catches a filter that is too broad, not a proof of the fix.

Full gate on the exact revision 3a45ada4f: lint (empty output), format:check, type-check
and build all exit 0; Test Suites: 340 passed, 340 of 347 total,
Tests: 6211 passed, 174 skipped, 6385 total; coverage gate exit 0.

Scope

Deliberately not in this PR: the complete / missingFields submit response, any change to
FinancialService, and the conditions contract declaration (that one is a separate PR). This
branch touches kyc-step.entity.ts and four spec files, nothing else.