The demo makes M3's reminder layer look broken: 54 of 120 contracts have no `legal_owner`, so four of six scheduled jobs notify nobody by zhuangjianguo · Pull Request #55 · objectstack-ai/hotclm

zhuangjianguo · 2026-09-10T07:28:06Z

Fixes #47

Data only. ⛔ No file under src/flows/, no hook, no DESIGN.md. Four files: three in src/data/, plus the operator table in README.md because the deal names two accounts an operator has to create.

⚠️ First: the card's two numbers, re-measured

Both are close and both are off, and one of them changes what the fix has to be. Read from the driver's own SQLite file (node:sqlite, read-only) on origin/main @ a7b7db5, clean database, pnpm demo, README operator setup performed, one run of each job.

The card says	Measured on `a7b7db5`
54 of 120 have no `legal_owner`	51 of 120	the compiled artifact agrees: `legal_owner empty: 51`
F3 selected 10, notified 5	10 selected, 5 notified	reproduces exactly
all 12 `sys_notification_receipt` rows carry the dev admin	44 receipts, 7 on the dev admin	5 from F3, 2 from F10
four of six jobs notify nobody	one does — F12, for the reason the card itself excludes	F4 1/1, F10 2/2, F11 18/18

The premise holds — the demo's legal reminders were the administrator writing to themselves — but its arithmetic was measured before #26's owner_id deal was in the database. What is actually true after #26 is sharper and smaller: owner_id already reaches three real requesters, and the only user references still parked on the dev admin are clm_contract.legal_owner (this card) and clm_obligation.owner (filed as #52).

And 51 is not the number to fix. Drafts, cancelled contracts, submitted contracts nobody has picked up (§05's 待受理 queue) and every contract on a type that skips legal review are ownerless because F2 never assigns one to them. Filling those would be inventing history. The rows that actually cost a notification are the twelve at in_review, because F3 is the only job whose recipients are legal_owner alone.

Two causes, and the second one is the whole of the 5-of-10

1. `legal_owner` named the dev admin — 69 rows, one account

Field.user, optional, so like owner_id (#26) it may name accounts that do not exist yet. Re-measured here on 17.4.0, because the card asked (Zone 2, assumption 2) and because the fixture's own rule is that a (measured) annotation which is not one is worse than no comment: seeded on a clean database with the accounts absent, all 120 rows land legal_owner = NULL and every row survives — 120 contracts / 60 reviews / 200 obligations / 300 instalments. An unresolvable name never refuses the row. Field.user and Field.lookup('sys_user') resolve identically here.

It now names the clm_legal_counsel ×2 of DESIGN.md §10, dealt by counterparty relationship — the identical rule ownerOf uses for requesters, and the one keys.ts already argues for at length (a lawyer owns a relationship, not a random slice of the book). 34 / 34.

⚠️ The deal does not claim to reproduce F2. F2 picks the counsel with the fewest OPEN contracts at the moment of submission; that answer depends on which contracts were still open on each of 68 different historical days, and a corpus loaded in one pass has no such history. The comment at the site says so rather than implying a replay.

2. Six `in_review` contracts sat on types that refuse to be in review

This is the finding the card did not have, and it is the entire 5-of-10:

contract_state_machine, submitted -> in_review, refuses twice:
  "This contract type does not require legal review; a submitted contract
   of this type goes straight to in_approval."
  "Assign a legal owner before the contract enters review."

Six of the twelve in_review contracts were on SOW / ORD, the two types whose requiresLegalReview is false. F2 never assigns a legal owner to such a type — so those six were rows no surface in this app could have made, and they were exactly the six F3 selects and can tell nobody about. plan.ts's own stated property 1 — no row is in a state the write layer would have refused — had a third edge nobody had covered.

⛔ Giving those six an owner would not have fixed them; it would have swapped one impossible claim for another (a legal owner on a type F2 skips) and let the fixture keep lying while the number looked better. So dealStatuses gains a third repair that swaps them onto legal-review types, in the same style as the two repairs already there, and §10's spread survives untouched. assertDealtStates then re-proves both per-row invariants — blocked counterparty, and in_review type — after all three passes, because a swap moves two rows and counting statuses cannot notice a repair undoing an earlier one.

The deliberate ownerless row — this is test coverage, and it is signposted as such

One contract in review is handed back to the queue with no legal owner, and the site says why in about forty lines: card 09 measured the partitioned "nobody to tell" edge working, loop-node.ts iterates with a bare await so that edge is all that stands between one unassigned row and a sweep reporting acted: 0 for every row after it, and a reader who "tidies this up" is deleting the only place this corpus can exercise it.

Two things make the row provable rather than hopeful, and handBackInReview throws on either:

It must sit in F3's (-60, -30] day band. Past 30 so the over-30 stage selects it — and inside 60, because the over-60 stage copies clm_legal_head, which would give the notification a recipient and take the notify edge instead. Only the over-30 stage has legal_owner as its sole recipient.
One is the minimum that keeps the edge live and therefore also the maximum: every additional handed-back row is a reminder the demo does not send, which is the defect this PR exists to fix. Raising the constant is a one-line change and the reachability proof re-runs for whatever number it is given.

Selected vs notified, before and after — read from the database

Identical protocol both sides: clean .objectstack/data, pnpm demo on port 3147, the same README operator setup (3 requesters + all 7 positions + dev admin into clm_admin), re-seed so the upsert hands the rows over, then one trigger of each job through POST /api/v1/automation/{name}/trigger; counts read from each run's own node summary.

Job	before: selected → notified	after: selected → notified
`legal_review_sla` (F3) over 30d	10 → 5 (quiet 5)	6 → 5 (quiet 1, by design)
`legal_review_sla` (F3) over 60d	0 → 0	0 → 0
`turn_stalled` (F4)	1 → 1	2 → 2
`obligation_due` (F10)	2 · 0 · 0 → 2	2 · 0 · 0 → 2
`payment_overdue` (F11)	0 · 18 → 18 (acted 18)	0 · 18 → 18 (acted 18)
`renewal_notice` (F12)	21 → 0	25 → 0
`expiration_sweep` (F13)	0 · 0 → 0	0 · 0 → 0

Every run green, failed=0 on both sides. F12's zero is unchanged and untouched — the card ruled it correct and out of scope, and the cause (the once-per-contract key excluding rows the seed already marks is_expiring) is not this column.

⚠️ Read F3's selected honestly: it fell from 10 to 6 and that is not an improvement or a regression, it is noise. timelineFor draws each in_review contract's review age from -8 … -58 days, so the share past 30 days is a draw; the status swap changes which contracts are in_review and shifts the shared PRNG stream, so the draw is a different one. 10-of-12 was a lucky sample, 6-of-12 is near the mean. The numbers that are not noise:

	before	after
`in_review` rows carrying a legal owner	6 / 12	11 / 12
F3 selections it can tell someone about	5 / 10 = 50%	5 / 6 = 83%
`legal_owner`-addressed receipts on the dev admin	5 of 5	0 of 5
distinct people receiving notifications	5	7
`in_review` rows in a state the app refuses	6	0

Where the notifications actually land

before                                  after
  Finance Controller     18               Finance Controller     18
  Business Requester 1   12               Business Requester 1    9
  Dev Admin               7               Business Requester 2    6
  Business Requester 2    4               Business Requester 3    5
  Business Requester 3    3               Legal Counsel 2         3
                                          Legal Counsel 1         2
                                          Dev Admin               2

clm_legal_review_over_30_days  ->  Dev Admin x5   BECOMES   Legal Counsel 1 x2, Legal Counsel 2 x3
clm_obligation_due_soon        ->  Dev Admin x2   unchanged — clm_obligation.owner, filed as #52

The quiet edge, at row level

F3 over-30 stage, after — the six selected rows and the notices each produced:

  MSA-2026-0005  Meridian Retail Group        56d   legal_owner NULL   notices 0   <- handed back
  LSE-2026-0004  Fernway Cleaning             45d   Legal Counsel 2    notices 1
  ICA-2026-0004  Camille Beauchamp            45d   Legal Counsel 2    notices 1
  SUP-2026-0001  Granite Facilities           39d   Legal Counsel 1    notices 1
  NDA-2026-0003  Kestrel Analytics            38d   Legal Counsel 1    notices 1
  NDA-2026-0016  Copperfield Travel           32d   Legal Counsel 2    notices 1

run: failed=0, acted=0, over30_tell runs 5 / skipped 1

Selected, ownerless, run green, nothing sent — the edge is still exercised, on a row that is now legal (a lawyer handing a file back clears the column through the ordinary edit form; nothing holds it set after the transition, and legal_owner is in the parties group so _grants.ts does not lock it).

And out of the box, with no operator accounts

legal_owner NULL x120 · owner_id NULL x120 · rows 120/60/200/300 all present
F3: over30 selected 6 -> notified 0 (quiet 6), failed=0, 0 notifications written

⚠️ Stated plainly because it is the one place this PR trades down: before, an out-of-the-box demo produced 5 legal notices to the dev admin; now it produces none until the operator creates the accounts. That is #28's open question, not a new one — owner_id has behaved this way since #26, out of the box GET /api/v1/meta/app/clm serves the only existing account navigation: [] so nobody can open the inbox to see them anyway, and the dev admin could not act on them if they did. The state a demo is actually evaluated in is the one after the README setup, and that is where the table above is measured.

Browser

Chromium 1194 at /opt/pw-browsers/chromium-1194/chrome-linux/chrome; playwright install was not run. Signed in as each lawyer in turn at /_console/, both landing on /_console/apps/clm/clm_contract/view/my_contracts with the full five-section navigation.

Legal Counsel 1 — bell badge 2, panel reads "Inbox · 2 total · 2 notifications · 0 pending approvals":

"In legal review over 30 days: Supplier Agreement — Granite Facilities" — "Contract SUP-2026-0001 has been in legal review since 2026-08-02… more than 30 days. This is a fixed 30-day threshold, not this contract type's own review SLA."
"In legal review over 30 days: Mutual Non-Disclosure Agreement — Kestrel Analytics" (NDA-2026-0003)

Legal Counsel 2 — bell badge 3, panel reads "Inbox · 3 total · 3 notifications · 0 pending approvals": LSE-2026-0004, ICA-2026-0004, NDA-2026-0016.

Two people, two different inboxes, five notices between them — 200 GET /api/v1/data/sys_notification_receipt?top=200&filter=[["and",["user_id","=","…"],["channel","=","inbox"]]] with a different user_id in each, which is what makes them genuinely two recipients rather than one list rendered twice. Before this PR all five of those rows carried the dev admin's id.

Console: one 404 for a static asset (present on the baseline, see PR #42) and 403 PERMISSION_DENIED on GET /api/v1/data/sys_activity — the platform Console's own activity feed, which no clm_* set grants; it appears because this is the first browser pass driven as a non-admin rather than as the dev admin. Neither is application output and neither is caused by this diff. No application errors.

Gates

Exit codes captured before any pipe (cmd > file 2>&1; EXIT=$?), on a3b7af1 with a clean working tree:

VALIDATE=0   LINT=0   TYPECHECK=0   I18N=0

  ✓ Validation passed (1012ms)
  21 warning(s), 5 suggestion(s)        ← identical to the count PR #42 records for origin/main
  ✓ i18n gate
  LOCALES  : "en", "zh-CN" checked (required: en, zh-CN)
  COVERAGE : 0 missing keys across 2 locale(s)

All 21 warnings are field-no-consumers and all 5 suggestions approval-approvers-may-resolve-empty, the same families and the same counts as the baseline: this PR adds none. Boot: the two known pre-existing platform ERROR lines (objectstack#17175 _objectstack_sequences, #17176 sys_oauth_resource) and the pre-existing [Seeder] Inline seed exceeded 8000ms budget notice. Nothing else. Both re-seeds completed with zero Failed to write lines.

Acceptance notes

scripts/demo.mjs now carries two stale sentences and this PR deliberately does not fix them. Its DEMO_USER doc block and its priming-failure message both still list clm_contract.legal_owner among the references that resolve against the dev admin. The check itself is unaffected and still earns its place (clm_review.reviewer is required, so a name drift still costs all 60 review rows) — only the enumeration is now two items, not three. That file is held by PR pnpm demo prints its "now create the three requesters" instruction ~290 lines and 120 ERROR lines before the ready banner, so nobody reads it #51 (pnpm demo prints its "now create the three requesters" instruction ~290 lines and 120 ERROR lines before the ready banner, so nobody reads it #49) this round, so editing it here would collide. Filed as scripts/demo.mjs still tells the operator that clm_contract.legal_owner resolves against the dev admin — PR for #47 made that false #54, to land after both.
clm_obligation.owner is the dev admin on all 200 rows. After this PR they are the only receipts left on that account (2 of 45). Same shape, different column, and it needs a fixture-design answer of its own about how much of F10's own quiet edge to preserve. Filed as The obligation reminders are still the administrator talking to themselves: clm_obligation.owner is the dev admin on all 200 seeded rows #52.
34 backfilled contracts carry a legal_owner while is_backfilled says on the same row that no review happened. legal_owner's own description is "The lawyer who accepted the review". This PR deals the column but deliberately does not change which rows carry it — and clearing them would cost F12/F13 a recipient, since §10 draws the expiring corpus from exactly those rows. Filed as 34 backfilled contracts carry a legal_owner, and is_backfilled says on the same row that no review ever happened #53 with both readings and their costs.
Accounts created through POST /api/v1/auth/admin/create-user land with must_change_password = 1, and every API call then answers 403 PASSWORD_EXPIRED until the password is rotated. sys_user.must_change_password is not writable through the data API (ADR-0092), so the rotation has to go through POST /api/v1/auth/change-password. Noted, not filed: an operator following the README uses Setup → Users, not this endpoint, and whether that path sets the same flag was not measured here.
F3's selected count is a sample from the review-age distribution, not a design parameter. ⛔ It was deliberately not tuned. Moving timelineFor's in_review window to make more rows fall past 30 days would inflate a reminder count against a number DESIGN.md does not pin — the exact "seeded number that looks computed" shape AGENTS.md forbids.
No changeset: this repo has no changeset gate.

🤖 Generated with Claude Code

https://claude.ai/code/session_01R3n3GGzobdegM4HUzah1iR

Generated by Claude Code

…ing `in_review` onto types that refuse it The seeded corpus gave M3's reminder layer nobody to tell and nobody but the administrator to tell it to. Measured on `a7b7db5` with the README operator setup performed, one run of each scheduled job: 44 notification receipts, and all 5 that `legal_owner` addressed carried the dev admin — an account that holds no `clm_*` permission set, so `clm_legal.access` gates every 法务工作台 item away from it and 审查中 (`legal_owner == me`) is a screen it can never open. F3 selected 10 contracts in review over 30 days and notified 5. Two causes, one column: 1. `legal_owner` named `DEMO_USER`. It is `Field.user` and optional, so like `owner_id` (#26) it may name accounts that do not exist yet: an unresolvable name lands NULL and never refuses the row, and every dataset is an upsert, so the operator creates the accounts and re-runs `pnpm demo` to take delivery. It now names the `clm_legal_counsel ×2` of DESIGN.md §10, dealt by counterparty relationship the way `ownerOf` deals requesters — 34 / 34. 2. Six of the twelve `in_review` contracts sat on the two contract types whose `requiresLegalReview` is false. `contract_state_machine` refuses that edge twice over and F2 never assigns a legal owner to such a type, so those six were rows no surface in this app could have made AND rows F3 selects and can tell nobody about — the whole of its 5-of-10 reading. A third repair in `dealStatuses` swaps them onto legal-review types, in the same style as the two repairs already there, so §10's spread survives; `assertDealtStates` re-proves that invariant and the blocked-counterparty one after all three passes, because a swap moves two rows and counting statuses cannot notice. One contract is deliberately handed back to the queue with no legal owner, so F3's partitioned "nobody to tell" edge — which card 09 measured working — stays exercised. `handBackInReview` proves reachability rather than assuming it: the row must sit in F3's `(-60, -30]` day band, past the 30-day threshold so the over-30 stage selects it and inside 60 so the over-60 stage (which copies `clm_legal_head`, giving the notification a recipient) does not take it first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R3n3GGzobdegM4HUzah1iR

…erences `clm_contract.legal_owner` names `Legal Counsel 1` / `Legal Counsel 2` instead of the dev admin, so the operator setup has to name them exactly the way it already names the three business requesters — otherwise the column resolves to nothing, silently, and the empty-screen problem has moved rather than been fixed. The two exact-name groups are now one table, and the paragraph that said `legal_owner` points at the dev admin no longer says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R3n3GGzobdegM4HUzah1iR

PR #55 deals `clm_contract.legal_owner` to the two `clm_legal_counsel` account names, so it no longer resolves against the dev admin. Two places in `scripts/demo.mjs` still enumerated it as one of three references that do: the `DEMO_USER` doc block, and the failure detail printed when the priming boot's account is not named `Dev Admin`. The mechanism is unchanged and the check keeps its place — `clm_review.reviewer` is `required: true` with `storage: { notNull: true }`, so a name drift still costs all 60 review rows. Only the enumeration moves: two references, not three. The sentence about what a drift costs is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R3n3GGzobdegM4HUzah1iR

PR #55 deals clm_contract.legal_owner to the two clm_legal_counsel account names (contract.seed.ts -> legalOwnerOf() -> LEGAL_OWNERS), so it no longer resolves against the dev admin. Two passages in scripts/demo.mjs still listed it as one of three references that do: the DEMO_USER doc block and the failure detail printed when the priming boot's account is not named "Dev Admin". Both now name the two that really do - clm_review.reviewer and clm_obligation.owner. The mechanism is untouched: clm_review.reviewer is required with storage.notNull, so a name drift still costs all 60 review rows and the check still earns its place. The lines saying what the drift costs and the recovery command are byte-identical. Verified by running the failure path rather than by the gates, which cannot see this file (tsconfig includes only objectstack.config.ts and src/**, and tsc --listFiles names zero files under scripts/): the pre-fix file prints the stale list on a wrong account name, this one prints the corrected list on the same input, and on the correct name the check does not fire at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R3n3GGzobdegM4HUzah1iR

claude added 2 commits September 10, 2026 07:12

zhuangjianguo mentioned this pull request Sep 10, 2026

The demo makes M3's reminder layer look broken: 54 of 120 contracts have no legal_owner, so four of six scheduled jobs notify nobody #47

Closed

zhuangjianguo marked this pull request as ready for review September 10, 2026 07:30

zhuangjianguo merged commit d34b225 into main Sep 10, 2026
2 checks passed

claude Bot mentioned this pull request Sep 10, 2026

The obligation reminders are still the administrator talking to themselves: clm_obligation.owner is the dev admin on all 200 seeded rows #65

Merged

Navigation Menu

Repository navigation

The demo makes M3's reminder layer look broken: 54 of 120 contracts have no `legal_owner`, so four of six scheduled jobs notify nobody - #55

The demo makes M3's reminder layer look broken: 54 of 120 contracts have no `legal_owner`, so four of six scheduled jobs notify nobody#55
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-47-seed-legal-owner

zhuangjianguo commented Sep 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

zhuangjianguo commented Sep 10, 2026

⚠️ First: the card's two numbers, re-measured

Two causes, and the second one is the whole of the 5-of-10

1. legal_owner named the dev admin — 69 rows, one account

2. Six in_review contracts sat on types that refuse to be in review

The deliberate ownerless row — this is test coverage, and it is signposted as such

Selected vs notified, before and after — read from the database

Where the notifications actually land

The quiet edge, at row level

And out of the box, with no operator accounts

Browser

Gates

Acceptance notes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. `legal_owner` named the dev admin — 69 rows, one account

2. Six `in_review` contracts sat on types that refuse to be in review