Finance records its own review through a Record Finance Review action

README.md

@@ -94,8 +94,8 @@ console does not list them; this table is where they are named.
 |---|---|---|---|
 | `Business Requester 1` · `2` · `3` | `requester1@example.com` · `requester2@example.com` · `requester3@example.com` | none — every member holds `clm_requester` | the 120 contracts' `owner_id`, dealt by counterparty — 43 / 43 / 34 · and the 143 delivery and reporting obligations under them — 50 / 53 / 40 · the 2 business reviews — 0 / 1 / 1 |
 | `Legal Counsel 1` · `2` | `counsel1@example.com` · `counsel2@example.com` | `clm_legal_counsel` | receives contracts on submission (round-robin by open load) and owns review · the `legal_owner` of every contract legal has accepted — 34 / 34 · all 56 compliance obligations — 29 / 27 · the `decided_by` of all 17 decided deviations — 10 / 7 · the reviewer of their contracts' 34 legal reviews — 16 / 18, the 7 still pending among them — 2 / 5 |
-| `Head of Legal` | `legal.head@example.com` | `clm_legal_head` | rung 2 of the approval ladder whenever `route_legal_head` is stamped · the reviewer of 8 reviews: the legal review of the contract handed back to the queue, and the 7 finance reviews on contracts not yet approved |
-| `Finance Controller` | `finance@example.com` | `clm_finance_controller` | rung 2/3, reads every contract with `route_finance`, and the only position that may edit bank details · the reviewer of the 16 finance reviews on approved-or-later contracts |
+| `Head of Legal` | `legal.head@example.com` | `clm_legal_head` | rung 2 of the approval ladder whenever `route_legal_head` is stamped · the reviewer of the legal review of the contract handed back to the queue |
+| `Finance Controller` | `finance@example.com` | `clm_finance_controller` | rung 2/3, reads every contract with `route_finance`, and the only position that may edit bank details · the reviewer of all 23 finance reviews, the 7 on contracts still in review or in approval among them, recorded there through **Record Finance Review** (记录财务审查) |
 | `Executive` | `executive@example.com` | `clm_executive` | rung 4, and reads every contract with `route_executive` |
 | `General Manager` | `gm@example.com` | `clm_general_manager` | rung 5, and reads every contract with `route_gm` |
 | `Records Manager` | `records@example.com` | `clm_records_manager` | execution formalities, the executed copy and the archive number |
@@ -172,11 +172,13 @@ reminders go to somebody who is not going to act on them.
 
 The 60 reviews are dealt to the desk whose stage they are. A legal review is the
 contract's lawyer's, or the head of legal's on the one contract handed back to
-the queue. A finance review is the finance controller's once its contract is
-approved, and the head of legal's before that, because finance edits a contract
-(and so its reviews) only from `approved` onward. A business review is the
-requester's who launched the contract. A compliance review would be the head of
-legal's; the demo deals none. `src/data/keys.ts` (`reviewerOf`) says why each.
+the queue. A finance review is the finance controller's, before approval as well
+as after: finance edits a contract (and so writes its reviews directly) only
+from `approved` onward, so on a contract still in review or in approval it
+records its review through the **Record Finance Review** (记录财务审查) button
+on the contract instead. A business review is the requester's who launched the
+contract. A compliance review would be the head of legal's; the demo deals
+none. `src/data/keys.ts` (`reviewerOf`) says why each.
 
 ### Rung 1 of the approval ladder is a manager
 

src/actions/finance-review.actions.ts

@@ -0,0 +1,64 @@
+import type { Action } from '@objectstack/spec/ui';
+import { P } from '@objectstack/spec';
+
+/**
+ * 记录财务审查 — finance records its own review of a contract in review or in
+ * approval (issue #190, ruling D, 2026-11-03).
+ *
+ * DESIGN.md §04 grants `clm_finance` "RCU（stage=finance）" on `clm_review`,
+ * but a `controlled_by_parent` detail is written only by a caller who may
+ * edit its master, and finance edits a contract only from `approved` onward.
+ * The ruling keeps finance off the contract and gives the review its own
+ * door instead: this button starts `record_finance_review`
+ * (`src/flows/finance-review.flow.ts`), which writes the caller's
+ * `stage: 'finance'` review system-elevated, creating it or updating the one
+ * they already gave, and writes nothing on the contract. The flow's header
+ * says what the elevation does and does not buy.
+ *
+ * ## The gate: `clm_finance.access`
+ *
+ * A token `clm_finance` already holds and no other set grants (§04), the
+ * same way `clm_legal.access` serves the legal-only actions; no capability
+ * is added. Declared twice with the same list, as `executed_upload` does:
+ * the flow's `requiredPermissions` stops the request at every door that
+ * starts it by name, and this copy hides the button, because the console's
+ * action strip and MCP `list_actions` read the action, not the flow behind
+ * it (objectstack#25643). The action route also refuses a caller without it,
+ * 403, before the flow is looked at.
+ *
+ * ## `visible` is a convenience, not the control
+ *
+ * Shown while the contract is `in_review` or `in_approval`, the window in
+ * which finance cannot write its review any other way. From `approved`
+ * onward finance edits the review directly, as §04 has always said. The
+ * flow repeats the status test as a refused end, so a caller who reaches the
+ * route on a contract in any other state is told so and nothing is written.
+ *
+ * ## The dialog
+ *
+ * The three fields of the review the reviewer decides, field-backed on
+ * `clm_review` (`objectOverride`) so each inherits that field's label, type
+ * and options in both locales. `decision` is required: a review recorded
+ * with no decision is not a review. The reviewer, the stage and the contract
+ * are not asked: the caller, `finance`, and the contract on screen.
+ */
+export const RecordFinanceReviewAction: Action = {
+  name: 'record_finance_review',
+  label: 'Record Finance Review',
+  objectName: 'clm_contract',
+  icon: 'calculator',
+  type: 'flow',
+  target: 'record_finance_review',
+  locations: ['record_header'],
+  description: 'Record your finance review of this contract. Pressing it again updates the review you already gave; a field left empty keeps what the review says. The contract itself is not changed.',
+  visible: P`record.status == 'in_review' || record.status == 'in_approval'`,
+  // The same list as the flow's (see the header): this copy hides the button.
+  requiredPermissions: ['clm_finance.access'],
+  params: [
+    { field: 'decision', objectOverride: 'clm_review', required: true },
+    { field: 'risk_level_assessed', objectOverride: 'clm_review' },
+    { field: 'comments', objectOverride: 'clm_review', helpText: 'Visible to the requester.' },
+  ],
+  successMessage: 'Finance review recorded.',
+  refreshAfter: true,
+};

src/actions/index.ts

@@ -11,3 +11,4 @@ export {
   StartRenewalAction,
 } from './contract-lifecycle.actions.js';
 export { ExecutedUploadAction } from './contract-backfill.actions.js';
+export { RecordFinanceReviewAction } from './finance-review.actions.js';

src/data/keys.ts

@@ -1,7 +1,6 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { STRINGS } from './demo-locale.js';
-import { POST_APPROVAL_STATUSES } from '../sharing/_lifecycle.js';
 import type { ObligationPlan, ReviewPlan, VersionPlan } from './plan-children.js';
 import { CONTRACT_PLAN, type ContractPlan, type DeviationPlan } from './plan-contracts.js';
 import { assertUniqueKeys } from './_shared.js';
@@ -348,7 +347,8 @@ export const FINANCE_CONTROLLER = 'Finance Controller';
  *    nobody (the contract handed back to the queue), the head of legal, who
  *    receives that queue's escalations.
  *  - **finance** — the finance controller, whose `clm_finance` holds
- *    `RCU（stage=finance）` on this object.
+ *    `RCU（stage=finance）` on this object, on every contract the matrix
+ *    routed to it, approved or not (see below).
  *  - **compliance** — the head of legal: compliance is legal's work (the
  *    argument {@link obligationOwnerOf} makes for compliance obligations) and
  *    a high-risk contract's compliance sign-off is the senior lawyer's.
@@ -357,22 +357,21 @@ export const FINANCE_CONTROLLER = 'Finance Controller';
  *  - **business** — the requester who launched the contract, {@link ownerOf}:
  *    the business review is the launching desk's own sign-off.
  *
- * ## Where the rule bends, measured on 18.2.0
+ * ## A finance review before approval is still finance's
  *
- * A finance review on a contract that has not reached `approved` goes to the
- * head of legal, not the finance controller. `clm_review` is
- * `controlled_by_parent`, so writing one needs EDIT on its contract, and
+ * 7 of the 23 finance reviews sit on a contract that has not reached
+ * `approved` (6 `in_review`, 1 `in_approval`, every one routed to finance).
+ * Finance cannot WRITE a review there directly: `clm_review` is
+ * `controlled_by_parent`, so a direct write needs edit on its contract, and
  * finance edits contracts only from `approved` onward
- * (`contract_finance_post_approval`); before that it READS the contracts the
- * matrix routed to it (`contract_finance_routed`) and no more. Measured: the
- * finance controller's `PATCH` of a finance review on an `in_review` or an
- * `in_approval` contract answers `403 PERMISSION_DENIED` ("requires edit
- * access to its master record"), the same write on an `active` contract `200`.
- * So on those rows the finance controller could neither have decided the
- * review it would be named for nor change it now; the head of legal can
- * (`review_legal_update_any_stage`, and `contract_legal_all_head` edits every
- * contract), and is the account the rule already falls back to. 7 of the 23
- * finance reviews sit on such a contract.
+ * (`contract_finance_post_approval`) — measured on 18.2.0, finance@'s
+ * `PATCH` of such a review answers `403` "requires edit access to its master
+ * record". Until issue #190 those 7 went to the head of legal, who could.
+ * Since #190 (ruling D) finance records and updates its own review there
+ * through the 记录财务审查 action (`record_finance_review`,
+ * `src/flows/finance-review.flow.ts`), which is shown on exactly those two
+ * states, so the finance controller is the reviewer on all 23, and the review
+ * the account is named for is one it can change.
  *
  * A business review stays the requester's although a requester cannot change
  * it: `clm_requester` holds `R` on this object (§04), and both seeded business
@@ -388,7 +387,7 @@ export const reviewerOf = (review: ReviewPlan): string => {
     case 'legal':
       return legalOwnerOf(contract) ?? LEGAL_HEAD;
     case 'finance':
-      return POST_APPROVAL_STATUSES.includes(contract.status) ? FINANCE_CONTROLLER : LEGAL_HEAD;
+      return FINANCE_CONTROLLER;
     case 'compliance':
       return LEGAL_HEAD;
     case 'business':