fix(agreements): fill merge fields at send and refuse placeholders (QA sweep DELIV-01)

app/components/agreements/SendAgreementModal.test.tsx

@@ -0,0 +1,32 @@
+// @vitest-environment happy-dom
+/**
+ * DELIV-01 — the API now refuses to send an agreement that still carries
+ * placeholders. That refusal used to land in a page-level line underneath the
+ * modal's scrim, so Send looked like it did nothing. It must read inside the
+ * dialog the inspector is looking at.
+ */
+import { describe, it, expect } from "vitest";
+import { render, screen } from "@testing-library/react";
+
+import { SendAgreementModal } from "./SendAgreementModal";
+
+const base = {
+  open: true,
+  onSend: () => {},
+  onClose: () => {},
+  initialSigners: [{ name: "Pat Rivera", email: "pat@example.com", role: "client" as const }],
+};
+
+describe("SendAgreementModal serverError", () => {
+  it("shows the API's refusal as an alert inside the dialog", () => {
+    render(<SendAgreementModal {...base} serverError="This agreement cannot be sent yet: unfilled placeholders: [AMOUNT]." />);
+    const alert = screen.getByRole("alert");
+    expect(alert.textContent).toContain("cannot be sent yet");
+    expect(alert.closest('[role="dialog"]')).not.toBeNull();
+  });
+
+  it("shows nothing when there is no refusal", () => {
+    render(<SendAgreementModal {...base} />);
+    expect(screen.queryByRole("alert")).toBeNull();
+  });
+});

app/components/agreements/SendAgreementModal.tsx

@@ -101,6 +101,7 @@ export function SendAgreementModal({
     busy,
     initialSigners,
     templates,
+    serverError,
 }: {
     open: boolean;
     onSend: (payload: SendAgreementPayload) => void;
@@ -109,6 +110,8 @@ export function SendAgreementModal({
     initialSigners?: SignerDraft[];
     /** Agreement templates to choose from. Omit to hide the picker entirely. */
     templates?: AgreementTemplateOption[];
+    /** The API's refusal of the last send. Shown inside the dialog: a page-level line sits under the modal's scrim and reads as "Send did nothing". */
+    serverError?: string | null;
 }) {
     const seed = (): SignerDraftRow[] => {
         if (initialSigners && initialSigners.length > 0) {
@@ -272,7 +275,7 @@ export function SendAgreementModal({
                 </div>
             </fieldset>
 
-            {error && <p className="text-[13px] text-ih-bad-fg mt-3">{error}</p>}
+            {(error ?? serverError) && <p role="alert" className="text-[13px] text-ih-bad-fg mt-3">{error ?? serverError}</p>}
         </Modal>
     );
 }

app/routes/inspector-portal.tsx

@@ -1274,12 +1274,10 @@ export default function InspectionHubPage() {
             : undefined
         }
         busy={agreementModal.busy}
+        serverError={agreementModal.error}
         onSend={submitSendAgreement}
         onClose={() => agreementModal.setOpen(false)}
       />
-      {agreementModal.error && (
-        <p className="text-[12px] font-medium text-ih-bad-fg">{agreementModal.error}</p>
-      )}
 
       {/* Inspector pre-sign — the envelope-level signature an inspector applies
           before the client sees it. Moved off the Library page with the rest of

docs/develop/fork-log.md

@@ -331,3 +331,4 @@ deliberate departure rather than work upstream simply has not done yet.
 - Record skips as explicitly as takes. "We looked and said no" and "we never
   looked" are the same silence otherwise.
 - Update the snapshot when you fetch, not when you remember.
+| `agreement` merge fields filled at send, placeholders block the send (2026-10-13 QA sweep DELIV-01, Blocker; no upstream fix) | `AgreementService.findOrCreate` (`server/services/agreement/envelope-legacy.ts`) runs the template through `fillAgreementTokens` (`server/lib/agreement-tokens.ts`) before it freezes `content_snapshot`: `[INSPECTOR_NAME]` (assigned inspector, else the company's legal or company name), `[CUSTOMER_NAME]`, `[PROPERTY_ADDRESS]`, `[INSPECTION_DATE]` (long civil day) and `$[AMOUNT]` (`getEffectivePriceCents`: invoice, then service lines, then `inspections.price`); `[CLIENT_SIGNATURE_BLOCK]` and `[SIGN_DATE]` become "(signature captured below)" and "(recorded at signing)", values HTML-escaped. If any other bracketed ALL-CAPS token remains, or the starter's "Review before sending to real customers" notice is still in the text, it throws 422 naming them and creates no envelope. `SendAgreementModal` takes `serverError` and shows it inside the dialog (the page-level line it replaces sat under the scrim, so Send looked dead). Specs: `tests/unit/agreements/agreement-send-tokens.spec.ts`, `app/components/agreements/SendAgreementModal.test.tsx` | The finding: a fresh company's only template could be sent and signed with every merge field still in brackets and the admin caveat in the permanent record. `git log main..upstream/main` by keyword (placeholder, merge field, starter agreement, agreement) and on the fixture file finds nothing; the upstream plan lists it under its PR 5 but upstream has not landed it. The starter text is unchanged, so an untouched starter is refused (notice and the liability clause both stay) until the company edits it in Settings; that is the intended state of a legal document, not a gap. **Known ceilings:** the match is `[A-Z][A-Z0-9_]{2,}`-style brackets only, so a custom template with lowercase or `{{x}}` fields is sent as written; the check runs only when an envelope is created (reusing a live envelope does not re-check); an inspection with no price blocks on `[AMOUNT]` rather than printing $0.00; the signing page itself still shows what the snapshot holds, so envelopes sent before this change keep their placeholders |

server/lib/agreement-tokens.ts

@@ -0,0 +1,61 @@
+/**
+ * Send-time merge fields for an agreement template.
+ *
+ * The starter template ships `[INSPECTOR_NAME]`, `[CUSTOMER_NAME]`,
+ * `[PROPERTY_ADDRESS]`, `[INSPECTION_DATE]` and `$[AMOUNT]`. They are filled
+ * from the inspection when the envelope is created, because the envelope's
+ * `content_snapshot` is the permanent signed record: whatever is still in
+ * brackets here is what the client signs and what a dispute produces.
+ *
+ * Anything bracketed that is NOT filled, and the starter's "review before
+ * sending" notice, block the send instead (`blockingAgreementIssues`).
+ */
+
+export interface AgreementTokenValues {
+    inspector?: string | null | undefined;
+    client?: string | null | undefined;
+    address?: string | null;
+    /** Civil day, `YYYY-MM-DD` (an `T…Z` suffix is ignored). */
+    date?: string | null;
+    amountCents?: number | null;
+}
+
+/** Substring of the starter template's admin-only caveat paragraph. */
+const STARTER_NOTICE_MARK = 'Review before sending to real customers';
+
+const esc = (s: string): string =>
+    s.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;');
+
+const longDay = (d: string): string | null => {
+    const t = new Date(`${d.slice(0, 10)}T12:00:00Z`);
+    return Number.isNaN(t.getTime()) ? null : t.toLocaleDateString('en-US', { timeZone: 'UTC', dateStyle: 'long' }); // i18n-lint-ok: the agreement body is English-only and frozen into content_snapshot
+};
+
+export function fillAgreementTokens(html: string, v: AgreementTokenValues): string {
+    const day = v.date ? longDay(v.date) : null;
+    const map: Record<string, string | null> = {
+        INSPECTOR_NAME:  v.inspector?.trim() || null,
+        CUSTOMER_NAME:   v.client?.trim() || null,
+        PROPERTY_ADDRESS: v.address?.trim() || null,
+        INSPECTION_DATE: day,
+        AMOUNT:          v.amountCents != null && v.amountCents > 0 ? (v.amountCents / 100).toFixed(2) : null,
+        // Not known at send time. The signature itself and its timestamp are
+        // recorded with the signer row; say so rather than leave a bracket.
+        CLIENT_SIGNATURE_BLOCK: '(signature captured below)',
+        SIGN_DATE:       '(recorded at signing)',
+    };
+    return html.replace(/\[([A-Z_]+)\]/g, (whole, key: string) => {
+        const val = map[key];
+        return val ? esc(val) : whole;
+    });
+}
+
+/** Human-readable reasons this body must not be sent as it stands. */
+export function blockingAgreementIssues(html: string): string[] {
+    const issues: string[] = [];
+    const tokens = [...new Set(html.match(/\[[A-Z][A-Z0-9_]{2,}\b[^\]]*\]/g) ?? [])]
+        .map((t) => (t.length > 40 ? `${t.slice(0, 37)}...]` : t));
+    if (tokens.length > 0) issues.push(`unfilled placeholders: ${tokens.join(', ')}`);
+    if (html.includes(STARTER_NOTICE_MARK)) issues.push('the "review before sending" notice is still in the text');
+    return issues;
+}

server/services/agreement/envelope-legacy.ts

@@ -1,5 +1,7 @@
 import { eq, and, inArray, lt, sql, desc, asc } from 'drizzle-orm';
-import { agreements, agreementRequests, agreementSigners, inspections } from '../../lib/db/schema';
+import { agreements, agreementRequests, agreementSigners, inspections, inspectionServices, invoices, users } from '../../lib/db/schema';
+import { getEffectivePriceCents } from '../../lib/effective-price';
+import { fillAgreementTokens, blockingAgreementIssues } from '../../lib/agreement-tokens';
 import { Errors } from '../../lib/errors';
 import { logger } from '../../lib/logger';
 import { mintToken, hashToken } from '../../lib/token-hash';
@@ -131,7 +133,10 @@ export function EnvelopeLegacyMixin<TBase extends Constructor<AgreementServiceBa
                 return { token, status: env.status, alreadyExists: true, requestId: env.id, addedSignerIds };
             }
             // Verify the inspection exists in this tenant.
-            const inspRows = await db.select({ id: inspections.id }).from(inspections)
+            const inspRows = await db.select({
+                id: inspections.id, inspectorId: inspections.inspectorId, address: inspections.propertyAddress,
+                date: inspections.date, price: inspections.price,
+            }).from(inspections)
                 .where(and(eq(inspections.id, inspectionId), eq(inspections.tenantId, tenantId))).limit(1);
             if (inspRows.length === 0) throw Errors.NotFound('Inspection not found');
             // Pick the agreement template: explicit id (tenant-scoped) or the tenant's first template.
@@ -173,13 +178,45 @@ export function EnvelopeLegacyMixin<TBase extends Constructor<AgreementServiceBa
             const completionPolicy = opts?.completionPolicy ?? 'all';
             const now = new Date();
             const requestId = crypto.randomUUID();
-            const contentSnapshot = agreement.content;
+            // The snapshot is the permanent signed record: merge fields are
+            // filled now, and a body that still carries placeholders or the
+            // starter's admin notice is refused rather than sent (DELIV-01).
+            const insp = inspRows[0];
+            const [inspector, invoice, lines] = await Promise.all([
+                insp.inspectorId
+                    ? db.select({ name: users.name }).from(users)
+                        .where(and(eq(users.id, insp.inspectorId), eq(users.tenantId, tenantId))).get()
+                    : undefined,
+                db.select({ amountCents: invoices.amountCents }).from(invoices)
+                    .where(and(eq(invoices.inspectionId, inspectionId), eq(invoices.tenantId, tenantId)))
+                    .orderBy(desc(invoices.createdAt)).limit(1).get(),
+                db.select({ priceSnapshot: inspectionServices.priceSnapshot, priceOverride: inspectionServices.priceOverride })
+                    .from(inspectionServices)
+                    .where(and(eq(inspectionServices.inspectionId, inspectionId), eq(inspectionServices.tenantId, tenantId))).all(),
+            ]);
+            const brand = await new BrandingService(this.db).getBrand(tenantId);
+            const contentSnapshot = fillAgreementTokens(agreement.content, {
+                inspector: inspector?.name || brand.legalName || brand.companyName,
+                client: resolvedClientName || primaryClient?.email,
+                address: insp.address,
+                date: insp.date,
+                amountCents: getEffectivePriceCents({
+                    invoiceAmountCents: invoice?.amountCents ?? null,
+                    serviceLines: lines,
+                    inspectionPriceCents: insp.price,
+                }),
+            });
+            const issues = blockingAgreementIssues(contentSnapshot);
+            if (issues.length > 0) {
+                throw Errors.UnprocessableEntity(
+                    `This agreement cannot be sent yet: ${issues.join('; ')}. Edit the template in Settings, then send again.`,
+                );
+            }
             const contentHash = await sha256Hex(contentSnapshot);
             // Spec 2026-08-04 section 3 — freeze the contracting identity onto
             // the envelope. Renaming the company must not retroactively rewrite
             // which entity a past agreement was signed with. `legalName` arrives
             // ALREADY resolved from getBrand; no fallback is re-applied here.
-            const brand = await new BrandingService(this.db).getBrand(tenantId);
 
             const newRow = {
                 id: requestId,

tests/unit/agreements/agreement-send-tokens.spec.ts

@@ -0,0 +1,97 @@
+/**
+ * DELIV-01 — the starter agreement used to be sendable and signable with every
+ * merge field still in brackets and the admin-only "review before sending"
+ * notice inside the permanent signed snapshot. Fields are now filled at send
+ * time and a body that still carries placeholders or the notice is refused.
+ */
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+import { eq } from 'drizzle-orm';
+import { AgreementService } from '../../../server/services/agreement.service';
+import { PeopleService } from '../../../server/services/people.service';
+import { seedRoleProfiles } from '../../../server/services/seed/seed-role-profiles';
+import { AGREEMENT_TEMPLATE } from '../../../server/services/starter-content/fixtures/agreement-template';
+import { fillAgreementTokens, blockingAgreementIssues } from '../../../server/lib/agreement-tokens';
+import { createTestDb, setupSchema } from '../db';
+import { asD1Db } from '../helpers/test-db';
+import * as schema from '../../../server/lib/db/schema';
+import type { BetterSQLite3Database } from 'drizzle-orm/better-sqlite3';
+
+vi.mock('drizzle-orm/d1', () => ({ drizzle: vi.fn() }));
+import { drizzle as mockDrizzle } from 'drizzle-orm/d1';
+
+const TENANT = '00000000-0000-0000-0000-000000000001';
+const CLIENT = 'contact-client-1';
+const INSP_ID = '550e8400-e29b-41d4-a716-446655440000';
+const AGR_ID = '00000000-0000-0000-0000-000000000020';
+const USER = 'user-inspector-1';
+
+const withoutNotice = AGREEMENT_TEMPLATE.content.replace(/^<p><strong>⚠️.*?<\/p>/, '');
+
+describe('fillAgreementTokens / blockingAgreementIssues', () => {
+    it('fills the starter fields and leaves nothing blocking', () => {
+        const out = fillAgreementTokens(withoutNotice.replace(/<h3>4\..*?<\/p>/, ''), {
+            inspector: 'Quinn <Assurance>', client: 'Pat Rivera', address: '742 Ivy Lane', date: '2026-10-20T09:00:00Z', amountCents: 40000,
+        });
+        expect(out).toContain('Quinn &lt;Assurance&gt; ("Inspector")');
+        expect(out).toContain('Pat Rivera ("Client")');
+        expect(out).toContain('October 20, 2026');
+        expect(out).toContain('Inspection fee: $400.00.');
+        expect(blockingAgreementIssues(out)).toEqual([]);
+    });
+
+    it('a missing value stays bracketed and blocks', () => {
+        const out = fillAgreementTokens('<p>[CUSTOMER_NAME] pays $[AMOUNT]</p>', { client: 'Pat', amountCents: 0 });
+        expect(out).toBe('<p>Pat pays $[AMOUNT]</p>');
+        expect(blockingAgreementIssues(out)[0]).toContain('[AMOUNT]');
+    });
+});
+
+describe('AgreementService.findOrCreate — starter agreement merge fields (DELIV-01)', () => {
+    let db: BetterSQLite3Database<typeof schema>;
+    let svc: AgreementService;
+
+    const seedAgreement = (content: string) =>
+        db.update(schema.agreements).set({ content }).where(eq(schema.agreements.id, AGR_ID));
+
+    beforeEach(async () => {
+        const fixture = createTestDb();
+        db = fixture.db;
+        await setupSchema(fixture.sqlite);
+        (mockDrizzle as unknown as ReturnType<typeof vi.fn>).mockReturnValue(db);
+        await db.insert(schema.tenants).values({
+            id: TENANT, slug: 'acme', status: 'active', deploymentMode: 'shared', tier: 'free', createdAt: new Date(),
+        });
+        await db.insert(schema.users).values({
+            id: USER, tenantId: TENANT, email: 'q@example.com', passwordHash: 'x', name: 'Quinn Assurance', role: 'owner', createdAt: new Date(),
+        } as typeof schema.users.$inferInsert);
+        await db.insert(schema.agreements).values({
+            id: AGR_ID, tenantId: TENANT, name: AGREEMENT_TEMPLATE.name, content: AGREEMENT_TEMPLATE.content, version: 1, createdAt: new Date(),
+        });
+        await seedRoleProfiles(asD1Db(db), TENANT, new Date(1));
+        await db.insert(schema.contacts).values({
+            id: CLIENT, tenantId: TENANT, type: 'client', name: 'Pat Rivera', email: 'pat@example.com', phone: '+15551234567', createdAt: new Date(),
+        });
+        await db.insert(schema.inspections).values({
+            id: INSP_ID, tenantId: TENANT, inspectorId: USER, propertyAddress: '742 Ivy Lane, Charlotte, NC', date: '2026-10-20',
+            status: 'requested', paymentStatus: 'unpaid', price: 40000, agreementRequired: true, paymentRequired: false, createdAt: new Date(),
+        });
+        await new PeopleService({ DB: {} as D1Database }).addPerson(TENANT, INSP_ID, CLIENT, `crp_${TENANT}_client`);
+        svc = new AgreementService({} as D1Database, { jwtSecret: 'test-secret' });
+    });
+
+    it('the unedited starter template is refused and creates no envelope', async () => {
+        await expect(svc.findOrCreate(TENANT, INSP_ID)).rejects.toMatchObject({ status: 422 });
+        expect(await db.select().from(schema.agreementRequests).all()).toHaveLength(0);
+    });
+
+    it('once the notice and liability placeholder are edited out, the snapshot carries the real values', async () => {
+        await seedAgreement(withoutNotice.replace(/<h3>4\..*?<\/p>/, ''));
+        const r = await svc.findOrCreate(TENANT, INSP_ID);
+        const env = await db.select().from(schema.agreementRequests).where(eq(schema.agreementRequests.id, r.requestId)).get();
+        expect(env?.contentSnapshot).toContain('Quinn Assurance ("Inspector")');
+        expect(env?.contentSnapshot).toContain('Pat Rivera ("Client")');
+        expect(env?.contentSnapshot).toContain('742 Ivy Lane, Charlotte, NC on October 20, 2026');
+        expect(env?.contentSnapshot).toContain('$400.00');
+        expect(env?.contentSnapshot).not.toMatch(/\[[A-Z_]{3,}/);
+    });
+});