feat(crm): enforce do_not_call on scheduled phone outreach

[os-steve]

Fixes #1180

crm_lead.do_not_call and crm_contact.do_not_call were declared, rendered, labelled in four locales — and read by nothing. Premise re-verified on main @ 2342811a: the consumer scan returned only the two Field.boolean declarations, zero enforcement sites. ADR-0049 enforce-or-remove, ruled enforce (maintainer, 2026-08-17: 「逐个 enforce-or-remove（推荐）」).

What this enforces

Two beforeInsert / beforeUpdate guards, one per object that can carry a forward-looking phone touch:

Guard	Refuses
task_do_not_call_guard (src/objects/task.hook.ts)	an open crm_task of type: 'call' parented to a flagged lead or contact
event_do_not_call_guard (src/objects/event.hook.ts)	a planned crm_event of type: 'call' against a flagged lead or contact

Both objects are covered, and both guards read the effective value (input falling back to previous), so the two single-key routes in — retyping an existing meeting task to call, and re-parenting an existing call task onto a flagged person — are closed too. Each is pinned by its own test.

The judgment call: log_call stays reachable, and so does every other record of a call

Scheduling a phone touch is refused; recording one that already happened never is. A completed Call task and a held Call event both pass the guards untouched, which is exactly what the log_call action writes (EVENT_STATUS = "held").

The reasoning, as the card asked me to state explicitly:

• Refusing a log hides the evidence of a call rather than preventing one. The call already happened; the only thing a refusal changes is whether there is a record of it.
• It would penalise the honest rep — the one who logs the call they arguably should not have made is precisely the person the refusal would block, while the rep who says nothing is unaffected.
• It would make the compliance story worse, not better: an auditor asking "was this flagged person called?" needs the row to exist.

So log_call carries no visible: gate and needs none — the status split in event_do_not_call_guard is what makes that carve-out principled rather than accidental. The test asserts both halves (no gate on the action, held on its body), so a later "consistency" pass cannot close it without reading why.

Two PM mechanism assumptions, both measured — one falsified

1. The visible: predicate route — rejected on architecture, not on mechanism. The predicate at src/actions/contact.actions.ts:121 does transfer structurally; Action.visible is on the generated activity actions too. But it is a rendering hint. It hides a button in the Console and leaves the write reachable over REST, from an AI tool call, from a flow, and from an import. For email_opt_out that is only half the story and the other half exists (campaign-enrollment.flow.ts filters on the flag, campaign_member.hook.ts writes it). do_not_call had no such backing, so a predicate alone would have moved the field from declared and unenforced to declared and unenforced unless you use the mouse — in the reference app every marketplace fork copies.

Enforcing on the write covers every entry point at once. That is the repo's own stated principle, quoted from src/actions/global.actions.ts: the recency bubble lives on crm_event's hook so that an event "created from the calendar, from an import, or from a future flow" behaves identically — "One writer, not one per entry point."

2. schedule_meeting is NOT phone outreach — the suggested route is falsified here. It hardcodes eventType: 'meeting' (global.actions.ts:490); the meeting kind is not a user choice, and its only location input is a free-text location. do_not_call is a promise about the phone, not do_not_contact: someone who will not take calls may still be met in person or over video, and email is governed by its own separate email_opt_out flag. Gating schedule_meeting would have silently widened a declared field into a promise the app never made — the mirror image of the defect being closed. It is deliberately left ungated, with a test pinning that.

3. schedule_followup needs no change, and the phone option cannot be gated. The card asked which the metadata can express. A screen field's options are static metadata (plainOptions(TASK_TYPE_OPTIONS)) evaluated with no record in scope — there is no expression in which "hide Call for THIS lead" can be written. Refusing at the point the phone type is chosen is what the metadata can express, and it costs the flow zero changes, because the flow reaches crm_task through the very write the guard sits on. A test pins that linkage so a refactor moving the flow off crm_task surfaces here.

Refusal evidence, and the #1075 wall

The card asked for code / status assertions. They are asserted at the level that is real, which is currently neither. Every business rejection in this app is a bare throw new Error(msg) (17 of them across src/objects/*.hook.ts); these two guards match that idiom deliberately rather than inventing a private error class for one field. #1075 (open) records that such rejections reach REST with code and status undefined — that is its job, not this card's.

So the test measures both properties instead of assuming them, and pins the message, which until #1075 lands is the entire contract a caller gets:

expect(err.code,   '#1075 has landed — tighten this assertion').toBeUndefined();
expect(err.status, '#1075 has landed — tighten this assertion').toBeUndefined();

Those toBeUndefined() calls are a deliberate tripwire, not an endorsement: when #1075 lands this test goes red and is the right place to strengthen.

Which code path the tests drive (#1167)

#1167 is partially falsified. It records that every guard assertion here drives the in-process handler rather than the shipped QuickJS path. That is true of the hook-harness.ts tests — it is not true of this file, and need not be: test/helpers/action-sandbox.ts already exports runHookBody, which lowers a hook via the CLI's extractHookBody and runs the resulting body.source through the same QuickJSScriptRunner + hookBodyRunnerFactory the runtime binds at boot. Every assertion here drives that shipped body. It matters for these two guards specifically: both are body-only closures over nothing, so a body reaching for an import or a module constant would be a runtime ReferenceError while a handler-level test stayed green.

Reverse verification

Two ablations, each predicted before running, both landing exactly as predicted:

Ablation	Predicted	Observed
neuter the task guard's flag check	task refusals red, event refusals green	5 red (4 task refusals + the #1075 probe), 12 green
effective() reads input only	only the two single-key update tests red	2 red, 15 green

Restored from the commit both times, tree confirmed clean against HEAD.

Docs

content/docs/sales/contacts.mdx stated the opposite of this change in three locales — "A marker only. Nothing in the product blocks a call, or the logging of a Call task…". Updated in en, zh-Hans and zh-Hant to state precisely what is now enforced, what is deliberately still allowed, and that the flag is phone-scoped.

Gates

Run at the final commit 5a7cb0cb, all green:

validate=0  typecheck=0  lint=0  i18n-gate=0
hygiene=0   stackblitz-lock=0    build=0     test:coverage=0

test:coverage — 107 files, 2624 passed, 1 skipped. Per #1149 the local pnpm verify is not the full CI set, so scripts/check-stackblitz-lock.mjs (ci.yml) and test:coverage (CI runs this, not test) were run too. test/runtime-coverage.test.ts caught the new hooks as untested and is updated to register this file in its ledger. Not run locally: e2e.yml (Playwright), codeql.yml, link-check.yml, docs-app.yml. This PR carries a changeset, so skip-changeset does not apply.

Scope note for the reviewer

The claim comment's declared file surface was src/actions/, src/flows/schedule-followup.flow.ts, test/. Measurement moved the correct fix out of that surface: the action-level predicate is not enforcement, and the only forward-looking phone surface in src/actions/ (schedule_meeting) turned out not to be phone outreach at all — so an in-surface fix could not have covered crm_contact at all, and the card's "both objects" requirement is non-negotiable. src/objects/task.hook.ts and src/objects/event.hook.ts are touched instead; both are unclaimed by the sibling batch cards (#1181 is src/objects/account.*, #1183 is scripts//CI), and #1182 is held to a later batch. Flagging rather than assuming — happy to re-shape if the surface boundary was meant to be hard.

Out of scope and deliberately left open: #1075 (rejections reach REST without code/status) and #1167 (handler-vs-shipped-path coverage) both remain open, are pre-existing, and are referenced above only as measurement context.

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
hotcrm	Ignored		Aug 17, 2026 2:25am