Skip to content

fix(guards): the account and contact deletion guards name what really lets the delete through - #2027

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-2019-deletion-guards
Oct 8, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-2019-deletion-guards

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #2019
Clause-②: no

The account and contact deletion guards told the user to close the blocking records. Closing them did not help, because the app's own required lookups keep the parent. This PR keeps the rule and changes the guards' sentences. Each sentence now names what really frees the record. The account guard also counts what the engine counts.

Measured before the fix (the wrong answer at the public door)

These runs were on dc58e047 (@objectstack/* 17.7.0), through @objectstack/verify's in-process handle. Every write was made as the admin through hooks.run, the engine's write door, which is the door a REST DELETE takes.

Account (F17). A customer account with one open deal:

  1. Delete the account: 409 DELETE_RESTRICTED, "Cannot delete customer account: 1 open opportunity still references it. Close or reassign it first."
  2. The admin closes the deal (stage: closed_lost): 200.
  3. Delete the account again: 409 DELETE_RESTRICTED, dependentObject: crm_opportunity. This time the engine answers in its own words: "This Account is still referenced by 1 Opportunity record(s) through “Account”, which is required and cannot be cleared. Delete or reassign them first."
  4. The admin moves the closed deal to another account: 409 RECORD_LOCKED. The closed-deal freeze refuses the move. The admin deletes the deal: 200. Then the account deletes.

Contact (F18). A contact named on an activated contract:

  1. Delete the contact: 409, with the guard's "… Close or reassign those records first."
  2. The admin terminates the contract: 200.
  3. Delete the contact again: 409 DELETE_RESTRICTED, dependentObject: crm_contract: "This Contact is still referenced by 1 Contract record(s) through “Primary Contact”, which is required and cannot be cleared. …". An expired contract gives the same answer.
  4. The admin gives the contract another primary contact: 200. Then the contact deletes, and the contract stays.

On dc58e047 both ⚠️ measured defect pins were green (test/hooks-runtime-sales.test.ts ~638 and ~935), so the defect reproduced.

What the repo says is meant: the history is kept, for both objects

A closed deal keeps its account.

  • crm_opportunity.crm_account has been a required lookup with no cascade since the app's baseline, so the engine refuses the delete while any deal names the account. The opportunity is privately shared and is not a detail of the account. Sharing coverage gap: territory/team-shared accounts show quote, task and contract related lists the rep cannot read #549 made quotes and contracts master-detail under the account and left the opportunity as a lookup.
  • opportunity_lifecycle freezes a closed deal. Not even an admin can move it to another account (step 4 above).
  • The Accounts page retires an account by setting it to Inactive. Retired accounts stay visible in All Accounts, and Settlement Only "never breaks history".
  • Closed deals are what the Won Opportunity Amount rollup, the forecast's closed figure and win/loss reporting read.
  • The repo already treats products the same way. product_catalog refuses to delete a product that past lines still use ("Set is_active=false to retire instead"). opportunity_line_item.crm_product stays on the restricting default so that deleting a product does not destroy the priced history of the deals that sold it.

An ended contract keeps its primary contact.

So both deletions stay blocked. No deleteBehavior changes, and no record becomes deletable or undeletable that was not already. Only the guards' answers change.

The fix

  • account_protection (src/sales/objects/account.hook.ts). The guard now counts every opportunity on a customer account, which is what the engine counts. It still writes one whole sentence per branch (account_hygiene 删除守卫在单数时主谓不一致:"1 open opportunity still reference it" #721):

    • one deal: "Cannot delete customer account: 1 opportunity, open or closed, still references it. Delete the opportunity first, or mark the account inactive to retire it instead."
    • several deals: the same sentence in the plural.

    The envelope is unchanged: DELETE_RESTRICTED / 409.

  • contact_integrity (src/sales/objects/contact.hook.ts). The counts are unchanged. The sentence's last part, "Close or reassign those records first.", becomes "Close or reassign the opportunities and quotes first, and give each contract another primary contact: a contract keeps its primary contact even after it ends." The envelope is unchanged.

  • content/docs/sales/accounts (en, zh-Hans, zh-Hant). The deletion rule now says that closed deals keep the account and that retiring the account means setting it to Inactive.

  • Changeset. .changeset/2019-deletion-guards.md, 'hotcrm': patch.

Why this is the smallest fix. The required lookups already say what the business means, so the declarations stay as they are. Only the two places that misstated the rule change: the guards' sentences and the Accounts page.

A deviation from the dispatch's suggested route: the contact guard's contract count stays activated. This guard also runs as a cascade child of an account delete, and it cannot tell the two apart (#693, pinned in test/cascade-guard-messages.test.ts). An account delete takes its draft and ended contracts with it (#549). If this guard counted ended contracts, it would refuse that account delete itself. That would turn the F8 sentinel red now, and it would still refuse after objectstack-ai/objectstack#22305 (the platform's cascade order) is repaired. On a direct contact delete, an ended contract is refused by the engine, whose message names the contract, and the guard's sentence already says that ending a contract does not release the contact.

Tests that pin the business facts

  • a closed deal keeps its customer account: closing it changes nothing, deleting it frees the account replaces the F17 ⚠️ pin. With an open deal, the guard refuses with its sentence. After the deal is closed, the guard refuses with the same sentence, DELETE_RESTRICTED / 409. After the deal is deleted, the account deletes.
  • an ended contract keeps its primary contact: ending it changes nothing, another primary contact frees the person replaces the F18 ⚠️ pin and runs for both terminated and expired contracts. While the contract is activated, the guard's sentence names the real remedy. Once it has ended, the engine refuses with DELETE_RESTRICTED on crm_contract. With another primary contact on the contract, the person deletes and the contract stays.
  • The F8 pin (~702, objectql: cascade delete trips a restrict on a record the same cascade was about to delete (registry-order, depth-first walk) objectstack#22305) keeps its body. Its shared fixture no longer seeds a closed deal on the account: that deal would now answer first and hide the cascade the pin measures. The pin's comment says so in one parenthetical.
  • The account_hygiene 删除守卫在单数时主谓不一致:"1 open opportunity still reference it" #721 agreement pins follow the new sentences, in the unit file and end to end in test/cascade-guard-messages.test.ts.

Ablation. Each run was done once and restored, under a trap. No test file was added for it.

  • Both guards reverted to dc58e047. The mutation was checked on disk with grep -c: new sentence 0, old sentence 1, in each file. Result: the singular, plural and both replaced pins fail, for example Received: "Cannot delete customer account: 1 open opportunity still references it. Close or reassign it first." and Received: "… Close or reassign those records first.". The F8 pin stays green.
  • Only the count reverted (open deals only), new sentence kept. Result: the replaced F17 pin fails at the closed-deal step with Received: "This Account is still referenced by 1 Opportunity record(s) through “Account”, …". The singular sentence pin stays green, so this pin is what measures the count.
  • After each run, the restore was checked: git hash-object equals the HEAD blob and git diff HEAD is empty.

Gates

pnpm verify on 97949036: exit 0. It covers Validation passed, typecheck, objectstack lint --strict, i18n lint gate: 0, source hygiene clean, source token ratchet clean and Build complete. Tests: Test Files 172 passed (172), Tests 3567 passed | 1 skipped (3568). pnpm changeset status --since=origin/main bumps hotcrm (patch).

Left to CI, not run locally: test:coverage (thresholds on src/*/objects/*.hook.ts), the docs-app build (apps/docs, a separate Next.js install) and link-check.

Files outside the claim's declared surface

Acceptance notes (seen, not filed)

  • The first pnpm verify run on the same sha failed one unrelated test: test/script-bodies.test.ts, "send_email writes the email and its timeline pointer", expected queued, received failed. The file passed on its own (57/57) and in the second full run. This looks like a timing flake under full-suite load. Carrier: none.
  • No product doc states the contact deletion rule. Open deals, active quotes and any contract all hold a contact, and the Contacts page lists none of this. Carrier: none.
  • The closed-deal freeze also stops an admin from moving a won deal from a duplicate account to the account that survives a merge. Merging two customer accounts therefore means deleting won deals or keeping both accounts. This is a product question, not a defect of this card. Carrier: none.

Generated by Claude Code

claude added 2 commits October 8, 2026 15:59
…tract (#2019)

account_protection counted open opportunities only and told the user to
"Close or reassign it first"; closing the deal was answered by the engine's
DELETE_RESTRICTED on crm_opportunity all the same, because the required
crm_account lookup restricts on any deal. A closed deal is the account's
sales history, so the guard now counts every opportunity and its sentence
names what really frees the account (delete the deal) and the retirement
path that keeps the history (mark the account inactive).

contact_integrity told the user to "Close or reassign those records first";
terminating or expiring a contract left the delete refused on the required
crm_contract.crm_contact. A contract keeps its primary contact for its whole
life, so the sentence now says to give each contract another primary contact.
The count stays activated: the guard also runs inside an account delete,
which takes ended contracts with it (#549).

Flips the two measured-defect pins to the business facts, follows the guard
sentences in the end-to-end pins, and fixes the account deletion rule in the
three-locale accounts page.

Claude-Session: https://claude.ai/code/session_012zh91QzFgePbkmuHnugLN3
Co-authored-by: Claude <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
hotcrm Ignored Ignored Oct 8, 2026 4:50pm UTC

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation ci/cd CI plumbing and the verification pipeline metadata Declarative metadata — schema, security posture, UI surfaces backend Server-side behaviour — hooks, flows, actions labels Oct 8, 2026
claude added 2 commits October 8, 2026 16:41
…ured ~57k

On the merge with main (f7cfb65) the token gate's Headline reads business
semantics ~57k (56,746 tokens), above the ~54k banner's 5% band
(51,300-56,700), so test/docs-readme-token-figures.test.ts went red. The
banner is pinned to the measurement (解耦:banner 钉实测,ceiling 独立):
README now says ~57k and the test docstring's worked row carries the band
the 5% rule derives from it (54,150-59,850). The ceiling stays 59,000.

Claude-Session: https://claude.ai/code/session_012zh91QzFgePbkmuHnugLN3
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 8, 2026 17:00
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit 08cfa20 Oct 8, 2026
11 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 documentation Improvements or additions to documentation metadata Declarative metadata — schema, security posture, UI surfaces

Projects

None yet

2 participants