diff --git a/.claude/TODO/2026-09-13-mergetemplate-logs-address-pii-on-error.md b/.claude/TODO/2026-09-13-mergetemplate-logs-address-pii-on-error.md
index 77ba706..dc9d811 100644
--- a/.claude/TODO/2026-09-13-mergetemplate-logs-address-pii-on-error.md
+++ b/.claude/TODO/2026-09-13-mergetemplate-logs-address-pii-on-error.md
@@ -6,7 +6,7 @@ area: components
 files: [src/components/address-labels/actions.ts]
 discovered: 2026-09-13
 discovered_by: coverage-agent-address-labels
-status: open
+status: resolved
 ---
 
 ## Problem
@@ -56,3 +56,31 @@ scope parser causes every printable household's name and mailing address for
 that batch to be written to application/server logs (e.g. Vercel function
 logs), which are typically retained, more widely readable, and less access
 controlled than the MP database itself.
+
+---
+
+## Resolution (2026-09-13)
+Added a `describeError()` helper in `src/components/address-labels/actions.ts`
+and routed all three `console.error` calls through it —
+`generateLabelPdf`, `generateLabelDocx`, and `mergeTemplate`.
+
+It reduces a caught value to `{ name, message }`, plus docxtemplater's own
+`properties.id` and `properties.explanation` when present. Those two are
+genuinely useful for telling which failure mode occurred and contain no caller
+data. Everything else on `properties` — crucially `scope`, which in this
+feature IS the household list — is dropped. A non-`Error` throw is reported as
+`{ name: 'NonError', type: typeof error }` rather than by value, since a thrown
+string can itself be data-derived.
+
+The user-facing return value still carries `error.message`. That is the
+diagnostic the person fixing their template needs, and docxtemplater's messages
+reference tag names from the uploaded template, not merge data.
+
+### Tests
+`src/components/address-labels/actions.test.ts` — 5 cases driving a
+docxtemplater-shaped error whose `properties.scope` holds a real-looking name
+and address, asserting none of it reaches the log while the identifiers do.
+One asserts on the logged object's KEYS rather than substrings: the word
+"scope" legitimately appears inside the safe identifier
+`scopeparser_execution_failed`, so a naive `not.toContain('scope')` fails for
+the wrong reason.
diff --git a/.claude/TODO/2026-09-13-page-logs-raw-error-object.md b/.claude/TODO/2026-09-13-page-logs-raw-error-object.md
index 14cf70b..77fac66 100644
--- a/.claude/TODO/2026-09-13-page-logs-raw-error-object.md
+++ b/.claude/TODO/2026-09-13-page-logs-raw-error-object.md
@@ -6,7 +6,7 @@ area: components
 files: [src/app/(web)/tools/addeditfamily/page.tsx]
 discovered: 2026-09-13
 discovered_by: coverage-agent-addeditfamily
-status: open
+status: resolved
 ---
 
 ## Problem
@@ -60,3 +60,29 @@ resolution has already failed, and only to server logs. But it is the second
 place after `tool-params.server.ts`'s example that this exact swallow-and-warn
 shape appears, and it is the one that gets it wrong; left uncorrected, it is
 an easy pattern to copy into the next new page.
+
+---
+
+## Resolution (2026-09-13)
+Fixed alongside the identical anti-pattern in
+`src/components/address-labels/actions.ts` — shipping a redaction fix for one
+file while leaving the same bug in the feature next door would have left the
+codebase with two standards.
+
+```ts
+console.warn("addeditfamily.resolve_contact_id_failed", {
+  table: params.pageData.Table_Name,
+  name: error instanceof Error ? error.name : "NonError",
+});
+```
+
+The table name is a configuration identifier, not record content, and rule 14
+explicitly allows "table, IDs, HTTP status". The error's `message` is dropped
+because `resolveContactIdFromPage` builds an MP `$filter` from the record id
+and column path, and MP surfaces that filter back inside its error text.
+
+### Tests
+`src/app/(web)/tools/addeditfamily/page.test.tsx` — the existing swallow test
+now asserts the redacted shape, plus two new cases: a rejection whose message
+contains a filter string never reaches the log, and a non-`Error` throw is
+described as `NonError` rather than by value.
diff --git a/.claude/TODO/2026-09-13-removegroup-order-guard-mismatch.md b/.claude/TODO/2026-09-13-removegroup-order-guard-mismatch.md
index 0cab77d..94a32e7 100644
--- a/.claude/TODO/2026-09-13-removegroup-order-guard-mismatch.md
+++ b/.claude/TODO/2026-09-13-removegroup-order-guard-mismatch.md
@@ -6,7 +6,7 @@ area: components
 files: [src/components/field-management/use-field-order-state.ts]
 discovered: 2026-09-13
 discovered_by: coverage-agent-field-management
-status: open
+status: resolved
 ---
 
 ## Problem
@@ -87,3 +87,43 @@ surfaced to the admin performing the edit. This directly corrupts live
 Ministry Platform page field configuration (view order, group assignment,
 required/hidden/filter settings all lost for the affected fields) until
 someone notices fields missing from the page and manually re-adds them.
+
+---
+
+## Resolution (2026-09-13)
+Applied option 1 from the proposed fix: `groupOrder` is now guarded the same
+way `groupedFields` was, so a non-empty group is a complete no-op.
+
+The guard moved OUT of the `setGroupedFields` updater and up to the top of the
+callback, reading the rendered `groupedFields` (added to the dependency array,
+matching how `addGroup` already reads `isFlat`):
+
+```ts
+const removeGroup = useCallback(
+  (name: string) => {
+    if ((groupedFields[name] || []).length > 0) return;
+    setGroupedFields((prev) => { const { [name]: _, ...rest } = prev; return rest; });
+    setGroupOrder((prev) => prev.filter((g) => g !== name));
+    setIsDirty(true);
+  },
+  [groupedFields],
+);
+```
+
+### A wrong first attempt, recorded so it is not retried
+The obvious fix — set a `let removed = false` flag inside the
+`setGroupedFields` updater and check it before calling `setGroupOrder` — does
+not work. React runs state updaters during the render phase, not synchronously
+at call time, so the flag is still `false` when the second setter is reached.
+It would have looked correct, passed a casual read, and silently kept the bug.
+The decision has to be made once, up front, from state both setters agree on.
+
+`isDirty` is no longer set when the removal is refused: nothing changed, so
+there is nothing to save.
+
+### Tests
+`src/components/field-management/use-field-order-state.test.ts` — the test that
+documented the broken behaviour was replaced with four that pin the fix: both
+halves of state left intact, fields still present in `buildSavePayload()`,
+`isDirty` untouched on refusal, and a group still removable once emptied (with
+its field surviving).
diff --git a/.claude/TODO/2026-09-13-testing-no-typecheck-gate-in-ci.md b/.claude/TODO/2026-09-13-testing-no-typecheck-gate-in-ci.md
index e3cfd4f..2ea39f5 100644
--- a/.claude/TODO/2026-09-13-testing-no-typecheck-gate-in-ci.md
+++ b/.claude/TODO/2026-09-13-testing-no-typecheck-gate-in-ci.md
@@ -6,7 +6,7 @@ area: testing
 files: [.github/workflows/test.yml, package.json]
 discovered: 2026-09-13
 discovered_by: coverage-review-orchestrator
-status: open
+status: resolved
 ---
 
 ## Problem
@@ -83,3 +83,37 @@ The specific breakage is repaired, but the gap that allowed it is still open. A
 type error reaching `dev` breaks `npm run build` and therefore a Vercel
 production deploy, while CI stays green — so it is found at deploy time by
 whoever is shipping, not at PR time by whoever wrote it.
+
+---
+
+## Resolution (2026-09-13)
+Added `"typecheck": "tsc --noEmit"` to `package.json` and wired it into
+`.github/workflows/test.yml`, along with two other gaps found while in there.
+
+**Added as STEPS, not new jobs.** Branch protection on `dev` and `main`
+requires the status check named `test`. Lint and typecheck as separate jobs
+would have been green-but-unrequired until someone also edited the protection
+rules — a gate that does not gate. As steps in the existing `test` job they are
+covered by the rule already in place, with no admin change needed.
+
+Order is install -> lint -> typecheck -> test:coverage, so the cheap checks
+fail first. The Codecov step keeps `if: always()` and `fail_ci_if_error: false`,
+so an early failure that produces no coverage file does not itself fail the
+build.
+
+### Two other CI issues fixed in the same pass
+- **`npm install` -> `npm ci`.** `npm install` resolves fresh versions and
+  rewrites `package-lock.json` inside CI, so CI could be testing a different
+  dependency tree than any developer had. `npm ci` installs exactly what the
+  lockfile pins and fails outright if `package.json` and the lockfile disagree.
+  Verified with `npm ci --dry-run` before committing.
+- **No `concurrency` group.** Rapid pushes to a PR branch left superseded runs
+  burning minutes. Now cancels in-progress runs for the same ref, except on
+  `main` and `dev`, whose runs gate merges and releases and must not be killed.
+
+Also added `permissions: contents: read` — this workflow only reads the repo,
+and Codecov authenticates with its own token rather than `GITHUB_TOKEN`.
+
+`CLAUDE.md` previously documented "CI gates tests only ... Type-check locally"
+as the mitigation. That line should now be read alongside this change: the
+honour system has been replaced by a gate.
diff --git a/.claude/TODO/2026-09-13-unvalidated-envelope-donor-ids-in-filter.md b/.claude/TODO/2026-09-13-unvalidated-envelope-donor-ids-in-filter.md
index a59cd7a..ae949b0 100644
--- a/.claude/TODO/2026-09-13-unvalidated-envelope-donor-ids-in-filter.md
+++ b/.claude/TODO/2026-09-13-unvalidated-envelope-donor-ids-in-filter.md
@@ -6,7 +6,7 @@ area: services
 files: [src/services/familyService.ts, src/app/(web)/tools/addeditfamily/actions.ts, src/lib/dto/family.ts]
 discovered: 2026-09-13
 discovered_by: coverage-agent-services
-status: open
+status: resolved
 ---
 
 ## Problem
@@ -75,3 +75,48 @@ not a plain positive integer. Depending on how the MP OData-style `$filter`
 parser handles the resulting string, this ranges from a confusing 400/500
 error to a filter-injection primitive against the `Donors` table — the same
 class of risk rule 11 exists to close off for string fields.
+
+---
+
+## Resolution (2026-09-13)
+Both layers of the proposed fix were applied.
+
+**1. Parse at the action boundary.** `saveFamily` now runs
+`HouseholdSchema.safeParse(household)` before the payload reaches
+`FamilyService`, and returns an `ActionError` on failure. The error reports the
+offending field PATHS only (`members.0.envelopeNo`), never the submitted
+values — rule 14 applies to returned error strings, which travel further than
+logs. Authorization still runs first, so an unauthorized caller learns nothing
+about the schema.
+
+**2. Validate where the filter is built.** `resolveUniqueEnvelopeNo` calls
+`validatePositiveInt` on `requested` and on `excludeDonorId`, and re-validates
+`candidate` on every loop iteration so a future change to
+`getNextEnvelopeNumber()` cannot reintroduce an unchecked value. `upsertDonor`
+validates `existingDonorId` before using it to target the `Donors` update.
+`null` and `0` are preserved as the "no donor to exclude" sentinels rather than
+being rejected.
+
+### Correction to the original severity assessment
+Re-reading this while fixing it: the classic injection string was already
+blocked, but incidentally rather than by design. `upsertDonor` guards with
+`envelopeNo > 0`, and JS coerces `"1 OR 1=1"` to `NaN`, making that comparison
+false — so the crafted string never reached the filter. What *did* get through
+were values that survive numeric coercion but are not positive integers:
+`1.5`, `Infinity`, `1e21` (which interpolates as the malformed `1e+21`), and
+numeric strings. Those produce malformed filters and confusing 400/500s, not a
+filter-injection primitive.
+
+So the practical severity was lower than "high" as originally filed. The fix is
+still correct and worth keeping: relying on an incidental coercion side effect
+to block injection is fragile, and a schema change or a new call path that
+drops the `> 0` guard would turn it into the real thing with nothing to catch
+it. Recorded here so the next reader is not misled by the original framing.
+
+### Tests
+- `src/app/(web)/tools/addeditfamily/actions.test.ts` — 11 cases: injection-shaped
+  `envelopeNo`/`donorId`, non-integer/array/object/boolean fields, missing
+  top-level fields, path reporting, no value echoed back, authorize-before-validate.
+- `src/services/familyService.test.ts` — 7 cases asserting no malformed value
+  ever reaches an MP query, that `donorId: 0` is still accepted as "none", and
+  that well-formed input produces the exact expected filter string.
diff --git a/.claude/TODO/INDEX.md b/.claude/TODO/INDEX.md
index adc8793..e6b6dc6 100644
--- a/.claude/TODO/INDEX.md
+++ b/.claude/TODO/INDEX.md
@@ -22,14 +22,19 @@ Severity tiers:
 - **medium**: doc drift, missing test, refactor with real cost
 - **low**: nits, minor doc fixes, stylistic improvements
 
-Total: **9 open TODOs**.
+Total: **4 open TODOs**.
 
 > **2026-09-13 — unit-test coverage push.** Statement coverage over authored
 > code went from 49.67% to 98.84% (3,610/3,652), lines to 99.70%, across 1,535
 > tests in 116 files. Most of the items below were opened during that work,
 > found by reading code while writing tests for it. A follow-up pass cleaned
-> the runner output from 1,507 lines to 17 with zero warnings. Details in each
-> file.
+> the runner output from 1,507 lines to 17 with zero warnings.
+>
+> **2026-09-13 (later) — remediation.** All three high-severity items and the
+> CI gap are fixed and closed; see each file's Resolution section. CI now runs
+> lint and `tsc --noEmit` as required steps, and installs with `npm ci`.
+> Everything still open is `medium` or below, and all of it sits in
+> `src/components`.
 
 ---
 
@@ -38,18 +43,12 @@ Total: **9 open TODOs**.
 ### Critical (0)
 _none open_
 
-### High (3)
-| Area | Tags | Title | File |
-|---|---|---|---|
-| services | security, bug | Unvalidated client-supplied numeric fields interpolated into MP `$filter` strings in FamilyService | [→](2026-09-13-unvalidated-envelope-donor-ids-in-filter.md) |
-| components | bug | `removeGroup` drops a non-empty group's fields from the save payload | [→](2026-09-13-removegroup-order-guard-mismatch.md) |
-| components | security, bug | `mergeTemplate` console.errors the raw docxtemplater error, which can carry household addresses | [→](2026-09-13-mergetemplate-logs-address-pii-on-error.md) |
+### High (0)
+_none open_
 
-### Medium (4)
+### Medium (2)
 | Area | Tags | Title | File |
 |---|---|---|---|
-| testing | bug, testing, drift | Add a type-check gate to CI (stale fixtures had silently broken `npm run build`) | [→](2026-09-13-testing-no-typecheck-gate-in-ci.md) |
-| components | security, drift | `AddEditFamilyPage` logs the raw error object instead of an identifier | [→](2026-09-13-page-logs-raw-error-object.md) |
 | components | bug, drift | Template editor ignores pageID/recordID (no MP persistence) | [→](2026-04-17-components-template-editor-no-mp-persistence.md) |
 | components | bug, refactor | Merge tokens `{{Field_Name}}` have no resolver anywhere | [→](2026-04-17-components-template-editor-merge-token-resolver.md) |
 
@@ -63,23 +62,15 @@ _none open_
 
 ## By tag
 
-### security (3)
-- unvalidated-envelope-donor-ids-in-filter — high
-- mergetemplate-logs-address-pii-on-error — high
-- page-logs-raw-error-object — medium
+### security (0)
+_none open_
 
-### bug (6)
-- unvalidated-envelope-donor-ids-in-filter — high
-- removegroup-order-guard-mismatch — high
-- mergetemplate-logs-address-pii-on-error — high
-- testing-no-typecheck-gate-in-ci — medium
+### bug (3)
 - components-template-editor-no-mp-persistence — medium
 - components-template-editor-merge-token-resolver — medium
 - search-empty-state-never-renders — low
 
-### drift (3)
-- testing-no-typecheck-gate-in-ci — medium
-- page-logs-raw-error-object — medium
+### drift (1)
 - components-template-editor-no-mp-persistence — medium
 
 ### missing-test (1)
@@ -89,8 +80,8 @@ _none open_
 - components-template-editor-merge-token-resolver — medium
 - dead-empty-fields-branch-handlenext — low
 
-### testing (1)
-- testing-no-typecheck-gate-in-ci — medium
+### testing (0)
+_none open_
 
 ### doc (0)
 _none open_
@@ -104,9 +95,9 @@ _none open_
 
 | Area | Count |
 |---|---|
-| components | 6 |
-| testing | 1 |
-| services | 1 |
+| components | 4 |
+| testing | 0 |
+| services | 0 |
 | auth | 0 |
 | mp-provider | 0 |
 | utils | 0 |
@@ -126,6 +117,11 @@ _none open_
 | 2026-09-13 | No tests for `src/components/template-editor/` | [→](2026-04-17-components-template-editor-missing-tests.md) |
 | 2026-09-13 | `vitest.config.ts` loaded as CommonJS (renamed to `.mts`) | [→](2026-09-13-testing-vitest-config-loaded-as-cjs.md) |
 | 2026-09-13 | Radix Select fields switched uncontrolled -> controlled | [→](2026-09-13-components-select-uncontrolled-to-controlled.md) |
+| 2026-09-13 | Unvalidated numeric fields reaching MP `$filter` strings | [→](2026-09-13-unvalidated-envelope-donor-ids-in-filter.md) |
+| 2026-09-13 | `removeGroup` dropped a non-empty group's fields from the save payload | [→](2026-09-13-removegroup-order-guard-mismatch.md) |
+| 2026-09-13 | Raw docxtemplater error logged household addresses | [→](2026-09-13-mergetemplate-logs-address-pii-on-error.md) |
+| 2026-09-13 | `AddEditFamilyPage` logged the raw error object | [→](2026-09-13-page-logs-raw-error-object.md) |
+| 2026-09-13 | No type-check gate in CI (also `npm ci`, lint, concurrency) | [→](2026-09-13-testing-no-typecheck-gate-in-ci.md) |
 
 ---
 
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 923cdea..1a14124 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -6,7 +6,24 @@ on:
   pull_request:
     branches: [main, dev]
 
+# A new push to the same branch supersedes the run in flight. Pushes to the
+# protected trunks are never cancelled — their runs gate merges and releases.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: ${{ github.ref != 'refs/heads/main' && github.ref != 'refs/heads/dev' }}
+
+# Least privilege: this workflow only reads the repo. The Codecov upload
+# authenticates with its own token, not the GITHUB_TOKEN.
+permissions:
+  contents: read
+
 jobs:
+  # Deliberately ONE job, not several.
+  #
+  # Branch protection on `dev` and `main` requires the status check named
+  # `test`. Adding lint/typecheck as separate jobs would leave them
+  # unrequired — green-but-ignored — until someone also updated the
+  # protection rules. As steps here they are covered by the existing rule.
   test:
     runs-on: ubuntu-latest
 
@@ -18,9 +35,27 @@ jobs:
           node-version-file: .nvmrc
           cache: npm
 
-      - run: npm install
+      # `npm ci`, not `npm install`: it installs exactly what the lockfile
+      # pins and fails if package.json and package-lock.json disagree, rather
+      # than silently resolving new versions and rewriting the lockfile in CI.
+      - run: npm ci
+
+      - name: Lint
+        run: npm run lint
+
+      # The gate this repo was missing.
+      #
+      # `tsconfig.json` includes `**/*.ts` and `**/*.tsx`, and `next.config.ts`
+      # sets no `ignoreBuildErrors` — so a type error in a TEST file breaks
+      # `npm run build`, and therefore a production deploy, while the suite
+      # itself stays green. That is not hypothetical: two committed test files
+      # sat on `dev` in exactly that state, caught only by a manual local run.
+      - name: Type check
+        run: npm run typecheck
 
-      - run: npm run test:coverage
+      # Enforces the coverage thresholds in vitest.config.mts.
+      - name: Test with coverage
+        run: npm run test:coverage
 
       - uses: codecov/codecov-action@v5
         if: always()
diff --git a/CLAUDE.md b/CLAUDE.md
index 5ff425a..5eb1a22 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -7,6 +7,7 @@ This guide provides essential information for AI assistants (like Claude) workin
 - **Dev**: `npm run dev` (Next.js dev server)
 - **Build**: `npm run build` (builds SQL install script, then production build with Turbopack + type checking)
 - **Lint**: `npm run lint` (ESLint CLI — `next lint` was removed in Next.js 16)
+- **Type check**: `npm run typecheck` (`tsc --noEmit`; enforced in CI)
 - **Generate MP Types**: `npm run mp:generate:models` (generates TypeScript types + Zod schemas from Ministry Platform API, cleans output directory first)
 - **Generate MP Stored Procs**: `npm run mp:generate:storedprocs` (generates stored procedure reference from Ministry Platform API)
 - **Build MP SQL Install**: `npm run mp:build:install` (combines SQL files from `db/` into unified `_INSTALL/ministryplatform-install.sql`, skips if unchanged)
@@ -239,7 +240,7 @@ await mp.createTableRecords('Contact_Log', records, {
 - **MPHelper mock**: Use mock class (`MPHelper: class { method = mockFn; }`), not `vi.fn().mockImplementation()`
 - **Singleton reset**: Reset `(ServiceClass as any).instance = undefined` in `beforeEach` to prevent state leakage
 - **Server action tests**: Mock `@/lib/auth` (`auth.api.getSession`), `next/headers` (`headers()`), and service singletons
-- **Type-check locally before pushing**: CI does not run `tsc`, and `tsconfig.json` includes `**/*.ts`/`**/*.tsx`, so a type error in a *test* file breaks `npm run build` while CI stays green. This has happened — see `.claude/TODO/2026-09-13-testing-no-typecheck-gate-in-ci.md`.
+- **CI type-checks**: `npm run typecheck` (`tsc --noEmit`) runs as a required step in the `test` job, alongside `npm run lint`. `tsconfig.json` includes `**/*.ts`/`**/*.tsx`, so a type error in a *test* file breaks `npm run build` — that used to reach `dev` unnoticed because CI ran tests only.
 - See **[Testing Reference](.claude/references/testing/README.md)** for all mock patterns, coverage data, and test inventory
 
 ## Dependencies
@@ -263,8 +264,16 @@ condition that clears it. Do not re-derive that analysis.
   with <https://vercel.com/docs/functions/runtimes/node-js/node-js-versions>,
   then bump `engines.node`, `.nvmrc`, `@types/node`, and `REQUIRED_NODE_MAJOR`
   in `scripts/setup.ts` together.
-- **CI gates tests only** — `.github/workflows/test.yml` runs `npm run test:coverage`
-  and never `npm run build`, so type errors do not fail CI. Type-check locally.
+- **CI gates lint, types and tests** — `.github/workflows/test.yml` runs
+  `npm run lint`, `npm run typecheck` and `npm run test:coverage`, in that
+  order, as steps of the single `test` job. They are steps rather than separate
+  jobs on purpose: branch protection requires the check named `test`, so extra
+  jobs would be green-but-unrequired until someone also edited the protection
+  rules. CI still does not run `npm run build`, but `typecheck` now covers the
+  part of it that used to break.
+- **CI installs with `npm ci`**, not `npm install` — it installs exactly what
+  `package-lock.json` pins and fails if the lockfile and `package.json`
+  disagree, instead of silently resolving new versions inside CI.
 - **Coverage path is load-bearing**: CI uploads `coverage/coverage-final.json` to
   Codecov. Verify that exact path still exists after any Vitest major.
 
diff --git a/package.json b/package.json
index 3faf4a4..f404496 100644
--- a/package.json
+++ b/package.json
@@ -10,6 +10,7 @@
     "build": "tsx src/lib/providers/ministry-platform/scripts/build-sql-install.ts && next build",
     "start": "next start",
     "lint": "eslint .",
+    "typecheck": "tsc --noEmit",
     "test": "vitest",
     "test:run": "vitest run",
     "test:coverage": "vitest run --coverage",
diff --git a/src/app/(web)/tools/addeditfamily/actions.test.ts b/src/app/(web)/tools/addeditfamily/actions.test.ts
index 67a80b2..07bb76c 100644
--- a/src/app/(web)/tools/addeditfamily/actions.test.ts
+++ b/src/app/(web)/tools/addeditfamily/actions.test.ts
@@ -382,3 +382,135 @@ describe("addeditfamily actions", () => {
     });
   });
 });
+
+/**
+ * Runtime validation of the saveFamily payload.
+ *
+ * `saveFamily(household: Household)` is a server action — a public POST
+ * endpoint whose TypeScript annotation is erased at runtime. Downstream,
+ * `FamilyService` interpolates `envelopeNo` and `donorId` into MP `$filter`
+ * strings and uses `donorId` to target a `Donors` update, so the payload has
+ * to be parsed, not merely typed.
+ *
+ * See `.claude/TODO/2026-09-13-unvalidated-envelope-donor-ids-in-filter.md`.
+ */
+describe("saveFamily payload validation", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockRequireSecurityRole.mockResolvedValue(42);
+  });
+
+  /** Bypass the compile-time type the way a hand-crafted POST body does. */
+  function malformed(household: Record<string, unknown>): Household {
+    return household as unknown as Household;
+  }
+
+  it("rejects a filter-injection-shaped envelopeNo before it reaches the service", async () => {
+    const household = makeHousehold();
+    const bad = malformed({
+      ...household,
+      members: [{ ...household.members[0], envelopeNo: "1 OR 1=1" }],
+    });
+
+    const result = await saveFamily(bad);
+
+    expect(result.success).toBe(false);
+    expect(mockSaveHousehold).not.toHaveBeenCalled();
+  });
+
+  it("rejects a filter-injection-shaped donorId before it reaches the service", async () => {
+    const household = makeHousehold();
+    const bad = malformed({
+      ...household,
+      members: [{ ...household.members[0], donorId: "5; DROP" }],
+    });
+
+    const result = await saveFamily(bad);
+
+    expect(result.success).toBe(false);
+    expect(mockSaveHousehold).not.toHaveBeenCalled();
+  });
+
+  it.each([
+    ["a non-integer envelopeNo", { envelopeNo: 1.5 }],
+    ["an array envelopeNo", { envelopeNo: [7] }],
+    ["an object donorId", { donorId: { id: 7 } }],
+    ["a boolean contactId", { contactId: true }],
+  ])("rejects %s", async (_label, patch) => {
+    const household = makeHousehold();
+    const bad = malformed({
+      ...household,
+      members: [{ ...household.members[0], ...patch }],
+    });
+
+    const result = await saveFamily(bad);
+
+    expect(result.success).toBe(false);
+    expect(mockSaveHousehold).not.toHaveBeenCalled();
+  });
+
+  it("rejects a household missing required top-level fields", async () => {
+    const result = await saveFamily(malformed({ householdId: 1 }));
+
+    expect(result.success).toBe(false);
+    expect(mockSaveHousehold).not.toHaveBeenCalled();
+  });
+
+  it("reports offending field paths so the user can fix the form", async () => {
+    const household = makeHousehold();
+    const bad = malformed({
+      ...household,
+      members: [{ ...household.members[0], envelopeNo: "1 OR 1=1" }],
+    });
+
+    const result = await saveFamily(bad);
+
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    expect(result.error).toMatch(/members\.0\.envelopeNo/);
+  });
+
+  it("never echoes the submitted value back in the error message", async () => {
+    // CLAUDE.md rule 14: error messages travel further than logs do. Paths
+    // are safe to report; the value that was rejected is not.
+    const household = makeHousehold();
+    const bad = malformed({
+      ...household,
+      members: [
+        { ...household.members[0], envelopeNo: "1 OR 1=1", emailAddress: "secret@example.com" },
+      ],
+    });
+
+    const result = await saveFamily(bad);
+
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    expect(result.error).not.toContain("1 OR 1=1");
+    expect(result.error).not.toContain("secret@example.com");
+  });
+
+  it("still authorizes before validating, so an unauthorized caller learns nothing about the schema", async () => {
+    mockRequireSecurityRole.mockRejectedValueOnce(new Error("Forbidden"));
+
+    const result = await saveFamily(malformed({ householdId: 1 }));
+
+    expect(result).toEqual({ success: false, error: "Forbidden" });
+    expect(mockSaveHousehold).not.toHaveBeenCalled();
+  });
+
+  it("passes a well-formed household straight through to the service", async () => {
+    const household = makeHousehold();
+    const progress: SaveProgress = {
+      mainAddressId: 1,
+      altAddressId: null,
+      householdId: 1,
+      members: [],
+    };
+    mockSaveHousehold.mockResolvedValueOnce(progress);
+
+    const result = await saveFamily(household);
+
+    expect(result).toEqual({ success: true, progress });
+    expect(mockSaveHousehold).toHaveBeenCalledTimes(1);
+  });
+});
diff --git a/src/app/(web)/tools/addeditfamily/actions.ts b/src/app/(web)/tools/addeditfamily/actions.ts
index af7abfa..6c36b17 100644
--- a/src/app/(web)/tools/addeditfamily/actions.ts
+++ b/src/app/(web)/tools/addeditfamily/actions.ts
@@ -3,6 +3,7 @@
 import { FamilyService, PartialSaveError } from "@/services/familyService";
 import { AuthorizationService } from "@/services/authorizationService";
 import { GooglePlacesService } from "@/services/googlePlacesService";
+import { HouseholdSchema } from "@/lib/dto/family";
 import type {
   ContactSearchResult,
   FamilyDefaults,
@@ -139,8 +140,36 @@ export async function saveFamily(
 ): Promise<{ success: true; progress: SaveProgress } | ActionError> {
   try {
     await requireAccess("Households", "update");
+
+    /**
+     * Parse before the payload reaches the service.
+     *
+     * `household: Household` is a compile-time annotation only — this is a
+     * server action, i.e. a public POST endpoint, and the types are erased at
+     * runtime. Fields from this object are interpolated into MP `$filter`
+     * strings and used to target `Donors`/`Contacts` updates downstream, so
+     * "it is typed `number`" is not a runtime guarantee of anything.
+     *
+     * The error deliberately reports field PATHS only, never the submitted
+     * values (CLAUDE.md rule 14: error messages travel further than logs).
+     */
+    const parsed = HouseholdSchema.safeParse(household);
+    if (!parsed.success) {
+      const paths = [
+        ...new Set(
+          parsed.error.issues.map((issue) =>
+            issue.path.length > 0 ? issue.path.join(".") : "(root)",
+          ),
+        ),
+      ];
+      return {
+        success: false,
+        error: `Invalid family data. Check these fields: ${paths.join(", ")}`,
+      };
+    }
+
     const service = await FamilyService.getInstance();
-    const progress = await service.saveHousehold(household);
+    const progress = await service.saveHousehold(parsed.data);
     return { success: true, progress };
   } catch (error) {
     if (error instanceof PartialSaveError) {
diff --git a/src/app/(web)/tools/addeditfamily/page.test.tsx b/src/app/(web)/tools/addeditfamily/page.test.tsx
index 301a751..d1c6d4d 100644
--- a/src/app/(web)/tools/addeditfamily/page.test.tsx
+++ b/src/app/(web)/tools/addeditfamily/page.test.tsx
@@ -133,10 +133,53 @@ describe("AddEditFamilyPage", () => {
     render(jsx);
 
     expect(screen.getByTestId("initial-contact-id").textContent).toBe("null");
-    expect(warnSpy).toHaveBeenCalledWith(
-      "Failed to resolve Contact_ID from page record:",
-      expect.any(Error),
+    // Identifiers and shape only — never the raw error, which can carry the
+    // interpolated $filter string (CLAUDE.md rule 14).
+    expect(warnSpy).toHaveBeenCalledWith("addeditfamily.resolve_contact_id_failed", {
+      table: "Households",
+      name: "Error",
+    });
+  });
+
+  it("never writes the raw error — or its message — to the log", async () => {
+    mockParseToolParams.mockResolvedValue({
+      recordID: 42,
+      pageData: {
+        Table_Name: "Households",
+        Primary_Key: "Household_ID",
+        Contact_ID_Field: "Contact_ID",
+      },
+    });
+    mockResolveContactIdFromPage.mockRejectedValue(
+      new Error("Invalid filter: Household_ID = 42 AND Secret = 'value'"),
     );
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+    render(await AddEditFamilyPage({ searchParams: searchParamsOf({ recordID: "42" }) }));
+
+    const logged = JSON.stringify(warnSpy.mock.calls);
+    expect(logged).not.toContain("Invalid filter");
+    expect(logged).not.toContain("Secret");
+  });
+
+  it("describes a non-Error throw by shape", async () => {
+    mockParseToolParams.mockResolvedValue({
+      recordID: 42,
+      pageData: {
+        Table_Name: "Households",
+        Primary_Key: "Household_ID",
+        Contact_ID_Field: "Contact_ID",
+      },
+    });
+    mockResolveContactIdFromPage.mockRejectedValue("a bare string");
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+    render(await AddEditFamilyPage({ searchParams: searchParamsOf({ recordID: "42" }) }));
+
+    expect(warnSpy).toHaveBeenCalledWith("addeditfamily.resolve_contact_id_failed", {
+      table: "Households",
+      name: "NonError",
+    });
   });
 
   it("passes params through to AddEditFamily", async () => {
diff --git a/src/app/(web)/tools/addeditfamily/page.tsx b/src/app/(web)/tools/addeditfamily/page.tsx
index 065e0ed..096736b 100644
--- a/src/app/(web)/tools/addeditfamily/page.tsx
+++ b/src/app/(web)/tools/addeditfamily/page.tsx
@@ -26,7 +26,13 @@ export default async function AddEditFamilyPage({ searchParams }: AddEditFamilyP
         params.pageData.Contact_ID_Field,
       );
     } catch (error) {
-      console.warn("Failed to resolve Contact_ID from page record:", error);
+      // Identifiers and shape only (CLAUDE.md rule 14). The raw error can
+      // carry the interpolated $filter string built inside
+      // resolveContactIdFromPage, and a $filter embeds record values.
+      console.warn("addeditfamily.resolve_contact_id_failed", {
+        table: params.pageData.Table_Name,
+        name: error instanceof Error ? error.name : "NonError",
+      });
     }
   }
 
diff --git a/src/components/address-labels/actions.test.ts b/src/components/address-labels/actions.test.ts
index 7192bac..f3dc4e3 100644
--- a/src/components/address-labels/actions.test.ts
+++ b/src/components/address-labels/actions.test.ts
@@ -669,3 +669,144 @@ describe('generateLabelPdf error branches', () => {
     if (!result.success) expect(result.error).toBe('PDF generation failed');
   });
 });
+
+/**
+ * Error-logging redaction.
+ *
+ * docxtemplater attaches the live merge scope to `err.properties.scope` when
+ * its scope parser fails. In this feature that scope IS the household list, so
+ * `console.error('mergeTemplate error:', error)` wrote every printable name
+ * and mailing address for the batch into server logs — exactly what CLAUDE.md
+ * rule 14 forbids.
+ *
+ * See `.claude/TODO/2026-09-13-mergetemplate-logs-address-pii-on-error.md`.
+ */
+describe('address-label actions redact errors before logging', () => {
+  const config: LabelConfig = {
+    stockId: '5160',
+    addressMode: 'household',
+    startPosition: 1,
+    includeMissingBarcodes: true,
+    barcodeFormat: 'postnet',
+    mailerId: '',
+    serviceType: '040',
+  };
+
+  const labels: LabelData[] = [
+    {
+      name: 'Jane Householder',
+      addressLine1: '742 Evergreen Terrace',
+      city: 'Springfield',
+      state: 'IL',
+      postalCode: '62704',
+    },
+  ];
+
+  /** Mirrors the shape docxtemplater throws on a scope-parser failure. */
+  function scopeParserError(): Error {
+    const error = new Error('Scope parser execution failed') as Error & {
+      properties?: Record<string, unknown>;
+    };
+    error.properties = {
+      id: 'scopeparser_execution_failed',
+      explanation: 'The tag {Name} failed to parse',
+      scope: labels.map((l) => ({
+        Name: l.name,
+        AddressLine1: l.addressLine1,
+        City: l.city,
+        State: l.state,
+        PostalCode: l.postalCode,
+      })),
+    };
+    return error;
+  }
+
+  function loggedText(): string {
+    const spy = console.error as unknown as { mock: { calls: unknown[][] } };
+    return JSON.stringify(spy.mock.calls);
+  }
+
+  beforeEach(() => {
+    mockRequireSecurityRole.mockResolvedValue(42);
+    mockGetSession.mockResolvedValue({ user: { id: 'user-1' } });
+  });
+
+  it('never writes the merge scope — household names and addresses — to the log', async () => {
+    mockDocxtemplaterRender.mockImplementationOnce(() => {
+      throw scopeParserError();
+    });
+
+    await mergeTemplate(Buffer.from('x').toString('base64'), labels, config);
+
+    const logged = loggedText();
+    expect(logged).not.toContain('Jane Householder');
+    expect(logged).not.toContain('742 Evergreen Terrace');
+    expect(logged).not.toContain('Springfield');
+    expect(logged).not.toContain('62704');
+
+    // The word "scope" legitimately appears inside the safe identifiers
+    // ("scopeparser_execution_failed"), so assert on the KEY, not the text:
+    // no `scope` property may survive into the logged object.
+    const spy = console.error as unknown as { mock: { calls: unknown[][] } };
+    const payload = spy.mock.calls.at(-1)?.[1] as Record<string, unknown>;
+    expect(payload).not.toHaveProperty('scope');
+    expect(Object.keys(payload).sort()).toEqual(['explanation', 'id', 'message', 'name']);
+  });
+
+  it('still logs the identifiers needed to diagnose the failure', async () => {
+    mockDocxtemplaterRender.mockImplementationOnce(() => {
+      throw scopeParserError();
+    });
+
+    await mergeTemplate(Buffer.from('x').toString('base64'), labels, config);
+
+    expect(console.error).toHaveBeenCalledWith('mergeTemplate error:', {
+      name: 'Error',
+      message: 'Scope parser execution failed',
+      id: 'scopeparser_execution_failed',
+      explanation: 'The tag {Name} failed to parse',
+    });
+  });
+
+  it('describes a non-Error throw by shape rather than value', async () => {
+    mockDocxtemplaterRender.mockImplementationOnce(() => {
+      // A thrown string could itself be attacker- or data-derived.
+      throw 'Jane Householder, 742 Evergreen Terrace';
+    });
+
+    await mergeTemplate(Buffer.from('x').toString('base64'), labels, config);
+
+    expect(console.error).toHaveBeenCalledWith('mergeTemplate error:', {
+      name: 'NonError',
+      type: 'string',
+    });
+    expect(loggedText()).not.toContain('Evergreen');
+  });
+
+  it('omits properties that are absent rather than logging undefined keys', async () => {
+    mockDocxtemplaterRender.mockImplementationOnce(() => {
+      throw new Error('plain failure');
+    });
+
+    await mergeTemplate(Buffer.from('x').toString('base64'), labels, config);
+
+    expect(console.error).toHaveBeenCalledWith('mergeTemplate error:', {
+      name: 'Error',
+      message: 'plain failure',
+    });
+  });
+
+  it('applies the same redaction in generateLabelPdf', async () => {
+    mockToBlob.mockRejectedValueOnce(scopeParserError());
+
+    await generateLabelPdf(labels, config);
+
+    const logged = loggedText();
+    expect(logged).not.toContain('Jane Householder');
+    expect(logged).not.toContain('Evergreen');
+    expect(console.error).toHaveBeenCalledWith(
+      'generateLabelPdf error:',
+      expect.objectContaining({ id: 'scopeparser_execution_failed' }),
+    );
+  });
+});
diff --git a/src/components/address-labels/actions.ts b/src/components/address-labels/actions.ts
index 24f699c..7f509a0 100644
--- a/src/components/address-labels/actions.ts
+++ b/src/components/address-labels/actions.ts
@@ -141,6 +141,37 @@ export async function fetchAddressLabels(
   return { printable: [], skipped: [] };
 }
 
+/**
+ * Reduce a caught error to something safe to write to a log.
+ *
+ * CLAUDE.md rule 14: log identifiers and shape, never record content.
+ * `console.error('...', error)` serialises the whole object, and docxtemplater
+ * attaches the live merge scope to `err.properties.scope` on a scope-parser
+ * failure — in this feature that scope IS the household list, so the raw
+ * object carries every printable name and mailing address for the batch
+ * straight into server logs.
+ *
+ * docxtemplater's own `id` and `explanation` are useful for diagnosing which
+ * failure mode occurred and contain no caller data, so they are kept
+ * explicitly. Nothing else from `properties` is.
+ */
+function describeError(error: unknown): Record<string, string> {
+  if (!(error instanceof Error)) {
+    return { name: 'NonError', type: typeof error };
+  }
+
+  const described: Record<string, string> = { name: error.name, message: error.message };
+
+  const properties = (error as { properties?: unknown }).properties;
+  if (properties && typeof properties === 'object') {
+    const { id, explanation } = properties as { id?: unknown; explanation?: unknown };
+    if (typeof id === 'string') described.id = id;
+    if (typeof explanation === 'string') described.explanation = explanation;
+  }
+
+  return described;
+}
+
 export async function generateLabelPdf(
   labels: LabelData[],
   config: LabelConfig
@@ -186,7 +217,7 @@ export async function generateLabelPdf(
 
     return { success: true, data: base64 };
   } catch (error) {
-    console.error('generateLabelPdf error:', error);
+    console.error('generateLabelPdf error:', describeError(error));
     return {
       success: false,
       error: error instanceof Error ? error.message : 'PDF generation failed',
@@ -229,7 +260,7 @@ export async function generateLabelDocx(
 
     return { success: true, data: base64 };
   } catch (error) {
-    console.error('generateLabelDocx error:', error);
+    console.error('generateLabelDocx error:', describeError(error));
     return {
       success: false,
       error: error instanceof Error ? error.message : 'Word generation failed',
@@ -325,7 +356,7 @@ export async function mergeTemplate(
 
     return { success: true, data: base64 };
   } catch (error) {
-    console.error('mergeTemplate error:', error);
+    console.error('mergeTemplate error:', describeError(error));
     const message = error instanceof Error ? error.message : 'Template merge failed';
     if (message.includes('tag')) {
       return { success: false, error: `Template error: ${message}. Check that merge tokens are correctly formatted.` };
diff --git a/src/components/field-management/use-field-order-state.test.ts b/src/components/field-management/use-field-order-state.test.ts
index 54e97b6..2eb3c0e 100644
--- a/src/components/field-management/use-field-order-state.test.ts
+++ b/src/components/field-management/use-field-order-state.test.ts
@@ -383,27 +383,89 @@ describe('useFieldOrderState > removeGroup', () => {
     expect(result.current.isDirty).toBe(true);
   });
 
-  it('is a no-op on groupedFields when the group still has fields, but still removes it from groupOrder', () => {
+  /**
+   * Regression: removing a non-empty group used to strip its name from
+   * `groupOrder` while leaving its fields in `groupedFields`. Because
+   * `buildSavePayload()` iterates `groupOrder`, those fields silently
+   * disappeared from the payload — live Ministry Platform page configuration
+   * lost with no error shown.
+   * See `.claude/TODO/2026-09-13-removegroup-order-guard-mismatch.md`.
+   */
+  it('is a complete no-op when the group still has fields', () => {
     const fields: PageField[] = [
       makeField({ Page_Field_ID: 1, Field_Name: 'A', Group_Name: '1 - First', View_Order: 1 }),
     ];
     const { result } = renderHook(() => useFieldOrderState(fields));
+    const orderBefore = result.current.groupOrder;
 
     act(() => {
       result.current.removeGroup('1 - First');
     });
 
-    // Documented actual behavior: groupedFields keeps the non-empty group (guard
-    // in removeGroup refuses to drop its fields), but groupOrder unconditionally
-    // filters the name out regardless of that guard — see filed TODO
-    // 2026-09-13-removegroup-order-guard-mismatch.md.
+    // Both halves of the state stay intact, not just groupedFields.
     expect(result.current.groupedFields['1 - First']).toEqual([1]);
-    expect(result.current.groupOrder).not.toContain('1 - First');
+    expect(result.current.groupOrder).toEqual(orderBefore);
+    expect(result.current.groupOrder).toContain('1 - First');
+  });
+
+  it("keeps a non-empty group's fields in the save payload after a refused removal", () => {
+    const fields: PageField[] = [
+      makeField({ Page_Field_ID: 1, Field_Name: 'A', Group_Name: '1 - First', View_Order: 1 }),
+      makeField({ Page_Field_ID: 2, Field_Name: 'B', Group_Name: '1 - First', View_Order: 2 }),
+    ];
+    const { result } = renderHook(() => useFieldOrderState(fields));
+
+    act(() => {
+      result.current.removeGroup('1 - First');
+    });
 
-    // Consequence: buildSavePayload iterates groupOrder, so field 1 is silently
-    // dropped from the save payload even though it still exists in groupedFields.
     const payload = result.current.buildSavePayload();
-    expect(payload.find((p) => p.Field_Name === 'A')).toBeUndefined();
+    expect(payload.map((p) => p.Field_Name).sort()).toEqual(['A', 'B']);
+  });
+
+  it('does not mark the form dirty when the removal was refused', () => {
+    // A refused removal changed nothing, so there is nothing to save.
+    const fields: PageField[] = [
+      makeField({ Page_Field_ID: 1, Field_Name: 'A', Group_Name: '1 - First', View_Order: 1 }),
+    ];
+    const { result } = renderHook(() => useFieldOrderState(fields));
+
+    act(() => {
+      result.current.removeGroup('1 - First');
+    });
+
+    expect(result.current.isDirty).toBe(false);
+  });
+
+  it('still removes a group once its fields have been moved out', () => {
+    const fields: PageField[] = [
+      makeField({ Page_Field_ID: 1, Field_Name: 'A', Group_Name: '1 - First', View_Order: 1 }),
+    ];
+    const { result } = renderHook(() => useFieldOrderState(fields));
+
+    act(() => {
+      result.current.removeGroup('1 - First');
+    });
+    expect(result.current.groupOrder).toContain('1 - First');
+
+    // Empty it the way the UI does (drag the field elsewhere), then retry.
+    act(() => {
+      result.current.moveHiddenToOther();
+    });
+    act(() => {
+      result.current.updateField(1, { Hidden: true });
+    });
+    act(() => {
+      result.current.moveHiddenToOther();
+    });
+    act(() => {
+      result.current.removeGroup('1 - First');
+    });
+
+    expect(result.current.groupOrder).not.toContain('1 - First');
+    expect(result.current.groupedFields['1 - First']).toBeUndefined();
+    // The field itself survived the group's removal.
+    expect(result.current.buildSavePayload().map((p) => p.Field_Name)).toContain('A');
   });
 
   it('is a no-op when the group name does not exist in groupedFields at all', () => {
diff --git a/src/components/field-management/use-field-order-state.ts b/src/components/field-management/use-field-order-state.ts
index fc59666..084f8cc 100644
--- a/src/components/field-management/use-field-order-state.ts
+++ b/src/components/field-management/use-field-order-state.ts
@@ -120,15 +120,36 @@ export function useFieldOrderState(fields: PageField[]): FieldOrderState {
     [isFlat]
   );
 
-  const removeGroup = useCallback((name: string) => {
-    setGroupedFields((prev) => {
-      if ((prev[name] || []).length > 0) return prev;
-      const { [name]: _, ...rest } = prev;
-      return rest;
-    });
-    setGroupOrder((prev) => prev.filter((g) => g !== name));
-    setIsDirty(true);
-  }, []);
+  /**
+   * Remove an empty group.
+   *
+   * Both pieces of state must agree. `buildSavePayload()` iterates
+   * `groupOrder`, not `Object.keys(groupedFields)`, so dropping a name from
+   * the order while its fields remain in `groupedFields` silently omits those
+   * fields from the payload — they vanish from the Ministry Platform page's
+   * field list on the next save, with no error shown to the admin.
+   *
+   * The guard therefore lives on the shared decision, not inside one setter.
+   * A non-empty group is a no-op: the UI only renders the delete control for
+   * empty groups, but this is a plain callback with no such enforcement.
+   */
+  const removeGroup = useCallback(
+    (name: string) => {
+      // Decide once, up front, from the rendered state — NOT inside one of the
+      // setters. A flag set inside a `setGroupedFields` updater cannot drive
+      // the `setGroupOrder` call: React runs updaters during render, so the
+      // flag is still false when the second setter is reached.
+      if ((groupedFields[name] || []).length > 0) return;
+
+      setGroupedFields((prev) => {
+        const { [name]: _, ...rest } = prev;
+        return rest;
+      });
+      setGroupOrder((prev) => prev.filter((g) => g !== name));
+      setIsDirty(true);
+    },
+    [groupedFields],
+  );
 
   const moveHiddenToOther = useCallback(() => {
     setGroupedFields((prev) => {
diff --git a/src/services/familyService.test.ts b/src/services/familyService.test.ts
index 908f1e9..447912a 100644
--- a/src/services/familyService.test.ts
+++ b/src/services/familyService.test.ts
@@ -804,3 +804,151 @@ describe("FamilyService", () => {
     });
   });
 });
+
+/**
+ * Defence in depth at the point the `$filter` string is built.
+ *
+ * `saveFamily` parses the payload with `HouseholdSchema` before it gets here,
+ * so these values should never be malformed in practice. This layer exists
+ * because `resolveUniqueEnvelopeNo` interpolates them into a raw filter and
+ * `upsertDonor` uses `donorId` to target which `Donors` row gets written — a
+ * future caller that skips the action, or a schema change that loosens a
+ * field, must not silently reopen that.
+ *
+ * See `.claude/TODO/2026-09-13-unvalidated-envelope-donor-ids-in-filter.md`.
+ */
+describe("FamilyService envelope/donor filter hardening", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+     
+    (FamilyService as any).instance = undefined;
+    mockRequireSecurityRole.mockResolvedValue(42);
+    mockGetDomainInfo.mockResolvedValue({ TimeZoneName: "Eastern Standard Time" });
+  });
+
+  /** Bypass the compile-time type the way an unvalidated caller would. */
+  function donorMember(overrides: Record<string, unknown>) {
+    return {
+      contactId: 900,
+      firstName: "Ada",
+      middleName: "",
+      maidenName: "",
+      lastName: "Lovelace",
+      nickname: "",
+      prefixId: 0,
+      suffixId: 0,
+      birthDate: null,
+      genderId: 0,
+      maritalStatusId: 0,
+      mobilePhone: "",
+      emailAddress: "",
+      bulkEmailOpt: false,
+      envelopeNo: 1001,
+      contactStatusId: 1,
+      primaryLanguageId: null,
+      faithBackgroundId: null,
+      householdPositionId: 1,
+      participant: null,
+      donorId: null,
+      isDonor: true,
+      ...overrides,
+    };
+  }
+
+  function householdWith(overrides: Record<string, unknown>): Household {
+    return makeHousehold({
+      householdId: 601,
+      members: [donorMember(overrides)],
+       
+    } as any) as unknown as Household;
+  }
+
+  async function saveAndCatch(household: Household): Promise<Error> {
+    const service = await FamilyService.getInstance();
+    try {
+      await service.saveHousehold(household);
+    } catch (error) {
+      return error as Error;
+    }
+    throw new Error("expected saveHousehold to reject");
+  }
+
+  it.each([
+    ["a filter-injection string", "1 OR 1=1"],
+    ["a non-integer", 1.5],
+    ["Infinity", Number.POSITIVE_INFINITY],
+    ["NaN", Number.NaN],
+  ])("refuses to build a Donors filter from %s as envelopeNo", async (_label, envelopeNo) => {
+    const error = await saveAndCatch(householdWith({ envelopeNo }));
+
+    expect(error).toBeInstanceOf(PartialSaveError);
+
+    // The malformed value must never have reached an MP query.
+    const donorQueries = mockGetTableRecords.mock.calls.filter(
+      (call) => call[0]?.table === "Donors" && typeof call[0]?.filter === "string",
+    );
+    for (const [args] of donorQueries) {
+      expect(args.filter).not.toContain(String(envelopeNo));
+    }
+  });
+
+  it("refuses to build a Donors filter from an injection-shaped donorId", async () => {
+    const error = await saveAndCatch(
+      householdWith({ envelopeNo: 1001, donorId: "7 OR 1=1" }),
+    );
+
+    expect(error).toBeInstanceOf(PartialSaveError);
+    const donorQueries = mockGetTableRecords.mock.calls.filter(
+      (call) => call[0]?.table === "Donors",
+    );
+    for (const [args] of donorQueries) {
+      expect(String(args.filter ?? "")).not.toContain("OR 1=1");
+    }
+    expect(mockUpdateTableRecords).not.toHaveBeenCalledWith(
+      "Donors",
+      expect.anything(),
+      expect.anything(),
+    );
+  });
+
+  it("still treats donorId 0 as 'no existing donor' rather than rejecting it", async () => {
+    // 0 is the historical "none" sentinel alongside null; validation must not
+    // turn it into an error.
+    mockGetTableRecords.mockResolvedValueOnce([]); // no envelope conflict
+    mockCreateTableRecords
+      .mockResolvedValueOnce([{ Address_ID: 501 }])
+      .mockResolvedValueOnce([{ Contact_ID: 900 }])
+      .mockResolvedValueOnce([{ Donor_ID: 901 }]);
+
+    const service = await FamilyService.getInstance();
+    const household = householdWith({ envelopeNo: 1001, donorId: 0, contactId: 900 });
+
+    await service.saveHousehold(household).catch(() => {
+      // Later stages of the save are not the subject here; what matters is
+      // that validation did not reject donorId: 0 up front.
+    });
+
+    const donorFilters = mockGetTableRecords.mock.calls
+      .filter((call) => call[0]?.table === "Donors")
+      .map((call) => call[0].filter as string);
+    expect(donorFilters.some((f) => f === "Envelope_No = 1001")).toBe(true);
+  });
+
+  it("builds the exact expected filter for well-formed values", async () => {
+    mockGetTableRecords.mockResolvedValueOnce([]);
+    mockCreateTableRecords
+      .mockResolvedValueOnce([{ Address_ID: 501 }])
+      .mockResolvedValueOnce([{ Contact_ID: 900 }]);
+    mockUpdateTableRecords.mockResolvedValue([{}]);
+
+    const service = await FamilyService.getInstance();
+    await service
+      .saveHousehold(householdWith({ envelopeNo: 1001, donorId: 77, contactId: 900 }))
+      .catch(() => {});
+
+    const donorFilters = mockGetTableRecords.mock.calls
+      .filter((call) => call[0]?.table === "Donors")
+      .map((call) => call[0].filter as string);
+    expect(donorFilters).toContain("Envelope_No = 1001 AND Donor_ID <> 77");
+  });
+});
diff --git a/src/services/familyService.ts b/src/services/familyService.ts
index f3b9e98..ba24c8c 100644
--- a/src/services/familyService.ts
+++ b/src/services/familyService.ts
@@ -688,8 +688,26 @@ export class FamilyService {
     requested: number,
     excludeDonorId: number | null,
   ): Promise<{ envelopeNo: number; bumped: boolean }> {
+    // Both values land in a raw $filter string below. They originate from a
+    // client-supplied Household, and a server action is a public POST endpoint
+    // whose TypeScript types are erased at runtime — so `number` here proves
+    // nothing. The action parses with HouseholdSchema; this is the second
+    // layer, at the point where the string is actually built.
+    validatePositiveInt(requested);
+    // 0 and null both mean "no donor to exclude" and are handled by the
+    // ternary below, so only a value that will actually be interpolated is
+    // required to be a positive integer.
+    if (excludeDonorId !== null && excludeDonorId !== 0) {
+      validatePositiveInt(excludeDonorId);
+    }
+
     let candidate = requested;
     for (let attempt = 0; attempt < 5; attempt++) {
+      // `candidate` is either `requested` (validated above) or the result of
+      // getNextEnvelopeNumber(), which is derived server-side — but validate
+      // each iteration anyway so no future change to that method can quietly
+      // reintroduce an unchecked value here.
+      validatePositiveInt(candidate);
       const filter =
         excludeDonorId && excludeDonorId > 0
           ? `Envelope_No = ${candidate} AND Donor_ID <> ${excludeDonorId}`
@@ -726,6 +744,8 @@ export class FamilyService {
     }
 
     if (existingDonorId && existingDonorId > 0) {
+      // Client-supplied, and it selects which Donors row gets written.
+      validatePositiveInt(existingDonorId);
       await this.mp!.updateTableRecords(
         "Donors",
         [{ Donor_ID: existingDonorId, Envelope_No: finalEnvelopeNo }],