Skip to content

[Decision] Finance cannot record its finance review before the contract is approved, though §04 grants it RCU on finance-stage reviews #190

Closed

Description

@objectstack-fleet

Ruled: 6607878401 · letter D · 2026-11-03T15:28:05Z

Raised by #186's dev (PR #189), measured on @objectstack/* 18.2.0, before and after that PR.

The conflict

  • DESIGN.md §04 matrix: clm_review · clm_finance RCU(stage=finance). src/profiles/finance.profile.ts declares review_finance_stage_insert / _update.
  • clm_review is controlled_by_parent on clm_contract. On 18.2.0, writing a child needs edit on its master.
  • Finance's contract edit window opens at approved (contract_finance_post_approval). Before that, the routed sharing rule (contract_finance_routed, PR The finance approver can open the contract it is asked to approve #153) gives finance read only.
  • So on a contract in in_review or in_approval, which is exactly when a finance review happens, finance@'s PATCH on its own finance-stage review answers 403 'requires edit access to its master record'. On an active contract, the same PATCH answers 200.

Interim: since PR #189, the demo deals the 7 pre-approval finance reviews to the legal head, who can write them.

Options

A. Keep the grants; write it down. §04 says legal records the finance review on finance's behalf until approval. No new grant. The demo already reads this way. Cost: finance cannot sign its own review in the window where it matters; legal types it in.

B. Give finance edit on contracts routed to it (contract_finance_routed read → edit), with field-level security locking every contract field, so in practice only the children open up. Cost: widens a sharing rule ruled in round 16, and every new contract field must remember the finance lock.

C. Ask the platform for a child write that does not need master edit (a controlled_by_parent exception, for example a per-child permission that admits a write to rows the caller may read the master of). Cost: an upstream card and a wait. It is a platform semantic, not a defect.

Dev's lean: A for now (no new grant). Seat's lean: B is the shape the business wants (finance signs its own review), but it is a §04 change; A is the honest interim.

Activity

  1. zhuangjianguo commented on Nov 3, 2026

    @zhuangjianguo
    Collaborator

    Ruling: batch #10 item 2 · letter D · maintainer 「同意」 2026-11-03T15:28:05Z

    D — finance records its review through a declared action; A (legal types it, PR #189) stays until D lands. Not B, not C.

    What the director checked before recording (hotclm main 7e7c2dc, 18.2.0; objectstack main):

    • The refusal is ADR-0055 §3's master-write check (plugin-security security-plugin-cbp-write-gate.ts): a controlled_by_parent detail is written only by a caller who may edit its master. That is the platform's declared semantic, the same as Salesforce's, so C would ask for an ADR change that applies to every detail object of every app. Not filed.
    • The approval lock (F5 lockRecord: true) holds only the contract row itself (plugin-approvals lifecycle-hooks.ts), not its children; the lock is not what refuses finance.
    • B's real cost: route_finance is stamped at submission, so an edit-level routed rule lets finance edit the contract from in_review on. Field locks are per set, not per row (A finance controller cannot launch a contract — clm_finance's field locks also apply to the draft they create #127), so either the finance lock covers every contract field, which ends finance's post-approval RU, or it keeps today's legal-field lock and finance can change amount, title, dates while legal is still reviewing. Amount drives the routing matrix. Not taken.
    • hotclm already has the shape: the lifecycle buttons (contract-lifecycle.actions.ts) and the backfill dialog (contract-backfill.actions.ts) are declared actions gated by requiredPermissions whose body writes system-elevated; the records manager's 发起签署 on a contract it can only read ([Decision] Who presses Start Signing? Records holds the capability but cannot read an approved contract, legal can read it but lacks the capability, and clm_admin cannot write the contract #100, ruling A) is exactly this case.

    Execution after the ruling (lane):

    1. Add a record_finance_review action on clm_contract: a dialog with decision, risk_level_assessed, comments; requiredPermissions on a token the finance set already holds; shown while the contract is in_review or in_approval; the body (system-elevated) creates the caller's stage: 'finance' review with reviewer = the caller, or updates it when one exists. It writes nothing on the contract.
    2. Measure with the finance demo account: the action records a review on an in_review contract (200) and the review shows under 审查与偏离; a direct PATCH on the contract and a direct POST of a clm_review row still answer 403; a requester does not see the button.
    3. Demo data: move the 7 pre-approval finance reviews back to the finance controller once the action exists.
    4. §04 matrix cell clm_review · clm_finance: keep RCU(stage=finance) and add 「经「记录财务审查」动作」 on the governed DESIGN.md PR.

    No platform card. Doc: https://claude.ai/artifact/QT2nrniJjuZMbwiT5v26Xq


    Generated by Claude Code

  2. objectstack-fleet commented on Nov 3, 2026

    @objectstack-fleet
    ContributorAuthor

    Claim: PM loop round 18
    Ruling-ref: 6607878401 (batch #10 item 2 · letter D)
    Session: session_017nzWFRUo2U7evMkPdVniAn
    Account: zhuangjianguo (the seat's linked user as GET /user answers it; the card's assignee)
    Branch: claude/issue-190-finance-review-action
    Worktree: hotclm-issue-190
    Domain: repo:hotclm (single-lane sister repo — no domain:*)
    Seat: repo:hotclm#1
    File surface: a new record_finance_review declared action on clm_contract (an actions file under src/actions/) and its translations · the finance grant token it requires (objectstack.config.ts capability + src/profiles/finance.profile.ts, only if no held token fits) · src/data/keys.ts / src/data/negotiation.seed.ts (move the 7 pre-approval finance reviews back to the finance controller) · README demo counts — stop on breach; explain in the report
    Container & model: S, mode:subagent, model: opus (TIER_DEFAULT; hand-derived for a sister repo), effort: medium
    Clause-②: no
    Metadata-protocol: none
    Responsibility: this app's finance review (finance records its own review while the contract is in review or approval) | none | finance controllers
    Thread-read: 6607878401
    Premise: re-measured by the dev on origin/main 7e7c2dc (@objectstack 18.2.0) with the declared demo staff
    Serial constraints cleared: round 17 ended with an empty queue; #152 is dispatched in parallel on disjoint files (party / clause objects).


    Generated by Claude Code

  3. objectstack-fleet commented on Nov 3, 2026

    @objectstack-fleet
    ContributorAuthor

    os-dev-report

    {
      "issue": 190,
      "status": "blocked",
      "branch": "claude/issue-190-finance-review-action",
      "pr": null,
      "session": "session_017nzWFRUo2U7evMkPdVniAn",
      "premise_still_valid": true,
      "summary": "I implemented ruling D and measured it end to end on 18.2.0 in zh-CN. The branch is pushed at 1692389. I did not open the PR, because the diff is outside the claim's file surface and the claim says stop on breach. The work is the new record_finance_review action (src/actions/finance-review.actions.ts) and a type:'screen', runAs:'system' flow of the same name (src/flows/finance-review.flow.ts), both gated on clm_finance.access, so no capability was added. The flow creates or updates the caller's stage=finance review on the contract the action route loaded in the caller's scope, and writes nothing on the contract. reviewerOf now deals all 23 finance reviews to the Finance Controller, and the README counts and prose follow. The breach is five files beyond the claimed actions file, translations, keys.ts and README: src/flows/finance-review.flow.ts, src/flows/index.ts and src/translations/{en,zh-CN}/flows.ts are the flow the dispatch prescribed after the #43 precedent, and src/pages/contract_detail.page.ts is needed because the custom record page's header lists its actions by id, so without that line the button does not appear. Once the seat widens the surface, the PR is one relay call: the actions file at scratchpad/issue-190/pr-actions.json, with the body in pr-body.md beside it.",
      "tests": "Before (origin/main 7e7c2dc, demo:zh, port 3163): reviews were Finance·Head of Legal 7 and Finance·Finance Controller 16. finance@ on POST /api/v1/actions/clm_contract/record_finance_review/ID got 404 'has no declaration'. finance@ POST clm_review and PATCH of the in_approval contract's finance review both got 403 'requires edit access to its master record'. The browser showed finance@ no 记录财务审查 button. After (HEAD 1692389), API as finance@: an in_review contract with no finance review returned 200, creating 1 row (reviewer finance@, changes_requested/medium/comment, decided_at stamped, created_by finance@). A second press with decision only returned 200; it was the same row id, still 1 row, now approved, with comments and risk kept. On an in_approval contract, create then update left 1 row. On the in_approval seeded review, the row was updated in place. Direct PATCH clm_contract returned 403, POST clm_review 403, and PATCH of finance@'s own in_review review 403. Submitted and active contracts were refused with the translated sentence and nothing was written. An unrouted in_review contract returned 404. The trigger route was refused with no record, even with a record smuggled into params. requester1 got 403 on both the action route and the trigger route ('requires capability [clm_finance.access]'). Browser, zh-CN: finance@ sees the button on in_review YES, in_approval YES, submitted NO, active NO. The dialog showed 结论*/评定风险/审查意见 with zh options. 确认 produced POST 200 and the toast 财务审查已记录。; a second press updated the same row; the 审查与偏离 tab lists Finance · Finance Controller 通过 中. requester1 saw no button on in_review MSA-2026-0001. Re-run: run 1 on a clean DB gave 1130 rows (reviews 60: Finance·Finance Controller 23, Legal·Head of Legal 1). Run 2 on the same DB showed the banner 'Seeds: app.objectstack.hotclm 1130 rows' with nothing refused; reviews were 62 = 60 plus the 2 the action made, untouched, with no duplicates. No ablation: metadata only, and no tested code path changed.",
      "gates": "HEAD 1692389, each exit captured before any pipe: pnpm install 0 · pnpm validate 0 (✓ Validation passed; 7 pre-existing warnings: hierarchy-security plus 6 contract_approval) · pnpm lint 0 (6 suggestions, 0 warnings, same as main) · pnpm typecheck 0 · pnpm lint:i18n-gate 0 (COVERAGE 0 missing across en, zh-CN) · CLM_DEMO_SEED=1 pnpm validate 0. CI: NOT MEASURED, no PR opened.",
      "dogfood_battery": "n/a — hotclm, no `!` in the title and no (narrowing) arm",
      "mcp_calls": "0",
      "api_writes": "1 — POST issues/190/comments (this os-dev-report, via post-stamped). git push is not a REST write.",
      "open_questions": [
        {
          "question": "Widen #190's claim surface to the five files the implementation needs, and open the prepared PR?",
          "options": [
            "A: widen the surface to src/flows/finance-review.flow.ts, src/flows/index.ts, src/translations/{en,zh-CN}/flows.ts and src/pages/contract_detail.page.ts, then send scratchpad/issue-190/pr-actions.json through the relay",
            "B: re-dispatch with the widened surface"
          ],
          "recommendation": "A. The branch is complete and measured. Nothing else in round 18 touches these files (#152 is on party/clause objects)."
        }
      ],
      "out_of_scope_findings": [
        "carrier: none · noted, not filed: every clm_review write moves the parent contract's updated_at (and updated_by on a caller-scoped write) through the platform's summary roll-up recompute. The action writes no contract field. Measured with a control (legal head direct review PATCH).",
        "carrier: none · noted, not filed: the 审查与偏离 tab badge read 1 while the related list showed 2 rows after the action's refreshAfter. Not investigated.",
        "carrier: none · noted, not filed: the seed's review externalId is (contract, stage) and the action keys on (contract, stage, reviewer), as ruled. On a DB seeded before this PR and not re-seeded, finance's press on a contract whose seeded finance review still names the head of legal adds a second finance-stage row. Not measured."
      ],
      "governed_text_proposals": {
        "DESIGN.md §04 matrix, clm_review · clm_finance": "RCU(stage=finance)经「记录财务审查」动作",
        "DESIGN.md §10 clm_review row (PR #120 wording), review-dealing sentence": "审查人按环节分派:法务 → 合同法务负责人(无则法务负责人);财务 → 财务负责人;合规 → 法务负责人;业务 → 发起人"
      }
    }

    Generated by Claude Code

  4. objectstack-fleet commented on Nov 3, 2026

    @objectstack-fleet
    ContributorAuthor

    Seat call — open question A · claim amended · 2026-11-03

    The claim's file surface widens to the five files the ruling's shape needs, and the seat opens the prepared PR:

    Nothing else in round 18 touches these files (#152 landed as PR #193 on the party and clause objects).


    Generated by Claude Code

  5. objectstack-fleet commented on Nov 3, 2026

    @objectstack-fleet
    ContributorAuthor

    Ruling-ref: 6607878401 (batch #10 item 2 · letter D)

    Landed — PR #194 · 2026-11-03

    reading value
    PR #194 MERGED (squash on main), merged at its ACCEPT head 1692389; Fixes #190 closes this card
    authority maintainer, project thread, verbatim: 「你可以自己审核的就自己合并」

    Release: finance now records its own review while a contract is in review or in approval, through the 「记录财务审查」 button on the contract; pressing it again updates that review. The demo's finance reviews all sit with the finance controller again. State: pm:dispatched removed, assignee cleared.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions