Publish a withheld city reply from the mail reader (#138)

[theobong]

Part of civfix/issue-tracker#138

What changed

An operator can publish a city reply the server held back, from the mail reader, after a confirmation. It then reaches the report chat or the event timeline. Admin.

Before you start

• Where: staging (admin.civfix.dev) after the main push
• Sign in as: operator
• Data: a thread linked to a report (and one linked to an event) holding a city reply with the "Withheld" pill; staging shows only the replies it already holds
• Stacked on: Mail reader shows the sender check and reply publication (#138) #35 (review only this PR's own diff; test with it merged)
• Size: 220 counted lines, 94 of them tests: the note, the confirm and the action are one flow

Verify

[Admin]

1. On the Dashboard, click the "Mail" tile header, then the "Needs attention" chip — Expect: threads with a withheld reply are listed with "Needs action".
2. Open a report thread whose city message has the "Withheld" pill — Expect: a note under the message and a "Publish reply" button. A failed or missing sender check reads "This reply failed sender authentication, so it wasn't posted to the report chat…" or "This reply couldn't be authenticated…"; a reply from another domain reads "This reply came from , which isn't a domain this thread was sent to…".
3. Look at the notes at the bottom of the thread — Expect: "Suggested: re-route this jurisdiction's contact in Jurisdictions." is not shown.
4. Click "Publish reply" — Expect: a "Publish this reply?" dialog reading "It will be posted in the report's public chat, the report moves to In progress if it's still open, and the reporter gets a notification. This can't be undone.", with "Cancel" and "Publish reply".
5. Click "Cancel" — Expect: the dialog closes; the note and the "Withheld" pill stay.
6. Click "Publish reply", then "Publish reply" in the dialog — Expect: toast "Reply published" (or "Reply approved. It will be published shortly."); the pill changes to "Published" (or "Publishing") and the note disappears.
7. Click "View report" — Expect: the report's "Chat" section shows the city's reply, and an open report is now "In progress".
8. Repeat steps 2 to 6 on an event thread — Expect: the note says "the event timeline", and the dialog reads "It will be added to the event's timeline. This can't be undone."

Regression

Needs-action thread with no withheld reply — [Admin]

1. Open a "Needs action" thread with no withheld message — Expect: "Suggested: re-route this jurisdiction's contact in Jurisdictions." still shows, with no "Publish reply" button.

Leaving a reply withheld — [Admin]

1. On a thread with a withheld reply, click "Mark replied" — Expect: toast "Marked replied"; the header pill reads "Replied"; the message keeps its "Withheld" pill and note.

Replying to the city — [Admin]

1. Type in the "Reply to …" box and click "Reply" — Expect: toast "Reply sent to "; the sent message has no note or button.

Published replies — [Admin]

1. Open a thread whose city messages are "Published" or "Publishing" — Expect: no note and no "Publish reply" button.

Not covered

• The reporter's notification after publishing arrives in the citizen app and is not checked here.
• Publishing from the Inbox's "Needs review" chip needs the unified inbox PR merged as well.

[greptile-apps]

Confidence Score: 4/5

Not safe to merge until the required styling directive is met and the publish-confirmation test is strengthened; the latter protects an irreversible operator action.

Findings

1.  Cover publish confirmation ▶
2.  Use the spacing token ▶

Fix with agent prompt

### Issue 1
apps/admin/src/features/mail/use-mail.test.ts:39-45
This test only checks the order of strings in the component source. It never renders the publish button or verifies that cancelling avoids publication and confirming sends the selected reply. The current interaction works, but this test can still pass if that irreversible behavior regresses, allowing accidental publication or preventing intended publication without test detection. Replace or supplement it with a rendered interaction test for both paths.

### Issue 2
apps/admin/src/styles/admin.css:2847
This new rule uses a raw `8px` margin even though the frontend styling directive requires design tokens. Use the existing `var(--space-2)` token instead. This repository requirement must be satisfied before merging.

```suggestion
.mail-withheld-note { margin-top: var(--space-2); }
```

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."}

[greptile-apps]

Use the spacing token

This new rule uses a raw 8px margin even though the frontend styling directive requires design tokens. Use the existing var(--space-2) token instead. This repository requirement must be satisfied before merging.

Suggested change

	.mail-withheld-note { margin-top: 8px; }
	.mail-withheld-note { margin-top: var(--space-2); }

Rule Used: # civfix review rules civfix is a live civic-tech platform that will hold government contracts. Review every PR for correctness, security and performance. Flag real defects with evidence; skip style nits that lint already covers. ## Repos - **... (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/admin/src/styles/admin.css
Line: 2847

Comment:
**Use the spacing token**

This new rule uses a raw `8px` margin even though the frontend styling directive requires design tokens. Use the existing `var(--space-2)` token instead. This repository requirement must be satisfied before merging.

```suggestion
.mail-withheld-note { margin-top: var(--space-2); }
```

**Rule Used:** # civfix review rules  civfix is a live civic-tech platform that will hold government contracts. Review every PR for **correctness, security and performance**. Flag real defects with evidence; skip style nits that lint already covers.  ## Repos  - **... ([source](https://app.greptile.com/civfix/-/custom-context?memory=39a53925-3d93-4c82-980e-27b67393717d))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Cover publish confirmation

This test only checks the order of strings in the component source. It never renders the publish button or verifies that cancelling avoids publication and confirming sends the selected reply. The current interaction works, but this test can still pass if that irreversible behavior regresses, allowing accidental publication or preventing intended publication without test detection. Replace or supplement it with a rendered interaction test for both paths.

Artifacts

Evidence from the check

• The executable harness transpiles and renders the production withheld-reply component with controlled dialog and mutation collaborators, then records both operator paths; it provides focused behavioral coverage.

Command output from the check

• Running `node trex-artifacts/withheld-reply-flow-validation.mjs` from `/home/user/repo` exited 0; cancel made no publish call and confirm published the expected thread and message IDs.

▶ Recording of the check

• The rendered component opens its irreversible publish confirmation and the operator cancels it; no publication occurs.

Poster frame after canceling withheld reply publication

• The cancel-path poster shows the rendered validation state after dismissal with an empty publish-call list; cancellation prevents publication.

▶ Recording of the check

• The rendered component accepts confirmation and invokes publication for `thread-7` and `message-9`; confirmation triggers the irreversible action.

Poster frame after confirming withheld reply publication

• The confirm-path poster shows the expected publish payload and success toast after dialog acceptance; confirmation publishes the selected reply.

_{Ran code and verified through T-Rex}

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/admin/src/features/mail/use-mail.test.ts
Line: 39-45

Comment:
**Cover publish confirmation**

This test only checks the order of strings in the component source. It never renders the publish button or verifies that cancelling avoids publication and confirming sends the selected reply. The current interaction works, but this test can still pass if that irreversible behavior regresses, allowing accidental publication or preventing intended publication without test detection. Replace or supplement it with a rendered interaction test for both paths.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Comments Outside Diff

These findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.

•  Irreversible publish confirmation is covered only by source-text order ▶

  • Bug
    • The test at apps/admin/src/features/mail/use-mail.test.ts:39-45 reads the component file and checks ordered substrings. It never mounts the publish button, opens a dialog, cancels, confirms, or observes the publish mutation. Therefore it can pass while interactive confirmation behavior is broken.
  • Cause
    • The test substitutes static source inspection for behavioral UI coverage of WithheldReplyNote.
  • Fix
    • Replace or supplement the source-order assertion with a component-level test that renders WithheldReplyNote, clicks Publish reply, verifies cancel makes no mutation call, then verifies confirm calls the mutation with the thread and message IDs.