feat: include Form ID as first entry of admin notification JSON

[tishttash]

Problem

Many form admins built automations on email subject lines or body content; the email-standardisation work broke those integrations. As admins move to the machine-parseable JSON snippet in the admin notification email, that JSON needs to say which form a response came from — today it starts at "Response ID" and carries no form identifier. The Form ID is the stable, globally-unique key automations should route on (form names are mutable and non-unique). Ref: Zendesk ticket 62023; PRD "JSON improvements".

Solution

Prepend { "question": "Form ID", "answer": <form._id as string> } as the first entry of the admin-notification response JSON. The JSON is assembled in two independent builders, and both are covered: sendSubmissionToAdmin in mail.service.ts for regular forms (one array feeds both the standardised and legacy templates), and buildMrfResponseJson for MRF forms (a single call site feeds both the workflow-completion and approval emails, via a new formId param). The branch's first commit added a "Form name" entry; the follow-up replaces it with Form ID only, so the net change against develop is the Form ID entry alone.

Before vs after — the JSON block in the admin notification email

Before	After
[ {"question":"Response ID", "answer":"6a41e30480938b7dd6c4ea64"}, {"question":"Timestamp", "answer":"Mon, 29 Jun 2026 11:14:25 AM"}, {"question":"Email", "answer":"tasha@open.gov.sg"}, {"question":"Short answer", "answer":""} ]	[ {"question":"Form ID", "answer":"64f8a1b2c3d4e5f60718293a"}, {"question":"Response ID", "answer":"6a41e30480938b7dd6c4ea64"}, {"question":"Timestamp", "answer":"Mon, 29 Jun 2026 11:14:25 AM"}, {"question":"Email", "answer":"tasha@open.gov.sg"}, {"question":"Short answer", "answer":""} ]

The same shape applies to regular-form notifications and MRF workflow-completion / approval emails.

Alternatives considered

• We considered including the form name alongside the Form ID, but dropped it — the ID alone answers "which form did this come from?", names are mutable and non-unique so automations must not route on them, and the "form name" vs "form title" naming convention (frontend vs backend/DB) is unresolved and not being pursued here.
• We considered putting Form ID after a form-name label (label-then-id grouping), but Form ID leads the payload because it is the identifier automations key on, matching the existing "Response ID" style.
• We considered consolidating the two JSON builders into one shared assembler, but rejected it for this change as a larger, out-of-scope refactor — the duplication is recorded (see inline comment on buildMrfResponseJson) so it is at least visible.
• CSV export is deliberately excluded: admins haven't asked for it, and a new column would make a form's older exports inconsistent.

Breaking Changes

No - backwards compatible (a new entry prepended to the response JSON array; existing entries and shape unchanged).

Tests

TC1: Regular form — admin notification

• Submit a response to a regular (storage-mode) form with email notifications on.
• In the admin notification email, confirm the -- Start of JSON -- block's first entry is {"question":"Form ID","answer":"<form id>"}, before "Response ID" and "Timestamp", and the ID matches the form's dashboard URL.

TC2: MRF — workflow completion

• Complete all steps of a multi-respondent form workflow.
• In the workflow-completion email, confirm the response JSON's first entry is the same Form ID entry.

TC3: MRF — approval outcome

• Complete an MRF workflow with an approval step (approve or reject).
• In the approval-outcome email, confirm the response JSON's first entry is the same Form ID entry.

TC4: No leakage into CSV

• Export responses to CSV for a form that received a post-change submission; confirm no new "Form ID" column appears.

---

🤖 Generated with Claude Code

[tishttash]

The standardised template embeds the response JSON as a text node (<p>{responseJson}</p>), so react-email escapes the JSON's double quotes to " in the rendered HTML. The test therefore builds the expected JSON with JSON.stringify(...) and then .replace(/"/g, '"') to match what actually lands in the email. Asserting on the rendered HTML (rather than reaching into the private #sendEmailWithTemplate args) keeps the test on observable output, at the cost of encoding this escaping detail.

[tishttash]

Gotcha for future changes: the admin-notification email JSON is assembled in two independent places that must be kept in sync — fullDataCollationData in mail.service.ts sendSubmissionToAdmin (regular forms; feeds both the standardised and legacy templates) and buildMrfResponseJson here (single call site; feeds both the workflow-completion and approval emails). Any field added to the notification JSON must land in both builders or MRF diverges — the original plan assumed one insertion point and silently missed MRF. MRF workflow-step and respondent-copy emails carry only formQuestionAnswers, not a JSON block, so they are correctly untouched.

[tishttash]

Review findings — Form ID in admin notification JSON

Multi-axis review of git diff develop...HEAD on branch form-name-json (commits d05b371af, 36a039cc0, a7117c7e2), run 2026-07-08. Four axes reviewed in parallel; correctness findings were independently verified and dropped below a confidence threshold of 70. Three findings were dropped by that filter (noted per axis) — the filter ran; the axes were not silent.

Standards

Clean — no surviving findings. The diff aligns with CONTEXT.md's glossary ("Form ID is the identifier automations should route on"; "Form name is for human reading only"), and both known builders of the notification JSON were updated consistently.

Dropped below threshold after verification:

• Comment placed inside an array literal rather than above the statement it explains (mail.service.spec.ts:465) — verified real but stylistic, no documented standard names it. Score 50.
• One test asserts via sendMailSpy.mock.calls[0][0].html as string + toContain instead of the sibling toHaveBeenCalledWith(expect.objectContaining(...)) pattern (mail.service.spec.ts:474) — style divergence real, but the "avoid any" claim didn't hold (as string is not any, and the lint rule is disabled in tests anyway). Score 35.

Spec

Clean — all acceptance criteria genuinely satisfied. Verified against .scratch/JSON improvements/ (handoff + issue-1 + PRD): Form ID is the first entry in both builders with the exact label and String(form._id) value; the single MRF call site feeds both the completion and approval emails; the regular-form array feeds both the standardised and legacy templates; form name is fully removed; CSV is untouched; tests cover both paths.

Dropped below threshold after verification:

• The former single "Response ID and Timestamp" test was split into two tests rather than just re-indexed — restructuring the spec didn't ask for, but harmless. Score 50.

Architecture

One non-blocking finding.

• The metadata-prefix contract is duplicated across the two builders. The ordered prefix (Form ID / Response ID / Timestamp, exact labels) is an interface contract downstream automations key on, now encoded verbatim in multirespondent-submission.utils.ts:593 and mail.service.ts:828-841, with hand-synced rationale comments. The risk is demonstrated, not hypothetical: this branch's own history shows the MRF builder being silently missed at first. Suggested deepening: extract just the prefix into a small shared buildAdminNotificationMetadata({ formId, responseId, timestamp }) used by both builders, so the next added field cannot half-land. Smaller than the deliberately-deferred full builder consolidation; fine as a follow-up. Non-blocking.

Divergent

Confirmed as the best available option: putting the ID in the JSON body (email headers are invisible to JSON-consuming automations; titles are unstable), passing formId: string rather than the form object (matches the builder's primitives-only shape), and — in explicit disagreement with the Architecture finding above — not extracting a shared metadata helper in this PR, since the prefix is the natural first piece of the deferred builder consolidation and extracting it piecemeal would half-do that work. That disagreement is a judgement call for the team.

One minor finding.

• Positional test assertions absorb all the churn of every metadata insertion (multirespondent-submission.utils.spec.ts:1099 and ~9 siblings). This diff's dozen result[2] → result[3] edits are the second index shift on this branch. A const METADATA_COUNT = 3 offset (or asserting on result.slice(METADATA_COUNT)) would keep the ordering coverage while making the next metadata change a one-line test edit. Maintainability only. Minor.

Summary

Standards 0, Spec 0, Architecture 1 (non-blocking), Divergent 1 (minor) surviving findings; 3 dropped below threshold. Worst issue: the duplicated metadata-prefix contract. Nothing blocks merging.

[endor-labs-pro]

Warning

Endor Labs detected 1 policy violations associated with this pull request.

Please review the findings that caused the policy violations.

📋 Policy: PR Warn (7 findings)

📥 Package npm://formsg-backend@7.34.1

⤵️ Dependency: npm://form-data@4.0.5

🚩 GHSA-hmw2-7cc7-3qxx: form-data: CRLF injection in form-data via unescaped multipart field names and filenames

Details

• Severity: High
• Tags: Transitive Normal Reachable Dependency Reachable Function Fix Available Warning
• Categories: SCA Vulnerability Security
• Remediation: Upgrade form-data to version 4.0.6 (current: 4.0.5, latest: 4.0.6).

⤵️ Dependency: npm://multer@2.1.1

🚩 GHSA-72gw-mp4g-v24j: Multer vulnerable to Denial of Service via deeply nested field names

Details

• Severity: High
• Tags: Direct Normal Reachable Dependency Reachable Function Fix Available Warning
• Categories: SCA Vulnerability Security
• Remediation: Update formsg-backend@7.34.1 to use multer version 2.2.0 (current: 2.1.1, latest: 3.0.0-alpha.2).

⤵️ Dependency: npm://multiparty@4.2.3

🚩 GHSA-xh3c-6gcq-g4rv: multiparty vulnerable to Denial of Service via Uncaught Exception in filename* parameter parsing

Details

• Severity: High
• Tags: Direct Normal Reachable Dependency Reachable Function Fix Available Warning
• Categories: SCA Vulnerability Security
• Remediation: Update formsg-backend@7.34.1 to use multiparty version 4.3.0 (current: 4.2.3, latest: 4.3.0).

🚩 GHSA-65x3-rw7q-gx94: multiparty vulnerable to ReDoS via filename parsing

Details

• Severity: High
• Tags: Direct Normal Reachable Dependency Reachable Function Fix Available Warning
• Categories: SCA Vulnerability Security
• Remediation: Update formsg-backend@7.34.1 to use multiparty version 4.3.0 (current: 4.2.3, latest: 4.3.0).

🚩 GHSA-qxch-whhj-8956: multiparty: Denial of Service via Prototype Pollution leads to Uncaught Exception

Details

• Severity: High
• Tags: Direct Normal Reachable Dependency Reachable Function Fix Available Warning
• Categories: SCA Vulnerability Security
• Remediation: Update formsg-backend@7.34.1 to use multiparty version 4.3.0 (current: 4.2.3, latest: 4.3.0).

⤵️ Dependency: npm://nodemailer@8.0.7

🚩 GHSA-p6gq-j5cr-w38f: Nodemailer: Message-level raw option bypasses disableFileAccess/disableUrlAccess, enabling arbitrary file read and full-response SSRF in the delivered message

Details

• Severity: High
• Tags: Direct Normal Reachable Dependency Reachable Function Fix Available Warning
• Categories: SCA Vulnerability Security
• Remediation: Upgrade nodemailer to version 9.0.1 (current: 8.0.7, latest: 9.0.3).

📥 Package npm://formsg-virus-scanner-guardduty@7.34.1

⤵️ Dependency: npm://uuid@9.0.1

🚩 GHSA-w5hq-g745-h8pq: uuid: Missing buffer bounds check in v3/v5/v6 when buf is provided

Details

• Severity: High
• Tags: Direct Normal Reachable Dependency Reachable Function Fix Available Warning
• Categories: SCA Vulnerability Security
• Remediation: Upgrade uuid to version 11.1.1 (current: 9.0.1, latest: 14.0.1).

---

This comment was automatically generated by Endor Labs.
Scanned @ 07-08-2026 07:36:08 UTC