Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion apps/admin/src/features/mail/mail-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
} from "@/features/inbox/inbox-feed"
import { AuthVerdictBadge, PublicationBadge } from "@/features/mail/mail-badges"
import { MAIL_STATUS_CLS } from "@/features/mail/mail-presentation"
import { WithheldReplyNote } from "@/features/mail/withheld-reply-note"
import { useNav, useToast } from "@/store/ui-store"
import { toAppError } from "@/lib/api"
import { errorMessage } from "@/lib/error-messages"
Expand Down Expand Up @@ -264,6 +265,7 @@ function MailReader({ threadId, eventId = null }: { threadId: string; eventId?:

const who = correspondent(sel)
const whoLabel = sel.org || who
const hasWithheld = sel.messages.some((m) => m.publication === "withheld")

const sendReply = () => {
const body = text.trim()
Expand Down Expand Up @@ -391,6 +393,9 @@ function MailReader({ threadId, eventId = null }: { threadId: string; eventId?:
)}
</div>
<p className="mail-msg-body">{msg.body}</p>
{msg.publication === "withheld" && (
<WithheldReplyNote threadId={sel.id} msg={msg} isReport={sel.reportId !== null} />
)}
{msg.truncated && (
<div className="hint">This message was cut at 64 KB for display.</div>
)}
Expand Down Expand Up @@ -424,7 +429,7 @@ function MailReader({ threadId, eventId = null }: { threadId: string; eventId?:
reporting form.
</div>
)}
{sel.dir === "in" && sel.status === "needs_action" && (
{sel.dir === "in" && sel.status === "needs_action" && !hasWithheld && (
<div className="mail-action-note">
<Icons.CornerArr size={14} />
Suggested: re-route this jurisdiction&apos;s contact in Jurisdictions.
Expand Down
48 changes: 48 additions & 0 deletions apps/admin/src/features/mail/mail-presentation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import {
AUTH_VERDICT_VIEW,
MAIL_STATUS_CLS,
PUBLICATION_CLS,
PUBLISH_TOAST,
domainOfAddress,
publicationTitle,
publishConfirmBody,
withheldNote,
withheldReason,
} from "./mail-presentation"

describe("mail badges", () => {
Expand Down Expand Up @@ -42,3 +47,46 @@ describe("mail badges", () => {
expect(publicationTitle("pending", false)).toMatch(/posted shortly/)
})
})

describe("withheld reply review", () => {
it("treats a failed, missing or unrecorded sender check as a possible forgery", () => {
expect(withheldReason("fail")).toBe("auth")
expect(withheldReason("unknown")).toBe("auth")
expect(withheldReason("pass")).toBe("domain")
expect(withheldReason(null)).toBe("auth")
expect(withheldReason(undefined)).toBe("auth")
})

it("explains why the reply was held back and where publishing sends it", () => {
expect(withheldNote({ authVerdict: "fail", from: "x@city.gov" }, true)).toMatch(
/^This reply failed sender authentication, so it wasn't posted to the report chat\. It could be forged/,
)
expect(withheldNote({ authVerdict: "unknown", from: "x@city.gov" }, false)).toMatch(
/^This reply couldn't be authenticated, so it wasn't posted to the event timeline\./,
)
expect(withheldNote({ authVerdict: "pass", from: "clerk@vendor.example" }, true)).toMatch(
/^This reply came from vendor\.example, which isn't a domain this thread was sent to, so it wasn't posted to the report chat\./,
)
expect(withheldNote({ authVerdict: null, from: "" }, false)).toMatch(
/^This reply couldn't be authenticated, so it wasn't posted to the event timeline\./,
)
expect(withheldNote({ authVerdict: "pass", from: "" }, false)).toMatch(/^This reply came from an unknown sender,/)
})

it("shows the sender's domain, never the whole address", () => {
expect(domainOfAddress("clerk@city.gov")).toBe("city.gov")
expect(withheldNote({ authVerdict: "pass", from: "clerk@city.gov" }, true)).not.toMatch(/clerk@/)
})

it("warns what publishing does before it happens", () => {
expect(publishConfirmBody(true)).toMatch(/report's public chat.*reporter gets a notification/)
expect(publishConfirmBody(false)).toMatch(/event's timeline/)
})

it("has a toast for every publish outcome", () => {
for (const publication of MailReplyPublicationSchema.options) {
expect(PUBLISH_TOAST[publication]).toBeTruthy()
}
expect(PUBLISH_TOAST.published).toBe("Reply published")
})
})
34 changes: 34 additions & 0 deletions apps/admin/src/features/mail/mail-presentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,37 @@ export function publicationTitle(publication: MailReplyPublication, isReport: bo
if (publication === "pending") return "Approved. It will be posted shortly."
return isReport ? "Posted to the report chat and timeline." : "Added to the event timeline."
}

export function withheldReason(verdict: MailAuthVerdict | null | undefined): "auth" | "domain" {
return verdict === "pass" ? "domain" : "auth"
}

export function domainOfAddress(address: string): string {
return address.slice(address.lastIndexOf("@") + 1)
}

export function withheldNote(
msg: { authVerdict?: MailAuthVerdict | null; from: string },
isReport: boolean,
): string {
const target = isReport ? "the report chat" : "the event timeline"
if (withheldReason(msg.authVerdict) === "auth") {
const check =
msg.authVerdict === "fail" ? "failed sender authentication" : "couldn't be authenticated"
return `This reply ${check}, so it wasn't posted to ${target}. It could be forged: confirm it with the city before publishing, or use Mark replied to leave it withheld.`
}
const sender = domainOfAddress(msg.from) || "an unknown sender"
return `This reply came from ${sender}, which isn't a domain this thread was sent to, so it wasn't posted to ${target}. Publish it if it's a genuine reply from the city, or use Mark replied to leave it withheld.`
}

export function publishConfirmBody(isReport: boolean): string {
return isReport
? "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."
: "It will be added to the event's timeline. This can't be undone."
}

export const PUBLISH_TOAST: Record<MailReplyPublication, string> = {
published: "Reply published",
pending: "Reply approved. It will be published shortly.",
withheld: "The reply is still withheld. Please try again.",
}
46 changes: 46 additions & 0 deletions apps/admin/src/features/mail/use-mail.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { readFileSync } from "node:fs"

import { MutationObserver, QueryClient } from "@tanstack/react-query"
import { describe, expect, it, vi } from "vitest"

import type * as apiModule from "@/lib/api"
import { api } from "@/lib/api"
import { queryKeys } from "@/lib/query"

import { publishMailReplyOptions } from "./use-mail"

vi.mock("@/lib/api", async (importOriginal) => ({
...(await importOriginal<typeof apiModule>()),
api: { publishMailReply: vi.fn() },
}))

describe("publish reply", () => {
it("publishes the one message and refreshes the thread, mail lists, reports and events", async () => {
vi.mocked(api.publishMailReply).mockResolvedValue({ publication: "published" })
const qc = new QueryClient()
const keys = [
queryKeys.mail.detail("t1"),
queryKeys.mail.list({}),
queryKeys.reports.detail("r1"),
queryKeys.events.detail("c1"),
]
for (const key of keys) qc.setQueryData(key, {})

const res = await new MutationObserver(qc, publishMailReplyOptions(qc)).mutate({
id: "t1",
messageId: "m1",
})

expect(res).toEqual({ publication: "published" })
expect(api.publishMailReply).toHaveBeenCalledWith({ id: "t1", messageId: "m1" })
for (const key of keys) expect(qc.getQueryState(key)?.isInvalidated).toBe(true)
})

it("asks the operator to confirm before publishing", () => {
const source = readFileSync(new URL("./withheld-reply-note.tsx", import.meta.url), "utf8")
const confirm = source.indexOf("await confirmDialog(")
expect(confirm).toBeGreaterThan(-1)
expect(source.indexOf("if (!ok) return")).toBeGreaterThan(confirm)
expect(source.indexOf("publish.mutate(")).toBeGreaterThan(source.indexOf("if (!ok) return"))
})
Comment on lines +39 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

View artifacts

T-Rex 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.

Fix in Claude Code

})
25 changes: 24 additions & 1 deletion apps/admin/src/features/mail/use-mail.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
"use client"

import { useInfiniteQuery, useMutation, useQuery, useQueryClient } from "@tanstack/react-query"
import {
mutationOptions,
useInfiniteQuery,
useMutation,
useQuery,
useQueryClient,
} from "@tanstack/react-query"
import type {
ComposeRequest,
GetForwardTemplateDefaultResponse,
Expand All @@ -11,6 +17,7 @@ import type {
MarkMailReadRequest,
PreviewForwardTemplateRequest,
PreviewForwardTemplateResponse,
PublishMailReplyRequest,
ReplyRequest,
ResendRequest,
SetForwardTemplateDefaultRequest,
Expand Down Expand Up @@ -104,6 +111,22 @@ export function useResendMail() {
})
}

export function publishMailReplyOptions(qc: ReturnType<typeof useQueryClient>) {
return mutationOptions({
mutationFn: (input: PublishMailReplyRequest) => api.publishMailReply(input),
onSuccess: (_res, { id }) => {
invalidateMail(qc, id)
qc.invalidateQueries({ queryKey: queryKeys.reports.all })
qc.invalidateQueries({ queryKey: queryKeys.events.all })
},
})
}

export function usePublishMailReply() {
const qc = useQueryClient()
return useMutation(publishMailReplyOptions(qc))
}

export function useForwardTemplateDefault() {
return useQuery<GetForwardTemplateDefaultResponse>({
queryKey: queryKeys.mail.forwardTemplate,
Expand Down
58 changes: 58 additions & 0 deletions apps/admin/src/features/mail/withheld-reply-note.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
"use client"

import type { MailMessageDTO } from "@civfix/shared"

import { Icons } from "@/components/icons"
import { confirmDialog } from "@/components/shared/dialog"
import { usePublishMailReply } from "@/features/mail/use-mail"
import {
PUBLISH_TOAST,
publishConfirmBody,
withheldNote,
withheldReason,
} from "@/features/mail/mail-presentation"
import { errorMessage } from "@/lib/error-messages"
import { useToast } from "@/store/ui-store"

export function WithheldReplyNote({
threadId,
msg,
isReport,
}: {
threadId: string
msg: MailMessageDTO
isReport: boolean
}) {
const publish = usePublishMailReply()
const toast = useToast()
const auth = withheldReason(msg.authVerdict) === "auth"

const onPublish = async () => {
const ok = await confirmDialog({
title: "Publish this reply?",
body: publishConfirmBody(isReport),
confirmLabel: "Publish reply",
cancelLabel: "Cancel",
danger: auth,
})
if (!ok) return
publish.mutate(
{ id: threadId, messageId: msg.id },
{
onSuccess: (res) => toast(PUBLISH_TOAST[res.publication]),
onError: (err) =>
toast(errorMessage(err, {}, { fallback: "Couldn't publish the reply. Please try again." })),
},
)
}

return (
<div className={`${auth ? "mail-bounce-note" : "mail-action-note"} mail-withheld-note`}>
{auth ? <Icons.AlertTriangle size={14} /> : <Icons.Shield size={14} />}
<span>{withheldNote(msg, isReport)}</span>
<button className="btn sm primary" disabled={publish.isPending} onClick={onPublish}>
<Icons.MessageSquare size={13} /> Publish reply
</button>
</div>
)
}
2 changes: 2 additions & 0 deletions apps/admin/src/styles/admin.css
Original file line number Diff line number Diff line change
Expand Up @@ -2844,6 +2844,8 @@ button.prow-main { color: inherit; font: inherit; background: none; border: 0; p
}
.mail-bounce-note { background: var(--bloom-50); color: var(--bloom-700); border: 1px solid color-mix(in oklab, var(--bloom) 18%, transparent); }
.mail-action-note { background: var(--sky-50); color: var(--sky-700); border: 1px solid color-mix(in oklab, var(--sky) 20%, transparent); }
.mail-withheld-note { margin-top: 8px; }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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!

Fix in Claude Code

.mail-withheld-note .btn { flex-shrink: 0; margin-left: auto; }
.mail-reader-foot { padding: 14px 18px; border-top: 1px solid var(--border-soft); display: flex; gap: 8px; flex-wrap: wrap; }

/* Mail thread bubbles */
Expand Down
Loading