feat(mrf): freeze a v4 snapshot before commit and send the reconstructed payload (S4)

[kevin9foong]

Problem

Everything below this PR in the stack is dormant — the snapshot store, the payload policy, reconstruction and the gates exist but nothing reaches them from a request. This PR is the activation: the minimal end-to-end v4 path (the tracer bullet the rest of PRD #9740 builds on), for the privileged plumber consumer only, on initial send.

Closes #9744.

Solution

On initial send to a plumber V4 form, freeze an immutable native-envelope snapshot before the step commits, then reconstruct the webhook payload from live row + snapshot. The v4 payload is the MRF envelope verbatim — content under the submission public key, plus the wrapped submission secret key as a read credential. No re-encryption to the form key (per PRD #9740); that is safe only because the same mrf-step-write-token flip also turns on the next-step write-guard (#9758).

S3-first ordering (atomicity by ordering, not a shared txn)

sequenceDiagram
    participant R as Respondent
    participant BE as Backend
    participant S3 as V4 snapshot bucket
    participant DB as Mongo row
    participant WH as Plumber webhook
    R->>BE: PUT next-step submission
    BE->>S3: PUT snapshot, create-if-absent
    Note over BE,S3: fresh-UUID retry on collision. A failed PUT aborts the save (500, nothing committed).
    BE->>DB: save, recording snapshotTokens.v4 on the winning step
    Note over DB: lost version race becomes 409. Loser object is a benign orphan, no wipe.
    BE-->>R: response (the snapshot object is threaded past it in memory)
    BE->>WH: reconstruct from row + in-memory snapshot, then send v4 payload
    Note over BE,WH: no S3 read-back on initial send. Retry (S5) reads by recorded token.

Loading

The ordering is the atomicity mechanism. A failed PUT aborts the save, so the respondent gets a 500 with nothing committed and a safe retry.

On create, the submission _id is generated up front so the snapshot key can be built before the row exists. On update, the step is appended only after the token is recorded, so the value survives mongoose's subdocument cast; a lost version race therefore surfaces as a 409, with the loser's object left as a harmless orphan.

Two-route parity

Whether a step got a snapshot is a storage decision, so the two plumber-V4 routes must ship the same bytes:

route	payload source	wire version
retries ON	v4 snapshot + row	4
retries OFF	live row	4

initial-send-route-parity.spec.ts pins that in CI, modulo attachment presigned URL values (minted per send) — the URL keys and the S3 object each points at must match.

A V3 row still ships verbatim via the legacy getWebhookView branch; only a V4 row goes through the policy and reconstruction.

Alternatives considered

• Commit the step first, then PUT the snapshot. Rejected — a failed PUT after commit leaves a committed step with no snapshot, breaking the guaranteed retry path and tripping the fail-loud reconstruction error. S3-first trades that for a benign orphan on an aborted txn, which is the safe direction: the token is recorded only on commit, so the orphan is never referenced and never read. Deliberately not deleted — a delete on the error path is another failure surface for no benefit.
• Re-read the snapshot from S3 on initial send. Rejected — it mostly re-verifies what a 200 from a conditional PUT already guarantees, while adding a failure surface on the happy path: a transient GET would collapse into SnapshotDataIntegrityError and drop the webhook for a submission whose snapshot was perfectly healthy. The object is threaded in memory instead, and deliberately not re-derived from the row — buildV4Snapshot is pure over row fields for v4, but the v1 form-key copy a later slice adds exists nowhere on the row, so re-deriving would reintroduce a live-row fallback through a side door.
• A pre-send payload-size guard. Rejected — an earlier revision had one. It reused a response cap as a request cap, measured the body before presigning, applied to only one of the two routes, and on trip returned success without sending: no webhook record, no retry, nothing admin-visible. An oversized POST already yields a recorded, retried, alarmable 413. A real outbound limit, if ever wanted, belongs in sendWebhook after presigning, applied to every route, with its own justification.
• Re-encrypt the v4 payload to the form key (the old Option-D shape). Rejected per PRD PRD: MRF v4 Webhooks — Option A (step-token write-guard + native-envelope v4, v1 backward-compat) #9740.

Breaking Changes

None in this PR. With mrf-step-write-token off, plumber keeps receiving today's V3 payload and no snapshot is written.

The one deliberate wire change in this stack — version describing the content format — is isolated in #9820 and needs plumber sign-off there, not here.

Tests

Requires the V4 bucket provisioned/localstack-created (#9753). Start from a 3-step workflow MRF form with a plumber webhook URL.

TC1: Plumber v4 initial send writes a snapshot and delivers the native envelope

• mrf-step-write-token ON, webhook retries ON. Submit step 1.
• Snapshot object exists at {formId}/{submissionId}/0/{token}.json and submittedSteps[0].snapshotTokens.v4 matches that token.
• Delivered payload is the native envelope: encryptedContent equals the row's, carries encryptedSubmissionSecretKey, version === 4, no step-token fields.
• Content decrypts with the submission secret key unwrapped from encryptedSubmissionSecretKey.

TC2: Flag-off parity

• mrf-step-write-token OFF, full plumber fill/advance → V3 shape, row mrfVersion 1, NO snapshot written.

TC3: Two-route parity for a V4 row

• Same step with retries ON and on an identical form with retries OFF → payloads identical field-for-field including version (4 both), modulo presigned URL values. (Gated by initial-send-route-parity.spec.ts.)

TC4: Attachments are frozen per step

• Different attachment at step 1 and step 2; retry the step-1 webhook after step 2 → attachmentDownloadUrls presigns step 1's keys.

TC5: Send gate for generic

• Generic webhook URL, enable-mrf-webhooks OFF → NO webhook fires. Turn it ON (with mrf-step-write-token also ON, which delivery requires) → fires with the Case A v4 payload.

TC6: Snapshot tokens never leave the server

• Public status tracker, admin API + export, and the delivered webhook all carry no snapshotTokens; nextStepRecipientEmails still present for the MRF metadata column.

Webhook payload content format, per case

Every case below is with the whole stack merged. X-FormSG-Signature is unchanged throughout; snapshotTokens never appears in any of them (#9817).

consumer	mrf-step-write-token	enable-mrf-webhooks	retries	row	snapshot	payload source	wire version
plumber	ON	either	ON	V4	written	snapshot + row	4
plumber	ON	either	OFF	V4	none	live row	4
plumber	OFF	either	either	V3	none	live row, legacy getWebhookView	3
generic / zapier	ON	ON	ON	V4	written	snapshot + row	4
generic / zapier	ON	ON	OFF	V4	none	live row	4
generic / zapier	ON	OFF	either	V4	none	not delivered	—
generic / zapier	OFF	either	either	V4	none	not delivered	—
no webhook	either	either	—	V4	none	not delivered	—

Case A — plumber V4 (TC1/TC3): the native envelope

Identical field-for-field on both the snapshot and no-snapshot routes, modulo presigned URL values:

{
  "data": {
    "formId": "66c0f0…",
    "submissionId": "66c0f1…",
    "created": "2026-08-03T04:15:22.180Z",
    "version": 4,
    "encryptedContent": "<V4 responses, encrypted to the submission public key>",
    "encryptedSubmissionSecretKey": "<submission secret key, wrapped to the form public key>",
    "verifiedContent": "<native verifiedContent, key omitted when absent>",
    "attachmentDownloadUrls": { "<fieldId>": "<presigned GET url, 1h expiry>" },
    "paymentContent": {},
    "workflowContent": {
      "workflow": [ /* FormWorkflowDto */ ],
      "workflowStep": 0,
      "submittedSteps": [
        {
          "isApproval": false,
          "submittedAt": "2026-08-03T04:15:22.180Z",
          "nextStepRecipientEmails": ["next@agency.gov.sg"],
          "submitterId": "S1234567D"
        }
      ]
    }
  }
}

No re-encryption to the form key: the consumer unwraps encryptedSubmissionSecretKey with the form secret key, then decrypts encryptedContent. No step-token field — includeEncryptedStepToken is computed by the policy but nothing in this stack reads it.

On the snapshot route encryptedContent, verifiedContent, encryptedSubmissionSecretKey, attachmentDownloadUrls and workflowContent.workflowStep all come from the frozen object, and submittedSteps is truncated to submissionIndex + 1. On the no-snapshot route they come from the live row and submittedSteps is untruncated — which for an initial send is the same content, since the sending step is the latest one. That equality is what initial-send-route-parity.spec.ts pins.

Case B — plumber, flag off (TC2): today's V3 payload

Same envelope, delivered by the legacy getWebhookView branch without consulting the policy:

{
  "data": {
    "version": 3,
    "encryptedContent": "<V3-shaped responses (adaptV4ToV3), same crypto>",
    "encryptedSubmissionSecretKey": "<unchanged from develop>",
    "verifiedContent": "<unchanged>",
    "attachmentDownloadUrls": { "<fieldId>": "<presigned GET url>" },
    "paymentContent": {},
    "workflowContent": { "workflow": [ /* … */ ], "workflowStep": 0, "submittedSteps": [ /* … */ ] }
  }
}

Identical to develop except version, which #9820 changes from 4 to 3 — the only wire change in the stack, and it is live independently of the flag.

Case C — generic / zapier (TC8)

Delivery needs both enable-mrf-webhooks and mrf-step-write-token. Since the payload policy keys contentFormat on mrf-step-write-token and not on consumer class, a send-eligible generic consumer receives Case A's payload — native envelope, version: 4, including encryptedSubmissionSecretKey. There is no combination in this stack that delivers a V3 payload to a generic consumer: with the flag off it is not delivered at all.

Unchanged

Non-MRF encrypt submissions are untouched: no workflowContent, no encryptedSubmissionSecretKey, version still the row's own field. Retry sends (S5) reconstruct from the recorded token and therefore reproduce Case A byte-for-byte apart from freshly minted presigned URLs.

---

Part of PRD #9740 (MRF v4 Webhooks — Option A). Top of the stack; the prerequisites are #9817 → #9818 → #9819 → #9820 → #9821. Previously a single 24-commit PR — split so each layer can be reviewed and merged on its own.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Activates the multirespondent (MRF) v4 initial webhook path by writing a v4 snapshot before committing the step, reconstructing the webhook payload from live row + snapshot, and sending that reconstructed payload (with route parity guarantees).

Changes:

• Allow initial webhook sending to accept a pre-built WebhookView (to support snapshot-reconstructed payloads).
• Write v4 snapshots S3-first pre-commit and thread the in-memory snapshot into post-submit actions for initial send reconstruction.
• Add CI coverage to assert two-route parity (snapshot-backed vs live-row) for v4 payloads (modulo presigned URL values).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
apps/backend/src/app/modules/webhook/webhook.service.ts	Accept optional pre-built webhook view for initial sends (enables snapshot-reconstructed payloads).
apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts	Implements S3-first snapshot write + v4 reconstruction for initial webhook send eligibility.
apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.controller.ts	Threads snapshot from save result into post-submission actions.
apps/backend/src/app/modules/submission/multirespondent-submission/webhook/tests/initial-send-route-parity.spec.ts	Adds parity tests ensuring snapshot-backed and live-row routes produce identical v4 payloads (modulo URL values).
apps/backend/src/app/modules/submission/multirespondent-submission/tests/multirespondent-submission.service.spec.ts	Adds integration-style unit tests for snapshot write ordering, eligibility gating, and reconstruction wiring.
apps/backend/src/app/modules/submission/multirespondent-submission/tests/multirespondent-submission.controller.spec.ts	Adds controller-level coverage for snapshot write failure mapping and aborted post-actions.

[kevin9foong]

resolved: this is intentional, since zaper is 'generic' consumer class.

[kevin9foong]

resolved: fixed

[kevin9foong]

resolved: fixed.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1191

• This ResultAsync chain is created inside a void-returning helper and is neither returned nor explicitly marked as fire-and-forget (e.g. void <promise>). If the repo enforces no-floating-promises (common in TS), this will fail lint; more importantly, it’s easy for future refactors to accidentally stop executing or observing this async work. Recommendation (mandatory if lint exists): either (1) return a ResultAsync from sendMrfInitialWebhookIfEligible and compose/await it in the caller, or (2) explicitly prefix this chain with void at the call site / in this helper to document intentional non-awaiting and satisfy floating-promise checks.

  ResultAsync.fromPromise(
    submission.getWebhookView(),
    () => new DatabaseError(),
  )
    .andThen((liveView) => {
      const policy = getWebhookPayloadPolicy({
        webhookType: webhookType === 'plumber' ? 'plumber' : 'generic',
        isStepWriteTokenEnabled,
        submissionIndex,
        submittedStepsLength: submission.submittedSteps?.length ?? 0,
      })
      return reconstructMrfWebhookData({
        liveData: liveView.data,
        snapshot,
        // Passed only when a snapshot exists; reconstruction fails loud
        // otherwise.
        submissionIndex: snapshot ? submissionIndex : undefined,
        policy,
      }).asyncAndThen((data) => {
        const webhookView: WebhookView = { data }

        return WebhookFactory.sendInitialWebhook(
          submission,
          webhookUrl,
          isRetryEnabled,
          webhookView,
        ).map(() => undefined)
      })
    })
    .mapErr((error) => {
      logger.error({
        message: 'Multirespondent submission webhook error',
        meta: logMeta,
        error,
      })
    })

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1172

• The comment is misleading: this code path also calls reconstructMrfWebhookData when snapshot is undefined (and passes submissionIndex: undefined), so it’s not accurate to say reconstruction 'fails loud otherwise'. Suggest updating the comment to the precise contract (e.g., 'When snapshot is provided, submissionIndex is required; otherwise reconstruction uses live data only').

        snapshot,
        // Passed only when a snapshot exists; reconstruction fails loud
        // otherwise.
        submissionIndex: snapshot ? submissionIndex : undefined,

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1174

• The inline comment is inconsistent with the behavior in this function: the code explicitly supports the no-snapshot route (it passes snapshot as undefined and omits submissionIndex). Update the comment to reflect the intended contract (e.g., reconstruction should fall back to live-row when snapshot is absent, and only requires submissionIndex when a snapshot is present), or adjust the call if the function truly must fail without a snapshot.

      return reconstructMrfWebhookData({
        liveData: liveView.data,
        snapshot,
        // Passed only when a snapshot exists; reconstruction fails loud
        // otherwise.
        submissionIndex: snapshot ? submissionIndex : undefined,
        policy,
      }).asyncAndThen((data) => {

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1135

• This helper is used from both create and update post-submission actions, but it always logs "Sending initial webhook…". This makes production logs ambiguous and can break log-based dashboards/alerts that previously distinguished create vs update. Consider renaming the helper and/or parametrizing the log message (e.g., "create" vs "update") so observability remains accurate.

  logger.info({
    message: 'Sending initial webhook for multirespondent submission',
    meta: { ...logMeta, webhookType },
  })

apps/backend/src/app/modules/submission/multirespondent-submission/tests/multirespondent-submission.service.spec.ts:67

• This mock replaces writeV4Snapshot/readV4Snapshot with bare jest.fn() across the entire test file, but default return values are only configured inside the new nested describe block. If any other existing tests in this file hit a snapshot-enabled path, they may fail at runtime because writeV4Snapshot() returns undefined (not a ResultAsync). To make the suite robust, set safe defaults in a top-level beforeEach (or in the mock factory) so these mocks always return a valid ResultAsync unless a test overrides them.

jest.mock('../webhook/submission-snapshot.store', () => {
  const actual = jest.requireActual('../webhook/submission-snapshot.store')
  return {
    ...actual,
    writeV4Snapshot: jest.fn(),
    readV4Snapshot: jest.fn(),
  }
})

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1135

• This log line is emitted from both create and update post-submission flows (since the same helper is called in both places), but the message always says 'initial webhook'. That will make production debugging ambiguous. Consider changing the message to a neutral 'Sending webhook for multirespondent submission', or pass an explicit context (e.g. 'create' | 'update') into sendMrfInitialWebhookIfEligible and include it in the log meta/message.

  logger.info({
    message: 'Sending initial webhook for multirespondent submission',
    meta: { ...logMeta, webhookType },
  })

apps/backend/src/app/modules/submission/multirespondent-submission/tests/multirespondent-submission.service.spec.ts:3771

• The growthbookWith helper returns the same isOn value for every flag, which can accidentally enable/disable unrelated flags (e.g. mrfStepWriteToken) and make future test changes brittle as more flags are consulted. Prefer using the more explicit growthbookWithFlags({ enableMrfWebhooks, mrfStepWriteToken }) everywhere, or update growthbookWith to only affect the intended flag(s) and default others to false.

    const growthbookWith = (enableMrfWebhooks: boolean) =>
      ({
        isOn: jest.fn().mockReturnValue(enableMrfWebhooks),
        getFeatureValue: jest.fn((_flag: string, def: unknown) => def),
      }) as any

apps/backend/src/app/modules/webhook/webhook.service.ts:255

• Adding an optional positional parameter here makes call sites easier to misread and harder to extend safely (especially if more optional args are added). Consider switching this function (and its callers) to accept a single options object (e.g. { submission, webhookUrl, isRetryEnabled, webhookView }) to make the API self-documenting and reduce the risk of argument-order mistakes.

    submission: IEncryptedSubmissionSchema | IMultirespondentSubmissionSchema,
    webhookUrl: string,
    isRetryEnabled: boolean,
    webhookView?: WebhookView,

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1169

• Typo in comment: 'RATONALE' should be 'RATIONALE'.

      return reconstructMrfWebhookData({
        liveData: liveView.data,
        snapshot,
        // RATONALE: if snapshot does not exist, we use the live row.
        submissionIndex: snapshot ? submissionIndex : undefined,
        policy,

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1170

• Overloading submissionIndex to mean both 'which step to reconstruct' and 'whether to reconstruct at all' (by passing undefined) makes the call-site harder to reason about and couples behavior to reconstructMrfWebhookData’s handling of undefined. A clearer approach is to always pass the actual submissionIndex, and let reconstruction choose snapshot-vs-live-row based on snapshot presence (or an explicit boolean like useSnapshot).

      return reconstructMrfWebhookData({
        liveData: liveView.data,
        snapshot,
        // RATONALE: if snapshot does not exist, we use the live row.
        submissionIndex: snapshot ? submissionIndex : undefined,
        policy,
      }).asyncAndThen((data) => {

apps/backend/src/app/modules/submission/multirespondent-submission/tests/multirespondent-submission.service.spec.ts:3865

• This mocked WebhookView shape looks inconsistent with the PR’s stated wire contract (e.g., v4 sends asserting version === 4 and created typically being serialized). Using a mock that more closely matches getWebhookView() output (or deriving the view from a real model instance, as the parity test does) would make these reconstruction/send-path tests more representative and reduce the risk of false confidence.

    const buildLiveWebhookView = (): WebhookView =>
      ({
        data: {
          formId: mockFormId,
          submissionId: 'live-submission-id',
          encryptedContent: 'live-content',
          encryptedSubmissionSecretKey: 'live-read-key',
          verifiedContent: 'live-verified',
          version: 2,
          created: new Date(),

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1169

• Typo in comment: 'RATONALE' should be 'RATIONALE'.

      return reconstructMrfWebhookData({
        liveData: liveView.data,
        snapshot,
        // RATONALE: if snapshot does not exist, we use the live row.
        submissionIndex: snapshot ? submissionIndex : undefined,
        policy,

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:892

• Snapshot-build + write + token-recording logic is now duplicated across create and update flows. This is easy to accidentally diverge (e.g. attachmentMetadata conversion, createdAt source, token recording timing). Consider extracting a shared helper that (a) builds the v4 snapshot, (b) writes it, and (c) returns { snapshot, token } (or directly mutates the step meta), so both code paths stay consistent and future changes only need to be made once.

      const snapshot = shouldWriteSnapshot
        ? buildV4Snapshot({
            formId: String(form._id),
            submissionId: String(submissionObjectId),
            submissionIndex: 0,
            workflowStep: 0,
            encryptedContent,
            encryptedSubmissionSecretKey,
            verifiedContent,
            attachmentMetadata: Object.fromEntries(
              attachmentMetadata ?? new Map(),
            ),
            createdAt: submittedStepMeta.submittedAt,
          })
        : undefined

      const writeSnapshotIfNeeded: ResultAsync<undefined, SnapshotWriteError> =
        snapshot
          ? writeV4Snapshot(snapshot).map(({ token }) => {
              submittedStepMeta.snapshotTokens = { v4: token }
              return undefined
            })
          : okAsync(undefined)

[kevin9foong]

TC1:

webhook payload
"submittedSteps": [
backend-1 | {
backend-1 | "isApproval": false,
backend-1 | "submittedAt": "2026-08-04T15:10:50.344Z",
backend-1 | "nextStepRecipientEmails": [
backend-1 | ""
backend-1 | ]
backend-1 | }
backend-1 | ]
the snapshotToken is stripped.

The snapshots are created, including for subsequent steps

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

apps/backend/src/app/modules/webhook/webhook.service.ts:275

• The const webhookViewToUse = ... statement is missing a terminating semicolon; in codebases that enforce semicolons via linting/formatting this can fail CI. Add a semicolon after the conditional expression assignment.

    const webhookViewToUse = webhookView
      ? okAsync(webhookView)
      : ResultAsync.fromPromise(
          submission.getWebhookView(),
          () => new DatabaseError(),
        )

    return webhookViewToUse.andThen((webhookView) =>

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1170

• Typo in comment: 'RATONALE' should be 'RATIONALE'.

      return reconstructMrfWebhookData({
        liveData: liveView.data,
        snapshot,
        // RATONALE: if snapshot does not exist, we use the live row.
        submissionIndex: snapshot ? submissionIndex : undefined,
        policy,
      }).asyncAndThen((data) => {

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:892

• The snapshot build + writeV4Snapshot + snapshotTokens assignment pattern is duplicated in both create and update flows. Consider extracting a small helper that (a) builds the v4 snapshot object and (b) returns a ResultAsync that writes it and applies the token to the step metadata. This reduces duplication and helps ensure future changes (e.g., snapshot schema evolution or logging) remain consistent between create/update.

      const snapshot = shouldWriteSnapshot
        ? buildV4Snapshot({
            formId: String(form._id),
            submissionId: String(submissionObjectId),
            submissionIndex: 0,
            workflowStep: 0,
            encryptedContent,
            encryptedSubmissionSecretKey,
            verifiedContent,
            attachmentMetadata: Object.fromEntries(
              attachmentMetadata ?? new Map(),
            ),
            createdAt: submittedStepMeta.submittedAt,
          })
        : undefined

      const writeSnapshotIfNeeded: ResultAsync<undefined, SnapshotWriteError> =
        snapshot
          ? writeV4Snapshot(snapshot).map(({ token }) => {
              submittedStepMeta.snapshotTokens = { v4: token }
              return undefined
            })
          : okAsync(undefined)

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (4)

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1168

• Typo in comment: 'RATONALE' should be 'RATIONALE'.

      return reconstructMrfWebhookData({
        liveData: liveView.data,
        snapshot,
        // RATONALE: if snapshot does not exist, we use the live row.
        submissionIndex: snapshot ? submissionIndex : undefined,

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1108

• This helper is invoked from both post-create and post-update flows, but its name implies create-only semantics ('Initial'). Consider renaming to something lifecycle-agnostic (e.g., sendMrfWebhookIfEligible / sendMrfStepWebhookIfEligible) to avoid misleading call sites and future misuse.

const sendMrfInitialWebhookIfEligible = ({
  submission,
  snapshot,
  webhookUrl,
  isRetryEnabled,
  growthbook,
  logMeta,
}: {

apps/backend/src/app/modules/submission/multirespondent-submission/multirespondent-submission.service.ts:1133

• This log message will be emitted for update sends too (since the same helper is used in both post-create and post-update actions). Please make the message accurately reflect the operation (e.g., include whether it's create vs update or just say 'Sending webhook...') to keep observability and incident triage unambiguous.

  logger.info({
    message: 'Sending initial webhook for multirespondent submission',
    meta: { ...logMeta, webhookType },
  })

apps/backend/src/app/modules/submission/multirespondent-submission/tests/multirespondent-submission.service.spec.ts:4056

• PR description states that with mrf-step-write-token off, plumber should keep receiving V3 and no snapshot should be written. This test (and the implied shouldWriteV4Snapshot/send gating behavior) asserts the opposite for plumber. Please align either (a) the implementation/tests to the stated flag behavior, or (b) update the PR description to reflect the actual plumber gating rules.

    it('still writes a snapshot for a plumber V4 row with no flags on', async () => {
      const result = await createMultiRespondentFormSubmission({
        form: buildV4Form(),
        encryptedPayload: buildV4Payload(),
        logMeta: { action: 'test' },
        growthbook: growthbookWithFlags({}),
      })

      expect(result.isOk()).toBe(true)
      expect(MockSnapshotStore.writeV4Snapshot).toHaveBeenCalledTimes(1)
    })

[eliotlim]

LGTM! 👍 to be merged as a stack.