Repository navigation
Seeded reviews name the Dev Admin as reviewer instead of the legal staff - #189
Merged
Merged
Conversation
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
Contributor
Author
席位核对 — ACCEPT · head
|
| 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
zhuangjianguo
marked this pull request as ready for review
October 27, 2026 13:04
This was referenced Oct 27, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.mjsno longer checks the dev admin's name, because no seeded row depends on it any more.What changed
src/data/keys.ts: newreviewerOf(review), plusLEGAL_HEADandFINANCE_CONTROLLER(thedemoStaffaccount names). The rule by stage:legal_owner(legalOwnerOf), or Head of Legal when there is none. That happens once: the contract handed back to the queue.approvedor later. Before that, Head of Legal (deviation 1 below).ownerOf).The
DEMO_USERdoc is updated: no reference names the dev admin any more. Only theourside of theclm_signature.signersJSON 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 onreviewSeedsays whyreviewerstaysalways(it is dealt the same way aslegal_ownerandowner_id) and that a re-run puts a review's decision back to the fixture's.scripts/demo.mjs: removed theDEMO_USERconstant 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).signInnow 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)
approvedgo to Head of Legal (7 of 23).clm_reviewiscontrolled_by_parent, so writing a review needs edit access to its contract. Finance gets edit only fromapprovedonward (contract_finance_post_approval). Before that it only reads (contract_finance_routed). Signed in as finance@ on origin/main37ce10c:review_legal_update_any_stage, pluscontract_legal_all_headedit 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.clm_requesteronlyRonclm_review. requester2@PATCHon its own business review:403 PERMISSION_DENIED(「您没有执行此操作的权限…」) before and after. Both seeded business reviews are alreadyapproved, 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 DBBefore (origin/main
37ce10c, clean DB). As admin@,GET /api/v1/data/clm_reviewreturnedtotal 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.Legal · Legal Counsel 1: 14 approved + 2 pendingLegal · Head of Legal1,Finance · Head of Legal7Finance · Finance Controller16 (all 23 finance reviews visible)Business · Business Requester 21Business · Business Requester 31Browser: Chromium
/opt/pw-browsers/chromium, locale zh-CN, signed in through/_console/loginas each persona.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 byreview_decision_stamp).Finance · Finance Controller | 财务 | Finance Controller | 通过. I opened it, 编辑 › 结论 = 要求修改 › 更新. Result:PATCH … → 200, toast 「审查“Finance · Finance Controller”已更新」, and after reload 结论 reads 要求修改.Finance · Head of Legalon 主服务协议 — 极光系统 (审查中), 编辑 › 结论 = 要求修改 › 更新:PATCH … → 200. finance@'s APIPATCHon the same review:403as above.Business · Business Requester 2 | 业务 | Business Requester 2 | 通过.[object-ui]"conditional predicate failed to evaluate" warnings for the header actions. They are unrelated to this diff.After, run 2 (
pnpm demo:zhagain 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:decisionwas neverapplyOn: '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 AdminandBusiness · Dev Admin. After one morepnpm demo:zhthey readLegal · Legal Counsel 1,Finance · Finance ControllerandBusiness · Business Requester 2again. 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.mjswithout the name check, on all three runs:1/2 done — admin account ready.then2/2, with every review row landing on the first boot of a clean DB.Gates (final head
5d7606e; run 1–3 used0410379, which differs only in comments and README)pnpm install: exit 0pnpm validate: exit 0 (✓ Validation passed)pnpm lint: exit 0 (0 warnings,6 suggestion(s), same as main)pnpm typecheck: exit 0pnpm 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
clm_financeRCU(stage=finance)onclm_review. But writing a child needs edit on the parent, and finance's contract edit window opens atapproved, after the stage a finance review belongs to (route_financeco-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.Legal · Legal Counsel 1,Finance · …), becausemirror.hook.tshard-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.owner_id) still reads Dev Admin: the platform's seed-settle claim gives unowned rows to the platform admin.clm_reviewiscontrolled_by_parent, soowner_idgrants nothing. Noted only.LEGAL_HEAD/FINANCE_CONTROLLERcopy the literal names of twodemoStaffentries inobjectstack.config.ts. That file is outside this card's surface, so it still spells them itself. Spelling them fromkeys.ts, as it already does forCONTRACT_OWNERSandLEGAL_OWNERS, would make a rename unable to drop rows silently. Proposed in the report.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