Follow-up to #496: behavior fixes and convention unification (tiers 3-4)

[yinlianghui]

#496's documentation drift and inert dead code were fixed in #504 (tiers 1–2). This issue tracks the remaining items, which change runtime behavior and each need a test proving the bug before the fix.

Tier 3 — behavior bugs

1. CSV export corrupts on real data (src/actions/global.actions.ts:161-162): values joined with , and no quoting/escaping — any description/notes field containing a comma, quote, or newline breaks the row (seed data guarantees this). Also: find() is unbounded, and headers come from records[0] only, silently dropping columns on heterogeneous rows.
2. record_label is null on 11 of 14 objects (global.actions.ts:36,101): reads ctx.record?.name, but most objects use a formula nameField (full_name, display_title, subject, …). Fix: resolve the object's declared nameField instead of hardcoding .name.
3. Unguarded validation predicates are silently skipped (product.object.ts:216,223, opportunity_line_item.object.ts:96): strict CEL aborts on null < int, so these rules never fire on empty fields — the same-named rule in quote_line_item.object.ts:106 has the guard. The hazard is documented at account.object.ts:244-245. Note: adding guards makes previously-passing data start failing; check seeds.
4. disqualification_reason promises "Required when status is Unqualified" with zero enforcement (lead.object.ts:269-282) — and zero writers, zero view/translation presence. ~25% of seeded leads are unqualified with the reason stuffed into free-text notes instead. Fix shape exists in-repo: case.object.ts:295-308 implements exactly this "required when status is X" pattern as a P validation.
5. Lead converted-record protection is implemented twice with different scope — decision needed: lead.object.ts:323-329 validation (4 identity fields, recoverable error) vs lead.hook.ts beforeUpdate guard (all fields, thrown error, and no ctx.user?.id guard — unlike the opportunity_lifecycle freeze-guard rejects seed re-writes on closed deals → 23 boot errors + closed-won probability stuck at 10% #459-hardened quote/opportunity freeze guards, so system/flow writes to a converted lead are blocked too). Options: (a) keep the hook, add the system-write guard, drop the narrow validation; (b) keep the validation, extend its field list, drop the hook; (c) keep both intentionally (UI-friendly error for the common fields, hard stop for the rest) and document it.
6. Dead readonly fields — decision needed: crm_case.first_response_date and crm_opportunity.created_date have no writer anywhere, and both are readonly: true, which 16.x makes unwritable even by hooks/flows (#2948). Options per field: remove it (case view / translations / opportunity.view.ts:138 startDateField need repointing), or drop readonly and wire a real writer.
7. revenue_positive duplicated with diverging wording/severity: account.object.ts:239-247 (validation, "must be positive") vs account.hook.ts (thrown error, "must be greater than or equal to 0"). Keep one; the messages also disagree about whether 0 is allowed (it is — both use < 0).
8. Formula fields authored with the P (predicate) tag instead of F: opportunity_line_item.object.ts:80, quote_line_item.object.ts:77,90 — :90 is also formula-on-formula (record.subtotal is itself a formula), warned against at lead.object.ts:61-64. campaign_member.object.ts:113 uses a raw {dialect:'cel'} object because the file never imports P.
9. task.hook.ts date handling: compares reminder.slice(0,10) > due with only one side sliced, and the handler derives the same effective due date twice with different treatment. Latent (due_date is a date field) but one of the two treatments is wrong.
10. Pending changeset consumed by nothing: .changeset/no-hardcoded-currency-format.md sits unreleased; no workflow runs changeset version/publish. Decide the release flow or fold the entry into CHANGELOG manually.

Tier 4 — convention unification (feature-shaped, larger diffs)

11. State machines: only lead/opportunity/case declare type: 'state_machine' transitions; quote/contract/campaign (and task/knowledge_article) have status fields with no transition map.
12. end_after_start operator drift: campaign uses < (allowing end == start, contradicting its own message), contract uses <=; forecast has a third variant (period_end_after_start).
13. fieldGroups missing on crm_campaign and crm_task (full detail-page objects; the two line-item objects are arguably fine without).
14. priority_rank default 1 (case) vs 2 (task) with the rank map duplicated in both hooks — unknown priorities sort differently per object. test/metadata-references.test.ts pins this area; check before changing.
15. Near-verbatim twins: opportunity_line_item.hook.ts price-fill hook is a byte-equivalent copy of the quote one (the rollup hooks share a skeleton but differ in real math — merge carefully or not at all); log_call/log_meeting in global.actions.ts differ only in prefix/metadata key, plus an undocumented asymmetry (duration required for calls, optional for meetings).
16. filter vs where: _hook-api.ts:19-25 accepts both; 10 hooks use filter, 2 use where, actions use a third update(doc, { where }) shape, and the test harness normalizes both so drift is invisible. Pick one.
17. Test gaps that let all of the above survive: 11 of 14 hooks have zero behavioral tests; actions are only regex-checked, never executed; and no test runs hooks under the real body-only sandbox — the harness calls handlers as plain closures, so a sandbox-breaking refactor stays green. The load-bearing conventions (body-only helpers, null guards, no formula-on-formula, system-write guard) live only in comments; consider lint rules or a sandbox-accurate test harness to enforce them.

🤖 Generated with Claude Code