Repository navigation
Description
PATCH /api/assignments/:id stores a series item's answers by appending them to encryptedData, without being told which item they answer and without checking that this is the next unanswered one. Position is everything downstream. GatewaySynchronizer pairs the nth ciphertext with the nth series item, and the root router resumes a returning patient at "number of stored entries". So any submission for an item that has already been answered corrupts the assignment. The easiest way to send one is to open the link twice, which happens readily: clicking the email link again, or restoring tabs. A tab left behind on an earlier item submits it again, and the gateway files it under the next position. Once that happens the patient cannot reopen the link (the server render throws and returns a 500), or the finished series is rejected by the API on every pass and never imported. #1668 fixes the same symptom for a double click inside one tab on the client; it cannot stop a second tab, because only the gateway sees both submissions.
Where
apps/gateway/src/routers/api.router.ts:125-138. A series submission is appended unconditionally:
let encryptedData: string, symmetricKey: string;
if (kind === 'SCALAR') {
// ...
} else {
encryptedData = (assignment.encryptedData ?? '').concat(
'$',
Buffer.from(encryptResult.cipherText).toString('base64')
);
symmetricKey = (assignment.symmetricKey ?? '').concat(
'$',
Buffer.from(encryptResult.symmetricKey).toString('base64')
);
}apps/gateway/src/routers/root.router.ts:39-42. The resume position is the entry count:
let initialSeriesIndex: number | undefined;
if (assignment.encryptedData?.startsWith('$')) {
initialSeriesIndex = assignment.encryptedData.slice(1).split('$').length;
}packages/react-core/src/components/InstrumentRenderer/SeriesInstrumentRenderer.tsx:52-61 throws when that count reaches the number of items. Nothing in the PATCH body says which item is being answered: $UpdateRemoteAssignmentData is { data, kind, status }, and the series submit context's index is the renderer's step (0/1/2), not the item.
Reproduce
- Assign the built-in
DNP_HAPPINESS_QUESTIONNAIRE_WITH_CONSENTseries (items:DNP_GENERAL_CONSENT_FORM, thenDNP_HAPPINESS_QUESTIONNAIRE) and open its link in two tabs. Verify and click Begin in both. - In tab A, give consent and submit. Tab A moves on to the happiness questionnaire.
- In tab B, which still shows the consent form, submit it.
- Reload either tab.
- Alternatively, skip step 4 and finish the happiness questionnaire in tab A, then wait one sync interval.
Actual: the gateway now holds two entries for a two-item series, both of them consent answers. Step 4 computes initialSeriesIndex = 2, SeriesInstrumentRenderer throws "Initial series index '2' must be less than length of items '2'" during the server render, and the patient gets {"message":"Internal Server Error","statusCode":500} from then on. The assignment never completes. In step 5 the series is marked COMPLETE with three entries, and GatewaySynchronizer throws "Expected number of submitted series items '3' to be between 1 and the series instrument content length '2'" on every pass, so it is never imported, while the patient was shown "Thank You!".
Expected: the second submission of the consent form is refused (for example with 409), and that tab reloads to the server's position, so the stored entries always line up one-to-one with the series items.
Tests
apps/gateway/src/routers/__tests__/api.router.test.ts:it('should reject a series item that is not the next unanswered one, so a second tab cannot shift later answers')andit('should reject any submission once the assignment is complete').apps/gateway/src/__tests__/Root.test.tsx:it('should send the position of the series item being submitted').- e2e,
testing/src/specs/gateway-assignment.spec.ts: like the existing malformed-submission test, verify once in the page and then send twoPATCHes for item 0 with the page's token. Assert that the second is refused and that reloading the link resumes at item 1.
Suggested fix
Add the item's position (currentItemIndex) to the series submit context in react-core and to $UpdateRemoteAssignmentData as, for example, itemIndex. In the PATCH handler, compute the stored entry count, reject a SERIES submission whose itemIndex differs with 409, and reject any submission once completedAt is set. On 409, have Root.tsx reload the page so it resumes from the server's count.
Activity
joshunrau commentedon Oct 26, 2026
Still reproduces on
main(d86740e). None of the cited lines has changed since this was filed, and no open PR touches them.The same series also fails in a way the bound check does not catch.
apps/api/src/gateway/gateway.synchronizer.ts:139only bounds the number of entries and never checks positions. If tab B declines consent at step 3,terminateends the series, and the row becomesCOMPLETEwith two entries for two items. B's consent answer is then validated asDNP_HAPPINESS_QUESTIONNAIREand fails with 422 (instrument-records.service.ts:113-121) on every pass. If the next item's schema happens to accept the shifted answer, it is imported under that item with no error.Changes to the suggested fix
indexinstead of adding a field.SeriesContext.index(packages/react-core/src/components/InstrumentRenderer/types.ts:29) is always1whenonSubmitruns, because item content only renders underindex: 1(SeriesInstrumentRenderer.tsx:186). Nothing reads it. Replace it withitemIndex: currentItemIndex.Root.tsx:60-63spreads...resultinto the body, so the field is sent without touchingupdateData. Updateapps/gateway/src/__tests__/Root.test.tsx:48andapps/web/src/routes/_app/instruments/render/__tests__/$id.test.tsx:188, which setindex: 0, andapps/gateway/src/routers/__tests__/api.router.test.ts:263and:274, which send series PATCHes with no position.$UpdateRemoteAssignmentDataa discriminated union onkind, with both variants extended from one base. ThenitemIndexis required onSERIESand does not exist onSCALAR.root.router.ts:39-42into a helper that the router and the PATCH guard both call.api.router.ts:106and writes it at:140, with several awaits in between. UseupdateManywhere{ id, encryptedData: <value read>, completedAt: null }, and return 409 whencount === 0. Otherwise two requests that read the same row both get 200, and the later write silently replaces the earlier one.itemIndex >= items.length, and setCOMPLETEwhenitemIndexis the last item, readingitemsfromtargetStringified. Otherwise a hand-written last-item PATCH withoutstatusfills every slot whilecompletedAtstays null, and the next load of the link is a 500.verifiedfalse (Root.tsx:40,:87), so the patient has to solve the proof of work again, although the server still holds the id as verified. Once Gateway lets a patient reload or leave mid-instrument without warning, discarding every unsaved answer #1726 lands,location.reload()also raises "Leave site?". Instead, return the stored count in the 409 body and remount the renderer at that count, with at({ en, es, fr })notice. The remount needs akeyas well asinitialSeriesIndex, becauseinitialSeriesIndexis only read by auseStateinitialiser (SeriesInstrumentRenderer.tsx:52).handleSubmitmust still throw. If it returns,createSubmitHandleradvances as though the item was saved.codetoHttpExceptionand to theerrorHandlerMiddlewarebody, as an enum in@opendatacapture/schemas/gateway. Add a per-request option toapps/gateway/src/services/axios.tsthat names the codes the caller handles, so the generic toast is skipped only for those. Web'smeta.disableDefaultErrorNotification(apps/web/src/services/axios.ts:18,:163) is the all-or-nothing version of this. After a gateway restart, a patient's submit fails with "HTTP Request Failed 403" and the only way to re-verify discards their answers #1758 needs both. Updateapps/gateway/AGENTS.md:54("ThrowHttpException(status, message)") in the same PR.Gotchas
completedAtguard is needed today, not only after After a gateway restart, a patient's submit fails with "HTTP Request Failed 403" and the only way to re-verify discards their answers #1758. COMPLETE clears verification (api.router.ts:151-153), but/api/auth/verifymarks any id (cap.router.ts:62). So a second tab opened before the first one finished can solve the Cap afterwards and is verified again. Its PATCH then adds a third entry to a finished series, or replaces a scalar's answers before the next sync.gateway malformed submissione2e sendskind: 'SERIES'with no position to a FORM assignment and expects 200 (testing/src/specs/gateway-assignment.spec.ts:162-166). AdditemIndex: 0to it. If the handler also refuses aSERIESsubmission to a scalar target, that test has to expect 400 instead.Tests
In addition to the tests listed in the issue:
api.router.test.ts: 409 when the conditional write matches no row,itemIndex >= items.lengthis refused, and the last item completes withoutstatus.packages/schemas/src/assignment/__tests__/assignment.test.ts: a series update withoutitemIndexis rejected.SeriesInstrumentRenderer.test.tsx:itemIndexcounts frominitialSeriesIndex.Root.test.tsx: a 409 remounts the renderer at the returned count and shows the notice, and the page does not reload.apps/gateway/src/services/__tests__/axios.test.ts: a handled code raises no toast, and other errors still do.DNP_HAPPINESS_QUESTIONNAIRE_WITH_CONSENT, because its two items look different. Look it up by title, sincefindInstrumentIdByName(testing/src/support/api-client.ts:185) only matches a scalar'sinternal.name. Verify once and send one PATCH for item 0 with the page's token. Then click Begin and submit consent in the page. Assert that after the next Begin the page shows the Happiness Questionnaire without a second verification, and that after a reload__ROOT_PROPS__hasinitialSeriesIndex: 1. A single Cap solve covers both the server's refusal and the page's recovery.Order
Land this after #1668, which guards the same
createSubmitHandlerand edits the sameonSubmit({...})call (SeriesInstrumentRenderer.tsx:99-106). Land it before #1758 or with it. #1703 adds its expiry refusal to the same PATCH guard.Triage note written with Claude Code. Code references point at
main@d86740e.