A lead flagged as a duplicate makes the contact (or lead) it duplicates undeletable: the same #696 / #711 construction on crm_lead's duplicate_of_* pairing #1072

Activity

1. huangyiirene

   added

   bugSomething isn't workingSomething isn't working

   metadataDeclarative metadata — schema, security posture, UI surfacesDeclarative metadata — schema, security posture, UI surfaces

   on Aug 11, 2026
2. os-zhuang

   added

   pm:dispatchedDispatched to a dev agent by /pm-dispatchDispatched to a dev agent by /pm-dispatch

   on Aug 16, 2026
3. os-zhuang

   self-assigned this

   on Aug 16, 2026
4. os-zhuang commented on Aug 16, 2026

   

   os-zhuang

   on Aug 16, 2026 – with Claude

   Contributor

   More actions

   Claim: PM loop round 1
   Session: session_019TYoxKa8yFLiDDh7tkBqtu
   Branch: claude/issue-1072-lead-duplicate-link-cleanup
   Worktree: hotcrm-issue-1072
   Domain: (hotcrm has no domain:* lane taxonomy — repo-wide seat)
   File surface: src/objects/lead.object.ts, src/objects/lead.hook.ts, test/ (stop on breach; explain in the report)
   Container & model: M, mode:subagent, model: opus
   Serial constraints cleared: no open PR and no in-flight claim touches src/objects/lead.*; the sibling dispatch this round (#1123) is confined to src/apps/crm.app.ts — disjoint.

   ---

   Triage: lands in hotcrm; rationale — the construction is entirely in this repo's authored metadata: a set_null lookup paired with a requiredWhen discriminator that the null leaves in breach. The engine is behaving as specified.

   Adjudicated in the delegated lane, not escalated. The card says "the discriminator-plus-status question is a product call, so this is filed rather than fixed" — on review that framing is narrower than it looks, and holding the whole card for it would leave a GDPR-shaped defect open over a sub-question:

   • Mechanical boundary test: restoring the ability to delete a contact does not widen an accepted set or a public surface. A record that no longer duplicates anything should stop claiming it does. That is declared = enforced restoration ⇒ bug fix ⇒ delegated lane.
   • Four prisms align: measured business pull ("delete this person" is blocked on the ordinary intake path, no conversion or user action needed); platform coherence (third appearance of the 报名过市场活动的线索/联系人无法删除:级联把 campaign_member 的引用置空,随即违反它自己的校验规则 #696 / 参加过会议的线索/联系人/用户同样无法删除:crm_event_attendee 与 #696 是同一构造的同一缺陷 #711 construction — a per-instance patch each time is the workaround, and the card's own option (1) is the coherent one); AI-agent error-resistance (clearing the discriminator keeps the requiredWhen honest, whereas the card's option (3) — loosening the rule — is exactly the lenient-consumer shape that hides authoring errors); startup scope discipline (no new capability).
   • Options (2) cascade and (3) loosening are rejected with the card's own reasoning: a lead is a first-class record, and weakening a rule that is correct at authoring time re-opens the drift the pairing closed.

   The one genuinely open sub-question is delegated to measurement, not guessed: whether clearing duplicate_of_type should also clear duplicate_status. Decide it by reading what duplicate_status means with no link left — if no view, filter, report or flow can act on suspected once the survivor is gone, it is stale state and goes; if something does read it, keep it and say what. Report the reading either way.

   If the measurement contradicts the premise — e.g. a platform change since 17.0.0-rc.6 already clears the discriminator, or the delete now succeeds — stop and report premise_still_valid: false. That is a valid delivery, not a failure.

   ---

   Generated by Claude Code
5. os-zhuang commented on Aug 16, 2026

   

   os-zhuang

   on Aug 16, 2026 – with Claude

   Contributor

   More actions

   {
     "issue": 1072,
     "status": "done",
     "branch": "claude/issue-1072-lead-duplicate-link-cleanup",
     "pr": "https://github.com/objectstack-ai/hotcrm/pull/1166",
     "premise_still_valid": true,
     "summary": "PM assumption 1 falsified in the safe direction: re-ran the card's probe on 17.0.0 GA and all four legs still reproduce (contact delete, account cascade, lead-lead, and the account above the contact), so the premise stands. PM assumption 2 verified: test/freeze-guard-reference-cleanup.test.ts does carry the #720 boundary test with the 'Duplicate Of Contact is required' assertion, and it is flipped here to a completed delete plus a cleared claim. Implemented the adjudicated option (1) in lead_duplicate_check: the discriminator may not outlive the lookup it names, so any beforeUpdate leaving the named lookup blank retires duplicate_of_type and duplicate_status in the same write. Two findings filed, plus one candidate falsified by measurement rather than filed.",
     "tests": "All runs serialized under flock /tmp/os-heavy-verify.lock. RED FIRST, on unmodified origin/main (fix committed first, then `git checkout origin/main -- src/objects/lead.hook.ts src/objects/lead.object.ts`, tests kept): `npx vitest run test/lead-duplicate-link-cleanup.test.ts test/freeze-guard-reference-cleanup.test.ts` => 'Test Files 2 failed (2) / Tests 10 failed | 42 passed (52)', including the flipped #720 boundary test and all six erasure-path cases. Restored with `git checkout <branch> -- <paths>`; `git diff HEAD --stat` empty afterwards, so the green numbers are from a tree byte-identical to the commit. GREEN: same two files plus lead-duplicate-management, converted-lead-guard, hooks-runtime-service => 'Test Files 5 passed (5) / Tests 202 passed (202)'. FULL GATE: `pnpm verify` (validate && typecheck && lint && lint:i18n-gate && hygiene && build && test) at HEAD 9a3ad8dc => 'Test Files 104 passed (104) / Tests 2552 passed | 1 skipped (2553)', VERIFY_EXIT=0. The trailing '✗ source hygiene failed: …' / '✗ i18n lint gate: …' lines are those gates' own captured self-test stdout, as the dispatch noted; the &&-chained exit is 0. No dogfood ablation applies (no packages/qa/dogfood in this repo, nothing resolves from dist/). Refusal assertions carry the measured envelope: ValidationError / code 'VALIDATION_FAILED' / status undefined, on top of the message wording.",
     "open_questions": [],
     "out_of_scope_findings": [
       "filed as #1164: a lead already DISQUALIFIED as a duplicate still blocks the delete, now via duplicate_disqualification_requires_survivor instead of the field pairing — refused in both trees so not a regression from this PR, but its resting state ('closed as a duplicate of a person since erased') is a product call every candidate for which either loosens a rule correct at authoring time or rewrites a recorded business verdict; boundary pinned in this PR's test file",
       "filed as #1165 (finding label, no pm:queue): the engine's __referentialFieldClear marker IS reachable from a hook on 17.0.0 GA at ctx.api.executionContext — present on the cascade write, absent on a hand edit — which falsifies the load-bearing note in test/freeze-guard-reference-cleanup.test.ts that no marker reaches a hook; deliberately NOT read by this fix (operation-private __ key, and reachability through the shipped QuickJS path is unproven, so it could be green in the test rig and false in production)",
       "NOT filed, falsified by measurement: swept every other object for the same #696/#711 construction. One candidate, crm_quote.crm_contact (a set_null lookup carrying requiredWhen on status), turned out unreachable — contact_integrity refuses that delete first with a deliberate, helpful business message, on every quote status including the draft control"
     ]
   }

   Sub-question the card left open, answered by measurement, not by choice: clearing duplicate_of_type also clears duplicate_status. Enumerated every reader of that field in the repo — the suspected_duplicates review queue and its form block (src/views/lead.view.ts), the duplicate_disqualification_requires_survivor validation (lead.object.ts), the hook's own insert-time stand-down, and the four translation bundles. With no link left, none of them can act on suspected: the queue's whole workflow is "open the two records and compare them" and the row would name nobody, while the confirmed route that drains the queue is closed by the validation the cleared discriminator satisfies. Leaving the flag would therefore park a permanently unworkable row in that queue rather than merely being inert. The verdict is not destroyed — duplicate_status declares trackHistory: true.

   Conflict to record: none between the dispatch prompt and the agent contract. The file surface held (src/objects/lead.object.ts, src/objects/lead.hook.ts, test/, plus the mandatory .changeset/*.md); src/apps/crm.app.ts was never touched.

   ---

   Generated by Claude Code
6. os-zhuang commented on Aug 16, 2026

   

   os-zhuang

   on Aug 16, 2026 – with Claude

   Contributor

   More actions

   ACCEPT — PR #1166. Landing gated on one job still running; see the bottom.

   Verified against GitHub, not the report:

   • Scope: 5 files — changeset, src/objects/lead.hook.ts, src/objects/lead.object.ts, test/lead-duplicate-link-cleanup.test.ts (new), test/freeze-guard-reference-cleanup.test.ts. All inside the declared surface (src/objects/lead.*, test/). Changeset present, Check Changeset green. No path touches docs/adr/**, .claude/skills/** or skills/** — normal landing path.
   • Filed findings both exist and are specific: A lead already disqualified as a duplicate still blocks deleting the record it named — duplicate_disqualification_requires_survivor is the second rule on the same erasure path #1164 (labelled needs-user-decision — see below) and The engine's reference-cleanup marker IS reachable from a hook on 17.0.0 GA — #720's "no marker reaches a hook" note is falsified, and three copies of a shape predicate rest on it #1165 (finding).
   • The sweep for further 报名过市场活动的线索/联系人无法删除:级联把 campaign_member 的引用置空,随即违反它自己的校验规则 #696 / 参加过会议的线索/联系人/用户同样无法删除:crm_event_attendee 与 #696 是同一构造的同一缺陷 #711 instances reported one candidate and falsified it by measurement rather than filing it: crm_quote.crm_contact is unreachable because contact_integrity refuses first on every status including the control case. A finding not filed, with the reason, is worth as much as one filed.

   What makes this the fix and not the loosening

   The thing to check on this card was whether the suite went green by relaxing requiredWhen. It did not, and the guard that prevents it is the restated check: a payload carrying its own non-blank duplicate_of_type stands the retirement down, so an author's claim is judged by the rule rather than silently swallowed. That distinction is pinned by its own test ("refuses a write that STATES a type and blanks its lookup in one payload"), which also asserts the live claim survives the refusal — refused for real, not merely announced.

   The measurement that made the design decision

   The dispatch asked for one sub-question to be measured, not guessed. It was measured harder than asked:

   • duplicate_status goes with the link — every reader in the repo enumerated, each shown unable to act on suspected with no link left. The decisive argument is not "nothing reads it" but that leaving it would park a permanently unworkable row in the suspected_duplicates queue: it names nobody to compare against, and the confirmed route that drains the queue is closed by the very rule the cleared discriminator satisfies. The verdict survives in field history (trackHistory: true).
   • The hook never asks who wrote the null, because it cannot: on GA the engine's cleanup and a hand-clear are byte-identical in input, ctx.user and ctx.session, and the survivor is still readable from both. Asking a question the context cannot answer is what forces shape-sniffing. The resulting trade — a hand-clear also retires the claim — is pinned as a named test so a later reader sees a measured decision rather than an oversight.
   • Priority 300 is load-bearing and has its own test: below lead_automation (200), the converted-lead lock would refuse the very cleanup 冻结/锁定守卫会拒绝引擎的引用清理:被冻结的记录让它引用的人和客户永远删不掉 #720 fixed it to allow, because duplicate_of_type / duplicate_status are not lookups going value→null.

   Red before green: 10 failures across two files on unmodified origin/main, including the flipped #720 boundary assertion. Refusals assert the measured envelope (ValidationError / code: 'VALIDATION_FAILED' / status: undefined) rather than bare toThrow().

   #1164 labelled needs-user-decision

   Correctly framed by the dev as out of scope: the delete is refused in both trees, so this PR changes which rule answers, not whether the delete succeeds. "Closed as a duplicate of a person since erased" has no obviously right resting state, and all three candidates either loosen a rule correct at authoring time (the thing this card rejected), rewrite a recorded business verdict, or add a capability. That is a product call, and the boundary is pinned in this PR's test file where a fix should flip it.

   #1165 — the restraint is the right call

   The engine's __referentialFieldClear marker turns out to be reachable at ctx.api.executionContext, which falsifies a load-bearing note in #720's test file. The dev did not read it, and the reasoning holds: it is an operation-private __ key, and hook bodies run body-only inside QuickJS where ctx.api may be a shim carrying no executionContext at all — so a predicate reading it could be green in the kernel rig and false in production. Finding the key and declining to use it, with the reason written down, is the correct outcome.

   Landing status

   8 of 9 checks completed: success. Build and Test (22.x) is still in_progress — same gate job that was last to converge on #1163. The PR stays draft until it reports success; it will be marked ready and queued on the next check-in, or diagnosed if it turns red.

   ---

   Generated by Claude Code
7. os-zhuang

   closed this as completedin #1166on Aug 16, 2026