Skip to content

fix(lead): tombstone a confirmed duplicate claim when its survivor is erased (#1164) - #1175

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-1164-duplicate-survivor-tombstone
Aug 16, 2026
Merged

fix(lead): tombstone a confirmed duplicate claim when its survivor is erased (#1164)#1175
os-zhuang merged 1 commit into
mainfrom
claude/issue-1164-duplicate-survivor-tombstone

Conversation

@os-zhuang

@os-zhuang os-zhuang commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #1164

Lets a GDPR erasure complete against a lead a reviewer already closed as a
confirmed duplicate, without deleting the verdict they recorded.

Deleting a Contact — or the Account above it, which cascades — was refused
whenever any lead had been disqualified as a duplicate of that record:

DELETE /api/v1/data/crm_contact/{id}
→ 400 "Disqualifying a lead as Duplicate requires naming the surviving record
        and setting Duplicate Status to Confirmed"

This is the drain path of the Suspected Duplicates review queue, so every lead a
reviewer ever closed as a confirmed duplicate held its survivor hostage. #1072
cleared the same wall for leads still carrying the machine's suspected guess;
duplicate_disqualification_requires_survivor (#598) is the second, independent
rule sitting on the same path.

Implements the maintainer decision of 2026-08-16 on the card — constraint (b)
"no new vocabulary" yields — in the third-duplicate_of_type-value shape, not
the duplicate_status shape.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change
  • Documentation update
  • Code refactoring
  • Performance improvement
  • CI/CD update

Related Issues

Fixes #1164
Related to #1072, #1166, #598, #688, #1149

Changes Made

  • src/objects/_picklists.ts — the duplicate_of_type vocabulary, declared as
    two sets: DUPLICATE_OF_TYPE_AUTHORABLE_OPTIONS (the two object names a
    human may pick) and DUPLICATE_OF_TYPE_OPTIONS (those plus the tombstone
    erased). The object spreads the full set, the form spreads the authorable
    one, so the tombstone is unpickable by construction rather than by a
    hand-kept exclusion list.
  • src/objects/lead.hook.tslead_duplicate_check job 1c splits the
    retirement on what the record already says: suspected (or no opinion) is
    still retired whole; a human's confirmed verdict is tombstoned — the
    type becomes erased, the status stands.
  • src/views/lead.view.ts — the shared duplicate-link block narrows the
    picker's options. All eight form surfaces inherit it.
  • src/translations/{en,zh-CN,ja-JP,es-ES}.ts — the value is labelled in all
    four locales. Unauthorable is not the same as invisible: a tombstoned lead
    reads "Erased Record", not a raw enum.
  • content/docs/sales/leads.mdx — a new "When the surviving record is later
    deleted" section.
  • tests + changeset.

Why nothing was relaxed, and why that is the whole design

A validation is evaluated against {...previous, ...data} and cannot see a
transition
, so on the record "the pointer was erased" and "this claim never
named anyone" were the same state. Every predicate taught to tolerate the
first also admits the second, and #598 dies with it. That is why the three
shapes measured in the closed PR #1172 all failed.

A distinct value makes the two states different facts, and then every
existing rule stands unedited:

pin status
the lookups' requiredWhen pairing green, unedited — pairs only on crm_lead / crm_contact, so it never fires on a tombstone
duplicate_disqualification_requires_survivor (#598) green, unedited — wants a non-blank type plus confirmed; a tombstoned lead has both
#688 form/server agreement (view-predicate-dialect.test.ts:414) green, unedited — no predicate changed, visibleOn still byte-identical to requiredWhen
declarative enforcement, hook-free rig (lead-duplicate-management.test.ts:142+) green, unedited
lead.hook.ts contains no disqualification_reason (:295) green, unedited — the hook scopes on duplicate_status, which it already reads and writes

Two pins the card's table did not list did need updating, and both are
vocabulary inventories rather than rules:
links through a type discriminator… and %s labels the picklist VALUES… each enumerated
duplicate_of_type's values as exactly crm_lead / crm_contact. Those are
the assertions that record the vocabulary, so they are precisely what the
decision to yield "no new vocabulary" moves. Neither is a predicate, a rule or a
doctrine pin. Both were re-derived from _picklists.ts rather than re-spelled
by hand, and the first was strengthened from a flat three-value list into the
structural property that actually matters: every authorable value is backed by a
lookup targeting it, and the tombstone is backed by none.

Keeping the tombstone written, never authored

  • Not authorable. The lead forms offer only the authorable set. Pinned, with
    the walk asserting it found the picker at all so the pin cannot go vacuously
    green.
  • One writer. erased is spelled in exactly two lines of src/ — its
    declaration and the single line that stamps it. Pinned by a source scan over
    the whole tree with comments stripped, so it fails on a flow, action or hook
    nobody thought to test, and on a second stamp inside lead.hook.ts itself.
  • Reachability narrowed further than the brief asked. The tombstone requires
    the write to be silent about duplicate_status. The engine's set_null
    cleanup arrives as exactly { id, link: null, updated_at, updated_by }; a
    caller that names the status is hand-managing the claim, and that claim is
    retired whole. No payload can manufacture a tombstone by supplying its own
    confirmed.

Residual, as the decision accepted: the value is declared, so a direct API write
can still set it. That is not closable declaratively — a rule refusing it
would refuse the cleanup's own write, since neither can see a transition. The
hole is visible and labelled rather than invisible.

Testing

  • Unit tests pass
  • Linting passes
  • Build succeeds
  • Manual testing completed — no browser pass; the delete paths are driven
    end-to-end against a real ObjectKernel instead
  • New tests added

Local gate union, all at final head 07a39310:

typecheck   0
hygiene     0   source hygiene clean
validate    0   (pre-existing colSpan warnings only)
lint        0   83 warning(s), 10 suggestion(s) — unchanged from main
i18n-gate   0   0 i18n/missing-* issues
build       0   Artifact: dist/objectstack.json (1993.6 KB)
test        0   Test Files 106 passed (106)
                Tests 2605 passed | 1 skipped (2606)

Both delete paths are covered: the contact directly, and the account cascading
through it via the crm_contact.crm_account master-detail.

Not run: pnpm test:e2e (Playwright, e2e.yml) — untouched by this change;
codeql.yml, link-check.yml, docs-app.yml, changeset-check.yml,
labeler.yml, label-sync.yml. Per #1149, pnpm verify is not the full CI gate
set, so the list above is what was actually run rather than a claim about CI.

Reverse verification — five ablations

Each was committed first, then mutated, run, restored with
git checkout {branch} -- {path} and confirmed byte-identical
(git diff HEAD --quiet clean every time).

ablation direction result
tombstone branch removed (always retire whole) red 6 redDisqualifying a lead as Duplicate requires naming the surviving record and setting Duplicate Status to Confirmed, the card's reported symptom reproduced verbatim
form narrowing removed red 1 reda lead form offers duplicate_of_type without narrowing its options — the erased tombstone is pickable there
'duplicate_status' in input silence guard removed red 1 redexpected 'erased' to be null; a hand-managed write manufactures a tombstone
a second stamp added inside lead.hook.ts red 1 redexpected 2 occurrences … but got 3, both offending lines printed
erased removed from the object's declared options red 6 redDuplicate Of must be one of: crm_lead…

The last one is the informative one and was run to answer a question rather than
to confirm a guess: it measures that the object-level option list is
enforced, not decorative. The engine rejects an undeclared select value, so
the tombstone genuinely has to be declared — which is also why the API-side
authorability residual above cannot be closed by omitting it.

Additional Notes

Closed PR #1172 is the evidence for the road not taken; nothing from that branch
is reused here — this is a fresh branch off origin/main.

Machine guesses are still retired whole, exactly as #1072 / #1166 decided. Only
a human's confirmed verdict is tombstoned.


Generated by Claude Code

… erased (#1164)

Deleting a contact — or the account above it, which cascades — was refused
whenever a lead had been disqualified as a confirmed duplicate of that record:

    DELETE /api/v1/data/crm_contact/<id>
    → 400 "Disqualifying a lead as Duplicate requires naming the surviving
            record and setting Duplicate Status to Confirmed"

#1072 cleared the same wall for leads carrying the machine's `suspected` guess.
`duplicate_disqualification_requires_survivor` (#598) is a second, independent
rule on the same path, and it is the drain path of the review queue: every lead
a reviewer ever closed as a confirmed duplicate held its survivor hostage.

`crm_lead.duplicate_of_type` gains a third value, `erased`, and
`lead_duplicate_check` splits the retirement on what the record already says: a
`suspected` guess is still retired whole, while a human's `confirmed` verdict is
tombstoned — the type becomes `erased`, the status stands, and the lead goes on
saying "confirmed duplicate of a record that has since been erased".

Nothing is relaxed, which is the point. A validation is evaluated against
`{...previous, ...data}` and cannot see a transition, so "the pointer was
erased" and "this claim never named anyone" were the same state; a distinct
value makes them different facts. The `requiredWhen` pairing fires only on
`crm_lead` / `crm_contact` so it never sees the tombstone, and #598's rule wants
a non-blank type plus `confirmed`, both of which a tombstoned lead has.

The value is written, never authored: the lead forms offer only the authorable
object types, and it is stamped from exactly one line, pinned by a source scan.
It stays labelled in all four locales so a tombstoned lead reads as "Erased
Record" rather than a raw enum.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019TYoxKa8yFLiDDh7tkBqtu
@vercel

vercel Bot commented Aug 16, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hotcrm Ignored Ignored Aug 16, 2026 6:22am

Request Review

@github-actions github-actions Bot added ci/cd CI plumbing and the verification pipeline metadata Declarative metadata — schema, security posture, UI surfaces backend Server-side behaviour — hooks, flows, actions labels Aug 16, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 16, 2026 06:25
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 16, 2026
Merged via the queue into main with commit 2342811 Aug 16, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Server-side behaviour — hooks, flows, actions ci/cd CI plumbing and the verification pipeline metadata Declarative metadata — schema, security posture, UI surfaces

Projects

None yet

2 participants