Skip to content

fix(revenue): a closed deal's line items are frozen with it - #2113

Merged
objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-2077-closed-deal-lines
Oct 15, 2026
Merged

objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-2077-closed-deal-lines

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #2077

Clause-②: no

What changes

A closed-won or closed-lost opportunity froze its own fields, but its line items did not freeze with it. A rep could add a line to a closed deal, change one of its lines, or move a line onto or off it, and the write went through. opportunity_amount_rollup rightly leaves a closed deal's amount alone, so the lines stopped summing to the deal's Amount.

opportunity_line_item_freeze, a new hook beside the object in src/revenue/objects/opportunity_line_item.hook.ts, now refuses a user's insert or update of a line whose old or new parent deal is closed. It answers with the deal freeze's own envelope, RECORD_LOCKED / 409, and a business sentence that names the deal by its name, never by its id:

Opportunity Initech Phase 1 is closed (closed_won), so its line items can no longer be added or changed.

Scope: this is the insert / update half of the card. The delete half is decision card #2078, which remains open. Delete is not guarded here.

The premise, measured on the unchanged base

Base 9e064b3d, @objectstack/* 18.0.0-next.1. A sales rep wrote through the engine write door (the path POST /api/v1/data/crm_opportunity_line_item takes), using a temporary probe file that was deleted before the commit:

  • A 3 x 500 line inserted onto the rep's own closed_won deal: admitted. The deal's amount stayed at 1,000 and its one line summed to 1,500.
  • A quantity change on a closed_lost deal's line: admitted (quantity 9 stored).
  • Moving an open deal's line onto a closed_won deal: admitted (the stored parent is now the closed deal).

Where the guard lives: beside the line, not in opportunity_lifecycle

The card asked for "one freeze, one envelope, in the existing lifecycle hook". opportunity_lifecycle lives in src/sales, and crm_opportunity_line_item belongs to src/revenue.

  • Measured: a temporary edit making opportunity_lifecycle name ['crm_opportunity', 'crm_opportunity_line_item'] keeps test/package-ownership.test.ts green (24/24). Today the app package registers both directories, so nothing mechanical refuses it. The edit was restored, and the blob was confirmed equal to HEAD.
  • Still refused by the layout: AGENTS.md Project Architecture rule 1 says "a hook may not attach to another package's object (ADR-0130 R4), and co-location is what enforces that". src/revenue/ is a package. Decision 5 of docs/architecture/module-split-plan.md makes it app.objectstack.hotcrm.revenue, and there R4 is a loud author-time refusal. The two campaign hooks set the precedent: they moved into src/sales to sit beside the objects they hook.

So the line freeze is its own hook, beside its object. It keeps the deal freeze's terms, which means one envelope and one definition of "closed":

  • Closed means stage is closed_won or closed_lost. That is the test opportunity_lifecycle applies, and opportunity_amount_rollup uses the same one. No second definition is introduced.
  • User writes only (ctx.user?.id), the boundary the deal freeze draws. A write with no user (the seed, a migration) passes.
  • An update is judged by what it changes, the same way quote_workflow judges quote lines. id, updated_at, updated_by and line_number do not count, so the 4.0.0 line-number back-fill's { line_number: null } touch still numbers a closed deal's line.
  • Both parents are judged, so a line can be moved neither onto a closed deal nor off one.
  • Priority 90 puts it ahead of the price fill (100) and the line numbering (110). The caller's own payload is what gets judged, not the keys those two hooks inject. This matches the reason opportunity_lifecycle gives for its own ordering.
  • The deal is read as the caller. A deal the caller cannot read stands this guard down. The platform's controlled_by_parent write gate still applies to that write.

Tests

test/opportunity-line-freeze.test.ts is the deal-side twin of test/quote-line-freeze.test.ts:

  • Refused, each asserting code + status (RECORD_LOCKED / 409), the sentence, and the absence of the deal id: an insert onto a closed_won deal and onto a closed_lost deal (nothing stored), a change to a closed deal's line (stored line unchanged), and a move off a closed deal and onto one (both lines stay put).
  • Shipped path: the lowered body runs in QuickJS (runShippedHook) and gives SandboxError carrying code / status / userMessage.
  • Controls: an open deal still takes and changes lines, and its amount follows them (1,550). A system write onto a closed deal still lands. The back-fill touch still numbers a closed deal's unnumbered line.

Two existing tests were also edited. test/refusal-envelope.test.ts: the hand-maintained call-site count goes from 24 to 25, with a history line. test/runtime-coverage.test.ts: the new file is listed as a runtime test file.

Ablation (one run, after the fix was committed). scripts/ablation-replace.mjs replaced the closed-stage predicate with a stage that never matches. The tests load source through objectstack.config.ts, so no dist/ is involved. Injected marker count was 1 and the anchor went from 1 to 0. test/opportunity-line-freeze.test.ts then had 5 failed, 3 passed: all five refusal cases went red and the three controls stayed green. The file was restored with blob == HEAD (192c970e) and an empty git diff HEAD. A first attempt aborted on a script variable error before any mutation was written, so that attempt was not an ablation.

Through the real REST door (dev server on port 4177, pnpm demo:staff, signed in as na.rep@objectos.ai):

  • POST /api/v1/data/crm_opportunity_line_item onto the rep's closed-won deal: 409, code: "RECORD_LOCKED", with userMessage set to the sentence above and no record id. The deal kept 3 lines before and after.
  • PATCH of one of that deal's lines: 409.
  • Moving an open deal's line onto it: 409.
  • Control: a POST onto the rep's open deal: 201.

Gates (local, at bf9bb7a6)

gate exit verdict line
pnpm validate (lock) 0 ✓ Validation passed
pnpm typecheck 0 tsc --noEmit clean; --listFiles includes both changed .ts files
pnpm lint (lock) 0 18 suggestion(s), unchanged in kind (approval-approver advisories)
pnpm lint:i18n-gate 0 ✓ i18n lint gate: 0 i18n/missing-* issues
pnpm hygiene 0 ✓ source hygiene clean
pnpm hygiene:tokens 0 ✓ source token ratchet clean
pnpm build (lock) 0 ✓ Build complete; the hook ships as a lowered js body (1,039 chars) at priority 90
pnpm test (lock) 0 Test Files 180 passed (180) · Tests 3671 passed / 1 skipped (3672)

Playwright e2e runs in CI.

Token ratchet (ceilings RULED x1.5):

Acceptance notes

  • Deviation from the card's wording. The freeze is not inside opportunity_lifecycle; it sits beside the line item, for the R4 / co-location reason above. It still has one envelope and one "closed" test, but it is a second hook body.
  • No mechanical check holds hook co-location today. test/package-ownership.test.ts judges by artifact package, and src/revenue is still registered by the app package, so a src/sales hook naming a src/revenue object passes every gate until revenue is packaged. This is an observation, not filed (AGENTS.md §3: no gate farm here).
  • Docs untouched. No sentence becomes false. content/docs/sales/opportunities.mdx "Closed deals are locked" now also holds for adding and changing lines. Whether removing a line is part of that lock is [Decision] Removing a line item from an accepted quote or a closed deal: keep allowing it and say so (A), or refuse a hand delete once the platform tells a hook "this delete is a parent's cascade" (C)? #2078's question.
  • Line budget. 225 changed lines (222 added, 3 removed) against the ~200 estimate. The new test file is 129 of them.

Generated by Claude Code

A closed-won or closed-lost opportunity froze its own fields, but a user
could still add a line to it, change one of its lines, or move a line onto
or off it. `opportunity_amount_rollup` rightly leaves a closed deal's amount
alone, so its lines stopped summing to it (measured on 18.0.0-next.1: a
rep's 3 x 500 line on their own closed-won deal was admitted, the deal kept
1,000, the lines summed to 1,500).

`opportunity_line_item_freeze` refuses a user's line insert or update whose
old or new parent deal is closed, with the deal freeze's own envelope
(RECORD_LOCKED / 409) and the deal named by its name. "Closed" is the test
`opportunity_lifecycle` uses (closed_won / closed_lost); only user writes
are judged, and an update is judged by what it changes, so the line-number
back-fill touch still numbers a closed deal's lines.

It sits beside `crm_opportunity_line_item` in src/revenue rather than in
`opportunity_lifecycle`: a hook may not attach to another package's object
(ADR-0130 R4, enforced here by co-location). Delete is not guarded; that
half is its own decision card.

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

vercel Bot commented Oct 15, 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 15, 2026 1:04am UTC

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 Oct 15, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

复核人:repo:hotcrm 执行席(#1353)· 判决 ACCEPT · 钉 head 1a3963c6a050e438d4eeb9d330527621bc1bdce2(dev 交付的 bf9bb7a6 + 席位经中继 pr_update_branch 合入 origin/main 8dfed905 的合并提交)· 2026-10-15T01:17Z

形态:一卡一 PR,Fixes #2077(卡范围即新增 / 修改这一半,删除那一半是决策卡 #2078),Clause-②: no;5 文件,+222/−3 = 225 行(派发预算约 200,超约 12%,大头是镜像 quote 线冻结测试的新测试文件);路径面无 governed 面 → 非 Tier H,席位落地。分支切自 9e064b3d,#2111 随后改了测试基础设施而队列不在合并树上重建,故合入 main 让 CI 在合并后的 head 上整跑;git merge-tree 无冲突。

前提实测:diff 逐行读过。src/revenue/objects/opportunity_line_item.hook.ts 新增 opportunity_line_item_freeze(beforeInsert / beforeUpdate,priority 90,先于价格填充与编号):仅判用户写(ctx.user?.id);更新只在真正改了业务字段时判(line_number 回填触碰照常落地);新旧两个父商机都判;「已关闭」沿用 opportunity_lifecycle 的判据(closed_won / closed_lost);拒绝为 RECORD_LOCKED / 409,文案用商机名、不带记录 id,信封与 _refusal.ts 同形(refusal-envelope 计数 24→25)。dev 读数:基线上插入、改数量、挪入三种写均被放行(金额 1,000、明细合计 1,500);分支上三者均 409,REST 实测同;对照:未关闭商机照常、系统种子写入照常、管理员回填照常。切除(把已关闭判据改成不可能值)只红 5 个拒绝用例、3 个对照仍绿。

门禁 check-runs(钉 head):Build and Test (22.x) ✓ · Quality Checks ✓ · Check Changeset ✓ · Playwright ✓ · link-check ✓ · CodeQL ✓ · Analyze Code (javascript) ✓ · Label Pull Request ✓ · Vercel Preview Comments ✓ — 9/9 success,mergeable_state clean(CI 跑在 8dfed905 + 本 PR 的合并树上)。Typecheck and Build(docs-app.yml)按路径过滤,本 PR 不触及 content/docs/**,缺席属正常。

接受的偏差:守卫没有放进卡里说的 opportunity_lifecycle(在 src/sales),而是放在 src/revenue 的明细行 hook 里——派发令 H3 预授权的退路:ADR-0130 R4 / AGENTS.md 布局规则 1 不许一个包的 hook 挂到另一个包的对象上(dev 实测当前机制上能挂、测试也不拦,但规则禁止)。判据与信封仍是同一套。

范围外去向:删除那一半 → 决策卡 #2078(已存在)。「hook 与对象同包」目前没有机械检查——仅记录(AGENTS.md §3,不加门禁)。

落地条款:席位经中继 pr_ready + automerge_enable,以 objectstack-fleet[bot] 入合并队列;合并后在 #2077 落探针表。#2112 已先落地(628bd2a5):席位在本地把本 head 与之合并,无冲突,#2112 带入的 6 个文件与本 PR 不相交且不含测试基础设施,token ratchet 干净(src/revenue authored ~19,557 / ~28,500,src/sales ~103,287 / ~160,500)。

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