Skip to content

Seeded reviews name the Dev Admin as reviewer instead of the legal staff - #189

Merged
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-186-seed-reviewers
Oct 27, 2026
Merged

zhuangjianguo merged 2 commits into
mainfrom
claude/issue-186-seed-reviewers

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #186

Clause-②: no
Metadata-protocol: none

All 60 seeded reviews named the Dev Admin as reviewer. With this PR each review names the person whose stage it is, using the seat's mapping from the card. Seven finance reviews are the one exception, explained below. scripts/demo.mjs no longer checks the dev admin's name, because no seeded row depends on it any more.

What changed

  • src/data/keys.ts: new reviewerOf(review), plus LEGAL_HEAD and FINANCE_CONTROLLER (the demoStaff account names). The rule by stage:

    • legal: the contract's legal_owner (legalOwnerOf), or Head of Legal when there is none. That happens once: the contract handed back to the queue.
    • finance: Finance Controller when the contract is approved or later. Before that, Head of Legal (deviation 1 below).
    • compliance: Head of Legal. The plan deals no compliance review today, so the rule just gives the first one an owner.
    • business: the requester who launched the contract (ownerOf).

    The DEMO_USER doc is updated: no reference names the dev admin any more. Only the our side of the clm_signature.signers JSON still spells it, and that string resolves against nothing.

  • src/data/negotiation.seed.ts: reviewer: reviewerOf(review). The header no longer says the dev admin is forced. The note on reviewSeed says why reviewer stays always (it is dealt the same way as legal_owner and owner_id) and that a re-run puts a review's decision back to the fixture's.

  • scripts/demo.mjs: removed the DEMO_USER constant and the "account is named Dev Admin" check. The priming boot and its sign-in check stay, because the header's other reason still holds (rows written before the organization exists). signIn now returns a boolean.

  • README.md: the "Reassign it if you want the reviews on a lawyer's desk" paragraph is replaced by the rule. The demo-staff table gives each account's review counts. The re-run limits now mention a review's reviewer and its decision.

The deal (60 reviews): Legal Counsel 1: 16 legal (2 pending). Legal Counsel 2: 18 legal (5 pending, 2 changes requested). Head of Legal: 8 (1 legal, 7 finance). Finance Controller: 16 finance. Business Requester 2 and 3: 1 business each.

Deviations from the card's mapping (measured on 18.2.0)

  1. Finance reviews on a contract before approved go to Head of Legal (7 of 23). clm_review is controlled_by_parent, so writing a review needs edit access to its contract. Finance gets edit only from approved onward (contract_finance_post_approval). Before that it only reads (contract_finance_routed). Signed in as finance@ on origin/main 37ce10c:
    PATCH /api/v1/data/clm_review/ID  (finance review, contract in_review)    -> 403 PERMISSION_DENIED
    PATCH /api/v1/data/clm_review/ID  (finance review, contract in_approval)  -> 403 PERMISSION_DENIED
      "[Security] Access denied: update on 'clm_review' requires edit access to its master record (master 'clm_contract' not editable by this user (row-level security))"
    PATCH /api/v1/data/clm_review/ID  (finance review, contract active)       -> 200
    
    Re-measured on this branch with the same 403. On those rows the finance controller could not have decided the review and cannot change it now. The nearest persona that can is Head of Legal (review_legal_update_any_stage, plus contract_legal_all_head edit on every contract), which is also the account the mapping already falls back to. Head of Legal deciding one of these seven in the browser is shown below.
  2. Not a deviation, but stated: a business review stays the requester's, and the requester cannot change it. §04 gives clm_requester only R on clm_review. requester2@ PATCH on its own business review: 403 PERMISSION_DENIED (「您没有执行此操作的权限…」) before and after. Both seeded business reviews are already approved, and the card asks only that the requester sees it, which it does (below). Naming legal instead would misattribute the business sign-off, so I kept the seat's mapping. See the open question in the report.

Live readings: pnpm demo:zh, port 3160, @objectstack 18.2.0, throwaway DB

Before (origin/main 37ce10c, clean DB). As admin@, GET /api/v1/data/clm_review returned total 60: Legal · Dev Admin ×35 (26 approved, 7 pending, 2 changes requested), Finance · Dev Admin ×23, Business · Dev Admin ×2. counsel1@ read the same 60, all … · Dev Admin.

After, run 1 (0410379, clean DB). Rows: contracts 120, versions 300, reviews 60, deviations 25, signatures 30, obligations 200, instalments 300. No seed error in the log. No review names Dev Admin.

Signed in as Reviews visible Named for this persona
counsel1@ 60 Legal · Legal Counsel 1: 14 approved + 2 pending
legal.head@ 60 Legal · Head of Legal 1, Finance · Head of Legal 7
finance@ 50 Finance · Finance Controller 16 (all 23 finance reviews visible)
requester2@ 22 Business · Business Requester 2 1
requester3@ 14 Business · Business Requester 3 1

Browser: Chromium /opt/pw-browsers/chromium, locale zh-CN, signed in through /_console/login as each persona.

  • counsel1@ sees its pending legal review named for itself and decides it. Contract MSA-2026-0003 (审查中) › 审查与偏离 › 审查 grid: Legal · Legal Counsel 1 | 法务 | Legal Counsel 1 | 待定 | 高. Next to it: Finance · Head of Legal | 财务 | Head of Legal | 通过. I opened the review, clicked 编辑, set 结论 = 通过, clicked 更新. Result: PATCH /api/v1/data/clm_review/ID → 200, toast 「审查“Legal · Legal Counsel 1”已更新」. After reload the page reads 结论 通过 and 结论时间 2026/10/27 (stamped by review_decision_stamp).
  • finance@ sees and decides a finance review. Contract SOW-2026-0005 (生效中) › 审查 grid: Finance · Finance Controller | 财务 | Finance Controller | 通过. I opened it, 编辑 › 结论 = 要求修改 › 更新. Result: PATCH … → 200, toast 「审查“Finance · Finance Controller”已更新」, and after reload 结论 reads 要求修改.
  • legal.head@ decides a pre-approval finance review (deviation 1). Finance · Head of Legal on 主服务协议 — 极光系统 (审查中), 编辑 › 结论 = 要求修改 › 更新: PATCH … → 200. finance@'s API PATCH on the same review: 403 as above.
  • requester2@ sees its business review. Contract ORD-2026-0005 (已批准) › 审查 grid: Business · Business Requester 2 | 业务 | Business Requester 2 | 通过.
  • Browser console: one 404 and one 401 while the login page loads, before sign-in. After sign-in, no response ≥ 400 on the contract pages. On the contract page there are [object-ui] "conditional predicate failed to evaluate" warnings for the header actions. They are unrelated to this diff.

After, run 2 (pnpm demo:zh again on the same DB, after the three desk decisions above). Banner: Seeds: app.objectstack.hotclm 1130 rows, no seed error, and the same seven totals. The deal is identical (same tally for admin@, counsel1@ and finance@). The three decided reviews were back to their fixture decisions (pending, approved, approved). That is not new: decision was never applyOn: 'create', and the README now says so.

After, run 3 (same DB). As legal.head@ I first reassigned three reviews to the dev admin, which made them read Legal · Dev Admin, Finance · Dev Admin and Business · Dev Admin. After one more pnpm demo:zh they read Legal · Legal Counsel 1, Finance · Finance Controller and Business · Business Requester 2 again. Rows naming Dev Admin: 0 of 60. No row was refused. So a database seeded before this PR is re-dealt by a re-run.

scripts/demo.mjs without the name check, on all three runs: 1/2 done — admin account ready. then 2/2, with every review row landing on the first boot of a clean DB.

Gates (final head 5d7606e; run 1–3 used 0410379, which differs only in comments and README)

  • pnpm install: exit 0
  • pnpm validate: exit 0 (✓ Validation passed)
  • pnpm lint: exit 0 (0 warnings, 6 suggestion(s), same as main)
  • pnpm typecheck: exit 0
  • pnpm lint:i18n-gate: exit 0 (COVERAGE : 0 missing keys across 2 locale(s))
  • CLM_DEMO_SEED=1 pnpm validate: exit 0 (✓ Validation passed)

Acceptance notes

  • Finance cannot record or decide a finance review before the contract is approved. §04 grants clm_finance RCU(stage=finance) on clm_review. But writing a child needs edit on the parent, and finance's contract edit window opens at approved, after the stage a finance review belongs to (route_finance co-sign during review and approval). The 403 is above. I did not change this here: it is a product decision for §04, and the report raises it for the seat.
  • Review titles are English in zh-CN (Legal · Legal Counsel 1, Finance · …), because mirror.hook.ts hard-codes the stage labels. This was already so before this PR (Legal · Dev Admin). Carrier: Child record names are stored in English, so a zh-CN user reads "v2 · Clean", "Legal · Legal Counsel 1", "Wet ink · Completed" #157.
  • The seeded reviews' 所有者 (owner_id) still reads Dev Admin: the platform's seed-settle claim gives unowned rows to the platform admin. clm_review is controlled_by_parent, so owner_id grants nothing. Noted only.
  • LEGAL_HEAD / FINANCE_CONTROLLER copy the literal names of two demoStaff entries in objectstack.config.ts. That file is outside this card's surface, so it still spells them itself. Spelling them from keys.ts, as it already does for CONTRACT_OWNERS and LEGAL_OWNERS, would make a rename unable to drop rows silently. Proposed in the report.
  • The priming boot's remaining reason (rows written before the organization exists carry organization_id: null) was not re-measured on 18.2.0. This PR only retires the name check.

🤖 Generated with Claude Code

https://claude.ai/code/session_017nzWFRUo2U7evMkPdVniAn

All 60 seeded clm_review rows named the dev admin as reviewer, a choice
forced while it was the only account at seed time. Since PR #171 the demo
staff exist before the seed runs, so reviewerOf deals each review by stage:
legal to the contract's legal_owner (head of legal where there is none),
finance to the finance controller, compliance to the head of legal, and
business to the launching requester.

A finance review on a contract before `approved` goes to the head of legal:
clm_review is controlled_by_parent and finance edits contracts only from
approved onward, so the finance controller's PATCH of such a review answers
403 on 18.2.0.

No row depends on the dev admin's name any more, so scripts/demo.mjs drops
the name check from its priming boot; the sign-in check stays.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017nzWFRUo2U7evMkPdVniAn
A re-run re-deals `reviewer` and re-stamps the review's title, measured on
18.2.0; it also puts a decided review back to the fixture's decision, which
the README now lists beside deviations and signature rounds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017nzWFRUo2U7evMkPdVniAn
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 — ACCEPT · head 5d7606e · 2026-10-27T13:03Z

check reading
shape draft → main, Fixes #186; mergeable clean
CI on the head both Validate check-runs success on 5d7606e = PR head.sha
scope 4 files (+153/−121), all within the claim: reviewerOf() in src/data/keys.ts, the reviewer column in negotiation.seed.ts, scripts/demo.mjs (the DEMO_USER name check is retired; the priming boot stays for organization_id), and README.md. Nothing governed; lint 0 warnings
the deal the seat's stage mapping. Legal goes to the contract's legal_owner, or the legal head on the one handed-back contract. Finance goes to the finance controller. Compliance goes to the legal head (the plan deals none). Business goes to the launching requester. Counts: counsel1 16, counsel2 18, legal.head 8, finance 16, requester2 1, requester3 1; 0 of 60 name the Dev Admin
measured deviation (accepted) the 7 finance reviews on contracts not yet approved go to the legal head. clm_review is controlled_by_parent, finance edits a contract only from approved on, and finance@'s PATCH on such a review answers 403, on main as on this branch. The §04 question this raises goes to its own card (needs-user-decision)
live (zh-CN) counsel1 decides its legal review in the browser (PATCH 200, 结论时间 stamped). finance@ decides a finance review on an active contract (200). legal.head decides a pre-approval finance review (200). requester2 sees its business review
re-run a second and a third pnpm demo:zh deal the same reviewers, with no row refused. A decided review returns to its fixture decision, which the README now states (the same limit as deviations and signature rounds)
open questions Q2 (business review stays with the requester, who has R only): seat call A, since both seeded rows are already decided. Q3 (config's two staff names mirror keys.ts): the seat files a follow-up card. Q1 (finance before approval): needs-user-decision card. §10 reviewer text goes to PR #120

Landing: under the maintainer's merge authorization (project thread: 「你可以自己审核的就自己合并」).


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Seeded reviews name the Dev Admin as reviewer instead of the legal staff

2 participants