fix(react): keep the support chat sync alive, and let a failed message be sent again

[joshuakrueger-dfx]

Not symptom-driven: no incident report — found by reading support.context.tsx end to end while working on the customer support chat in DFXswiss/services. Every finding below is quoted from the file as it stands on develop.

Scale: @dfx.swiss/react is consumed by DFXswiss/services, where /support/chat is the only way a customer reads and answers a support ticket; no other repository consumes this context today (checked against dfx-wallet, realunit-app, btc-wallet: 0 hits). Finding 1 below hits any session in which a single request fails — every network switch with the chat open, not a rare constellation. No production measurement is available, and that is stated rather than glossed over.

Smaller fix considered: the smallest change that removes finding 1 alone is one line — increment a counter in the catch and add it to the effect's dependency list so the chain keeps going. Deliberately not chosen: it leaves the chain as a construction, so the next change that introduces a path without a state update breaks it again, and again silently. A real interval cannot have this failure mode. Findings 2–5 are independent and small on their own; they are taken along because they sit in the same file and the same flow, and none of them justifies a release by itself.

---

What was wrong

1 — The sync stops for good after one failed request.

// support.context.tsx:42-46 on develop
useEffect(() => {
  const interval = setTimeout(() => sync && syncSupportIssue(), 5000);
  return () => clearInterval(interval);
}, [supportIssue, sync]);

That is a chain, not a loop: the next run only exists because syncSupportIssue changes supportIssue and the effect re-runs. The error branch (line 77) sets isError and leaves supportIssue untouched, so no further run follows. One failed request — a WLAN switch, a brief 5xx — and the chat never updates again. Nothing is visible to the customer: isError is set but never read by the chat screen. (The handle also comes from setTimeout and is cleared with clearInterval; works in browsers, still wrong.)

2 — State was mutated instead of replaced. Lines 87, 135, 166, 197 and 243 wrote into the existing object (messages.push(...), message.file = newFile) and returned a shallow copy, so the messages array kept its identity. Under StrictMode the updater runs twice and the message is inserted twice.

3 — settleMessage resolved by index against stale state.

// support.context.tsx:235-237 on develop
const idx = supportIssue?.messages.findIndex((m) => m.id === messageId);
if (!supportIssue || !idx || idx === -1) return;

supportIssue comes from the closure of the render that started the upload, while submitMessage (line 119) starts one createMessage per file in parallel — the second callback computes its index against an array that does not know about the first. And !idx is true for idx === 0, so the first message of a thread was never settled and kept showing the clock forever.

4 — The delta anchor could be a placeholder id. Line 74 took messages[length - 1].id. Optimistic messages carry negative placeholder ids, so fromMessageId=-1 reached the server and it returned the entire thread. The same line threw on an empty thread.

5 — A failed message was unrecoverable. settleMessage(messageId) without a payload set the status to Failed but kept the negative placeholder id, and the entry existed only in this context — never on the server. There was no way to retry it and no way to remove it, so the next loadSupportIssue rebuilt state from the server and the customer's text was gone.

What changed

Four pure functions in src/support-messages.ts carry the logic and are directly testable:

Function	Purpose
lastSettledMessageId	highest confirmed id, undefined if there is none
mergeMessages	append what is not already present, always a new array
settleMessage	resolve by id, never by index; drops the optimistic entry if a sync already delivered the settled one
prepareRetry	claims a Failed entry in place and hands back its payload

In the context: a real setInterval that the error branch cannot end, current state read through refs instead of closures, every update in updater form, and retryMessage(messageId) added to the interface.

retryMessage is additive — no existing field renamed or removed, no signature changed. A second call while the first is in flight is a no-op, because the entry is already Sent by then.

Verification

Full gate on a 28-core machine, on the exact SHA of this branch (compared with the local one):

• npx lerna run lint — exit 0, empty for @dfx.swiss/react; the 4 remaining warnings are pre-existing in bip322-multisig test files, untouched here
• npx lerna run format:check — clean
• npx lerna run build — clean; dist/__tests__ is not emitted (see below)
• npx lerna run test — core 7 suites / 72 tests, bip322-multisig 4 / 46 (1 todo), react 1 / 24

@dfx.swiss/react had no test script at all, so lerna run test silently skipped it. The setup added here is copied from packages/core (same jest.config.js shape, same tsconfig.test.json including strict: false) — no new pattern invented. That surfaced a second thing: packages/react/tsconfig.build.json, unlike core's, did not exclude src/__tests__. With files: ["dist"] the compiled tests would have shipped in the published package. Fixed in the same commit.

Counter-checks. Every new test fails without the fix. Against a copy of the old logic: 9 of 12 red. Eight mutations, each red, each with the failing test named:

Mutation	Result
!idx restored in settleMessage	3 failed
sign filter removed from lastSettledMessageId	1 failed
push instead of spread in mergeMessages	2 failed
mergeMessages order reversed	1 failed
status assignment in settleMessage swapped	5 failed
duplicate guard removed	1 failed
prepareRetry status guard loosened	2 failed
retry entry appended instead of claimed in place	3 failed

What is not covered

The interval itself has no test. The four pure functions are covered; the effect that drives them — the interval, the refs, the overlap guard — is not, because this package has no React renderer and none was introduced for this. Finding 1 is therefore argued structurally, not pinned by a test: a setInterval whose scheduling does not depend on a state change cannot be ended by an error branch. If that is not enough, the honest fix is a renderer and @testing-library/react as dev dependencies, which is a separate decision.

Also not verified: behaviour in a real browser under StrictMode, and a real network failure followed by a retry end to end. Both were exercised as logic, not as a running app.

Diff size

8 files, +8111 lines — of which 7621 added / 1507 removed are package-lock.json, the dependency tree of jest and ts-jest under packages/react/node_modules/. The actual source change is two files — support.context.tsx (+141/-70) and the new support-messages.ts (+63) — plus 252 lines of tests and 21 lines of jest setup. Calling this a surgical diff would be wrong, so here are the numbers.

Downstream

DFXswiss/services consumes retryMessage in its customer chat, but only after this is released — the branch there deliberately does not use it yet, because npm ci resolves the published 1.7.x and the build would fail. Per CONTRIBUTING: add additively, release, then consume.

• Release @dfx.swiss/react after merge. The version bump belongs to the Lerna publish, not to this branch — every version change on packages/react/package.json in the last eight commits came from a Publish commit. Needs whoever runs the release.
• Then raise the dependency in DFXswiss/services, which is pinned at ^1.7.0 and therefore does not include 1.8.x. Not doable from this repository.

A sync response could land in the wrong ticket

The interval reads the open issue from a ref, asks the API for it, and merged whatever came back
into whatever was open by the time it answered — nothing compared the two. Switching tickets while
a getIssue is in flight therefore merged the old ticket's messages into the new one: foreign
conversation inside an open support chat. The ref lags the state by an effect tick, which widens
the window, and keeping the sync alive after an error — the main change in this PR — widens it
further.

applySupportIssueUpdate now compares the uid and discards a response that does not belong to
the issue on screen. It sits with the other pure helpers, so the existing helper tests cover it.

Counter-probes, each hitting exactly one behaviour: dropping the comparison leaves 1 of 24 red
(the stale-response case), inverting it leaves 2 of 24 red (both cases).

createSupportIssue calls the same function. With another ticket open its response is now
discarded rather than merged under the wrong uid — both consumers navigate to the new thread
straight after (startChat in support-issue.screen.tsx, openThread in app2/screens/support.tsx),
and that path loads the issue through loadSupportIssue, which sets state directly. So the new
ticket cannot be swallowed.

Lockfile

package-lock.json grows by 7621 lines and loses 1507. That is not a surgical change: adding jest
and ts-jest for the new test script re-resolves ranges across the tree. CI is green on it.

Final pass (8c16f03):
Coherent: Every change serves one subject — the support chat sync surviving and staying correct. Keeping the interval alive, settling by id, retrying a failed message and discarding a foreign response are the four ways the same sync loop was broken.
Nothing extra: No AbortController, no generation counter, no rewrite of syncSupportIssue, and no fix for the three pre-existing test-hygiene points the mechanical gate reports (a bare toBeDefined, a fixed calendar date, a toHaveLength(3) without index assertions) — all three predate this branch's last commit and none sits in the new tests.
Sources closed: External review of 2026-08-10 (reviewer account since suspended, findings preserved only in mail): the stale-sync finding is fixed here; the submitMessage fire-and-forget and the createSupportIssue-with-open-ticket findings are named above as deliberately unbuilt. No reviewer commits, no open comments on any of the three channels.

[joshuakrueger-dfx]

@mara-steiner could you take a look? This keeps the support chat sync alive: it was a chain rather than a loop, so a single failed request ended it for good and the chat silently stopped updating. Also settles optimistic messages by id instead of index, and adds retryMessage so a failed message can be sent again instead of being lost on the next load.

@dfx.swiss/react had no test script at all, so lerna run test skipped it; the setup added here is copied from packages/core. DFXswiss/app#1297 consumes retryMessage once this is released.

[joshuakrueger-dfx]

@TaprootFreak @davidleomay please review

This is a PR, not an issue: 8 files against develop, head 8c16f03, checks green.

One commit added since: the sync merged whatever getIssue returned into whatever ticket was open by the time it answered, without comparing the two. Switching tickets while a request is in flight put the old ticket's messages into the new one. Keeping the sync alive after an error — the main change here — widens that window, so it belongs in this PR.

Counter-probes and the reason createSupportIssue is unaffected are at the end of the description. The lockfile grows by 7621 lines; that is jest/ts-jest re-resolving ranges, not a surgical change.

[marassteiner]

EN: Approving — the full gate suite passes locally on 8c16f03 and no finding meets the merge-blocker bar, with six follow-ups filed as #215–#220.
DE: Freigabe — die vollständige Gate-Suite läuft lokal auf 8c16f03 durch und kein Fund erreicht die Blocker-Schwelle; sechs Follow-ups sind als #215–#220 erfasst.

Details

Reviewed 8c16f03 against develop (merge-base 3e693a3). Two independent review passes over the full diff — one on conformance, one on correctness — plus a local run of the complete gate suite.

Verdict

Approve. No merge blocker. Six findings are filed as follow-ups; the two that matter most are unreachable today, and one of them should be closed before a consumer adopts retryMessage.

Findings

Ordered by severity. None of these cause harm if this PR merges today, which is why they are follow-ups rather than change requests.

#	Severity	Finding	Reachable today?
#215	major	A sync response for the previous ticket is adopted during a ticket switch, because applySupportIssueUpdate returns incoming when prev is undefined and loadSupportIssue clears the ticket before fetching	yes, but identical on develop
#216	major	Retrying a failed createSupportIssue posts the text into a different, open ticket instead of re-creating the ticket	no — retryMessage has no caller
#217	major	retryMessage can duplicate a message when the original send failed ambiguously (no idempotency key on CreateSupportMessage)	no — retryMessage has no caller
#218	minor	submitMessage posts to a captured issueUid but writes optimistic state into whatever ticket is open	pre-existing, unchanged by this PR
#219	minor	No tests for the provider layer — interval, overlap guard, retry integration	acknowledged in the description
#220	minor	CONTRIBUTING.md no longer lists @dfx.swiss/react among the tested packages	docs only

Why none of these blocks the merge

#215 is the same class of bug this PR sets out to fix, and it is the one finding I hesitated over. It is not a blocker because the behaviour is byte-for-byte the one already on develop — updateSupportIssue there also returned newState when no ticket was open — and this PR closes the far more common variant of the same race. Worth stating plainly: this PR makes the situation better, not worse, but it does not make it correct. If a reviewer reads the widened polling window differently, that is a fair disagreement and I would rather name it than quietly leave it out.

#216 and #217 are real defects in new code, but retryMessage has no caller anywhere in this repository — grep -rn retryMessage packages/ returns only the interface declaration, the definition and the context value. The path cannot execute until a consumer adopts the released version, which the description already sequences as a separate step. That puts both squarely in "fixable before activation, activation not part of this PR". They should be closed before the consuming app wires the button up.

#218 predates this branch. This PR in fact improves the surrounding code by capturing issueUid before the await rather than reading it after.

What I verified myself, rather than taking from the description

The description is unusually thorough and self-critical; these are the points I checked independently rather than accepting.

The lockfile is purely additive. +7621/-1507 lines invites suspicion of a smuggled dependency bump. Comparing both lockfiles entry by entry: 211 entries added, 0 existing entries changed version, 0 removed, and all 211 are dev-flagged. The line churn is npm reordering. Nothing that ships to consumers moves.

No hand-bump. No version field and no CHANGELOG.md is touched, so the "do not hand-bump" rule in CONTRIBUTING.md is not in play. Workspace versions in the lockfile are unchanged on all four packages.

The fromMessageId fix is real. SupportUrl.getIssue builds its query as fromMessageId ? '?fromMessageId=' + fromMessageId : '', so undefined omits it. On develop the anchor was messages[length - 1].id, which is negative for an optimistic entry and therefore truthy — a placeholder id was genuinely reaching the API.

The stale interval closure is harmless. The interval closes over getIssue from one render, which looked like a stale-token risk. It is not: getAuthToken reads tokenRef.current, so the token is resolved live at call time.

The build excludes the tests. dist/ contains no __tests__ directory, and support-messages.js/.d.ts are emitted. The new exclude in tsconfig.build.json does what it claims.

The claim that createSupportIssue is unaffected does not fully hold. With another ticket open, its response is now discarded on uid mismatch while the optimistic entry has already been settled into the open ticket. The description addresses the first half and argues both consumers navigate away via loadSupportIssue; that cannot be verified from this repository. The retry consequence is #216.

The test setup mirrors packages/core exactly. Same jest.config.js shape, same tsconfig.test.json including strict: false. No new pattern invented — the only addition is a moduleNameMapper for @dfx.swiss/core, which is needed because that package ships extensionless ESM.

Local run — LOCAL_RUN_OK

This is a library monorepo with no application to start, so the local run is the path documented in CONTRIBUTING.md. Every step exit 0 on 8c16f03:

npm ci                        ok
npx lerna run lint            ok   4 projects
npx lerna run format:check    ok   4 projects
npx lerna run build           ok   4 projects
npx lerna run test            ok   3 projects — 24 tests in @dfx.swiss/react, all green

Beyond the suite, I exercised the new logic directly against the transpiled source, to confirm the behaviour rather than the assertions:

stale cross-ticket response discarded ....... true
applySupportIssueUpdate(undefined, A) ....... returns uid A   <- #215
lastSettledMessageId([-1,-2,7,3]) ........... 7
lastSettledMessageId([-1,-2]) ............... undefined
settle at index 0 by id ..................... true            <- the !idx bug
duplicate settle, ids ....................... [100,101]
retry claimed once, second click no-op ...... true / true
mergeMessages leaves input untouched ........ true
unknown id leaves list unchanged ............ [1,2]

CI is green on the same head.

Notes on the change itself

The extraction of four pure functions is the right call — it is what makes the delta anchor, the id-based settle and the duplicate guard testable at all, and the counter-probes in the description are the kind of evidence that makes a test suite worth trusting. The switch from mutate-then-shallow-copy to updater form fixes a real StrictMode double-insert, and the new updaters are idempotent under a double invocation, which is what makes that fix hold.