From 7b7017bc2c1ee9ff40508d1b89d614bc0f290fe4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 18:30:19 +0000 Subject: [PATCH 1/2] fix(app-showcase): write the expense approvals' decision back onto the report showcase_expense_signoff and showcase_committee_quorum ended both outcome arms on bare end nodes, so an approved expense report kept reading Submitted. Each arm now runs an update_record (approve -> approved, reject -> draft) under runAs: system, and a showcase test drives a real decision through the approvals service on both flows with a pre-fix control. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016g7nYP9wdNx1GDa5XDr7Uy --- .../src/automation/flows/index.ts | 82 ++++++- .../test/expense-approval-write-back.test.ts | 219 ++++++++++++++++++ 2 files changed, 297 insertions(+), 4 deletions(-) create mode 100644 examples/app-showcase/test/expense-approval-write-back.test.ts diff --git a/examples/app-showcase/src/automation/flows/index.ts b/examples/app-showcase/src/automation/flows/index.ts index 76f739e24df..a47c39e3548 100644 --- a/examples/app-showcase/src/automation/flows/index.ts +++ b/examples/app-showcase/src/automation/flows/index.ts @@ -1747,6 +1747,29 @@ export const InquiryPurgeFlow = defineFlow({ * pattern (钉钉/Salesforce style) that needs no parallel branches. Set * `minApprovals` > 1 for "two from each group"; use `behavior: 'quorum'` + * `minApprovals` for M-of-N collective sign-off. + * + * **The decision is written back onto the report** — an explicit + * `update_record` on each outcome branch: approve → `status: 'approved'`, + * reject → `status: 'draft'` (back to the submitter, who can rework and + * resubmit — the same revert-on-reject {@link InvoiceDualSignoffFlow} does). + * Without these two nodes both outcomes were bare `end`s, so an expense report + * both groups had approved kept reading "Submitted" forever. + * + * Why not the approval node's `approvalStatusField` mirror instead: the mirror + * writes the REQUEST's status vocabulary verbatim (`pending` on open, + * `approved` / `rejected` on decide, `recalled`, `returned`), has no value + * mapping, and writes only values the target field declares among its options + * (anything else is skipped with a warn). `status` here is the report's own + * lifecycle select (`draft` / `submitted` / `approved` / `reimbursed`) — it + * declares neither `pending` nor `rejected`, so a mirror onto it would warn on + * every submit and leave a REJECTED report reading "Submitted", the very + * defect being fixed. The spec also advises a mirror target be readonly, and + * this one is the field the submitter edits. A business-lifecycle field + * driven by the decision is what `update_record` per branch is for. + * + * `runAs: 'system'` for the same reason as the invoice flow: the write-back is + * an outcome of the approval process, not an act of the submitter, so it must + * land whatever the submitter's edit rights on a submitted report are. */ export const ExpenseSignoffFlow = defineFlow({ name: 'showcase_expense_signoff', @@ -1754,6 +1777,7 @@ export const ExpenseSignoffFlow = defineFlow({ description: 'Manager + finance per-group sign-off (会签) on a submitted expense report (#3266).', type: 'autolaunched', status: 'active', + runAs: 'system', nodes: [ { id: 'start', @@ -1779,13 +1803,35 @@ export const ExpenseSignoffFlow = defineFlow({ lockRecord: true, }, }, + { + id: 'mark_approved', + type: 'update_record', + label: 'Mark Report Approved', + config: { + objectName: 'showcase_expense_report', + filter: { id: { dialect: 'cel', source: 'record.id' } }, + fields: { status: 'approved' }, + }, + }, + { + id: 'return_to_draft', + type: 'update_record', + label: 'Return Report to Draft', + config: { + objectName: 'showcase_expense_report', + filter: { id: { dialect: 'cel', source: 'record.id' } }, + fields: { status: 'draft' }, + }, + }, { id: 'approved', type: 'end', label: 'Approved' }, { id: 'rejected', type: 'end', label: 'Rejected' }, ], edges: [ { id: 'e1', source: 'start', target: 'dual_signoff' }, - { id: 'e2', source: 'dual_signoff', target: 'approved', label: 'approve' }, - { id: 'e3', source: 'dual_signoff', target: 'rejected', label: 'reject' }, + { id: 'e2', source: 'dual_signoff', target: 'mark_approved', label: 'approve' }, + { id: 'e3', source: 'dual_signoff', target: 'return_to_draft', label: 'reject' }, + { id: 'e4', source: 'mark_approved', target: 'approved' }, + { id: 'e5', source: 'return_to_draft', target: 'rejected' }, ], }); @@ -1797,6 +1843,11 @@ export const ExpenseSignoffFlow = defineFlow({ * `minApprovals` clamps to the approver count so a misconfiguration can never * deadlock. Where `per_group` requires one from EACH group, `quorum` counts a * flat tally across all approvers. + * + * The decision is written back onto the report exactly as in + * {@link ExpenseSignoffFlow} (approve → `approved`, reject → `draft`), and for + * the same reasons — see that flow's docblock for why this is an + * `update_record` per branch and not an `approvalStatusField` mirror. */ export const CommitteeQuorumFlow = defineFlow({ name: 'showcase_committee_quorum', @@ -1804,6 +1855,7 @@ export const CommitteeQuorumFlow = defineFlow({ description: 'Any 2-of-3 committee members approve a high-value expense report (#3266 quorum / M-of-N).', type: 'autolaunched', status: 'active', + runAs: 'system', nodes: [ { id: 'start', @@ -1830,13 +1882,35 @@ export const CommitteeQuorumFlow = defineFlow({ lockRecord: true, }, }, + { + id: 'mark_approved', + type: 'update_record', + label: 'Mark Report Approved', + config: { + objectName: 'showcase_expense_report', + filter: { id: { dialect: 'cel', source: 'record.id' } }, + fields: { status: 'approved' }, + }, + }, + { + id: 'return_to_draft', + type: 'update_record', + label: 'Return Report to Draft', + config: { + objectName: 'showcase_expense_report', + filter: { id: { dialect: 'cel', source: 'record.id' } }, + fields: { status: 'draft' }, + }, + }, { id: 'approved', type: 'end', label: 'Approved' }, { id: 'rejected', type: 'end', label: 'Rejected' }, ], edges: [ { id: 'e1', source: 'start', target: 'committee' }, - { id: 'e2', source: 'committee', target: 'approved', label: 'approve' }, - { id: 'e3', source: 'committee', target: 'rejected', label: 'reject' }, + { id: 'e2', source: 'committee', target: 'mark_approved', label: 'approve' }, + { id: 'e3', source: 'committee', target: 'return_to_draft', label: 'reject' }, + { id: 'e4', source: 'mark_approved', target: 'approved' }, + { id: 'e5', source: 'return_to_draft', target: 'rejected' }, ], }); diff --git a/examples/app-showcase/test/expense-approval-write-back.test.ts b/examples/app-showcase/test/expense-approval-write-back.test.ts new file mode 100644 index 00000000000..f63c6be2b5b --- /dev/null +++ b/examples/app-showcase/test/expense-approval-write-back.test.ts @@ -0,0 +1,219 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25469] The expense approvals write their decision back onto the report. + * + * THE DEFECT. `showcase_expense_signoff` (per-group 会签) and + * `showcase_committee_quorum` (2-of-3) both ran `start → approval → end`, with + * bare `end` nodes on the approve and reject arms. The approval request moved + * to approved / rejected, but nothing told the business record: an expense + * report both groups had signed off kept reading `status: 'submitted'`. + * + * THE FIX is an `update_record` on each outcome arm (approve → `approved`, + * reject → `draft`), not an `approvalStatusField` mirror — the flows' own + * docblock says why (the mirror writes the request vocabulary verbatim, and + * `status` declares neither `pending` nor `rejected`, so a rejected report + * would still read "Submitted"). + * + * WHY A REAL DECISION AND NOT A METADATA READ. The write-back happens on + * RESUME, after a pause, through the approvals service's `decide` → + * `automation.resume` path, and under the record lock the node takes. Only + * driving a real request to a real decision on the production stack (real + * kernel, ObjectQL, automation and approvals plugins over real sqlite, the + * app's real objects and flows) shows the record actually changes. The + * CONTROL re-authors the pre-fix shape (bare `end` arms) on the same harness + * and shows the same decisions leave the report at `submitted` — so the + * positive assertions can go red. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQLPlugin } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation'; +import { ApprovalsServicePlugin } from '@objectstack/plugin-approvals'; + +import { ExpenseReport, ExpenseLine } from '../src/data/objects/index.js'; +import { ExpenseSignoffFlow, CommitteeQuorumFlow } from '../src/automation/flows/index.js'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +/** Everything the approvals service treats as an unconditional actor. */ +const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as any; +const SYS = { context: SYSTEM_CTX }; + +const openKernels: Array<{ shutdown?: () => Promise }> = []; +const openDrivers: Array<{ disconnect?: () => Promise }> = []; +afterEach(async () => { + while (openKernels.length) { + try { await openKernels.pop()?.shutdown?.(); } catch { /* noop */ } + } + while (openDrivers.length) { + try { await openDrivers.pop()?.disconnect?.(); } catch { /* noop */ } + } +}); + +interface Booted { + automation: AutomationEngine & Record; + approvals: any; + data: any; +} + +async function boot(flow: any): Promise { + const kernel = new ObjectKernel({ logger: { level: 'silent' } } as any); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(new AutomationServicePlugin()); + await kernel.use(new ApprovalsServicePlugin()); + await kernel.bootstrap(); + openKernels.push(kernel as any); + + const objectql: any = kernel.getService('objectql'); + const data: any = kernel.getService('data'); + const automation = kernel.getService('automation') as AutomationEngine & Record; + const approvals: any = kernel.getService('approvals'); + + const driver: any = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await driver.connect(); + objectql.registerDriver(driver, true); + openDrivers.push(driver); + + for (const obj of [ExpenseReport, ExpenseLine]) { + objectql.registry.registerObject(obj, 'showcase', 'showcase'); + } + await objectql.syncSchemas(); + await installDefaultOrganization(objectql); + + // One flow per boot: both start on the same record transition, and the + // approvals service dedupes pending requests per record. + automation.registerFlow(flow.name, flow); + return { automation, approvals, data }; +} + +function one(res: any): any { + return Array.isArray(res) ? res[0] : res; +} + +function rows(res: any): any[] { + return Array.isArray(res) ? res : (res?.records ?? []); +} + +/** A submitted report, opened into approval the way the seed's `launchSignoff` does. */ +async function submitReport(booted: Booted, flowName: string): Promise<{ report: any; request: any }> { + const report = one(await booted.data.insert('showcase_expense_report', { + name: 'EXP-TEST', + employee: 'Mei Phone', + status: 'submitted', + }, SYS)); + const launched: any = await booted.automation.execute(flowName, { + record: report, + previous: { ...report, status: 'draft' }, + object: 'showcase_expense_report', + organizationId: null, + userId: 'mei@example.com', + } as any); + expect(launched.status, `launch did not park on the approval node: ${launched.error ?? ''}`).toBe('paused'); + const request = rows(await booted.data.find('sys_approval_request', { where: { status: 'pending' }, ...SYS }))[0]; + expect(request, 'no pending approval request was opened').toBeTruthy(); + return { report, request }; +} + +async function statusOf(booted: Booted, id: string): Promise { + const rec = one(await booted.data.findOne('showcase_expense_report', { where: { id }, ...SYS })); + return rec?.status; +} + +async function decide(booted: Booted, requestId: string, decision: 'approve' | 'reject', actorId: string) { + return booted.approvals.decide(requestId, { decision, actorId }, SYSTEM_CTX); +} + +/** The pre-fix shape: the outcome arms go straight to bare `end` nodes. */ +function withoutWriteBack(flow: any, approvalNodeId: string): any { + const writeBack = new Set(['mark_approved', 'return_to_draft']); + return { + ...flow, + nodes: (flow.nodes as any[]).filter((n) => !writeBack.has(n.id)), + edges: [ + { id: 'e1', source: 'start', target: approvalNodeId }, + { id: 'e2', source: approvalNodeId, target: 'approved', label: 'approve' }, + { id: 'e3', source: approvalNodeId, target: 'rejected', label: 'reject' }, + ], + }; +} + +const CASES = [ + { + flow: ExpenseSignoffFlow, + node: 'dual_signoff', + // per_group: one from EACH group finalizes. + approvers: ['position:manager', 'position:auditor'], + rejecter: 'position:auditor', + }, + { + flow: CommitteeQuorumFlow, + node: 'committee', + // quorum: any 2 of 3 finalizes. + approvers: ['position:manager', 'position:legal'], + rejecter: 'position:finance', + }, +] as const; + +describe.each(CASES)('#25469 — $flow.name writes its decision back onto the report', ({ flow, node, approvers, rejecter }) => { + it('approve: the report reads `approved` once the approval finalizes, not before', async () => { + const booted = await boot(flow); + const { report, request } = await submitReport(booted, flow.name); + + const first = await decide(booted, request.id, 'approve', approvers[0]); + expect(first.finalized, 'one approval already finalized the request').toBe(false); + // Still mid-approval: nothing has been decided yet, so nothing is written. + expect(await statusOf(booted, report.id)).toBe('submitted'); + + const second = await decide(booted, request.id, 'approve', approvers[1]); + expect(second.finalized).toBe(true); + expect(second.resumed, `the run was not resumed: ${second.error ?? second.resumeError ?? ''}`).toBe(true); + + const run = (await booted.automation.listRuns(flow.name))[0]; + expect(run.status, `run ended ${run.status}: ${run.error ?? ''}`).toBe('completed'); + expect(run.steps.map((s: any) => s.nodeId)).toContain('mark_approved'); + expect(run.steps.map((s: any) => s.nodeId)).not.toContain('return_to_draft'); + + expect(await statusOf(booted, report.id)).toBe('approved'); + }, 30_000); + + it('reject: the report goes back to `draft`, a value its status field declares', async () => { + const booted = await boot(flow); + const { report, request } = await submitReport(booted, flow.name); + + const res = await decide(booted, request.id, 'reject', rejecter); + expect(res.finalized).toBe(true); + expect(res.resumed, `the run was not resumed: ${res.error ?? res.resumeError ?? ''}`).toBe(true); + + const run = (await booted.automation.listRuns(flow.name))[0]; + expect(run.status, `run ended ${run.status}: ${run.error ?? ''}`).toBe('completed'); + expect(run.steps.map((s: any) => s.nodeId)).toContain('return_to_draft'); + + expect(await statusOf(booted, report.id)).toBe('draft'); + const declared = ((ExpenseReport as any).fields.status.options as Array<{ value: string }>).map((o) => o.value); + expect(declared).toEqual(expect.arrayContaining(['approved', 'draft'])); + }, 30_000); + + it('CONTROL — the pre-fix shape (bare `end` arms): the same decisions leave the report `submitted`', async () => { + const preFix = withoutWriteBack(flow, node); + // The rollback must actually be a rollback. + expect((preFix.nodes as any[]).some((n) => n.type === 'update_record')).toBe(false); + + const booted = await boot(preFix); + const { report, request } = await submitReport(booted, flow.name); + await decide(booted, request.id, 'approve', approvers[0]); + const second = await decide(booted, request.id, 'approve', approvers[1]); + expect(second.finalized).toBe(true); + expect(second.resumed).toBe(true); + + const run = (await booted.automation.listRuns(flow.name))[0]; + expect(run.status).toBe('completed'); + // The defect as reported: approved request, report still "Submitted". + expect(await statusOf(booted, report.id)).toBe('submitted'); + }, 30_000); +}); From 3e661a73f69027cb59d2ca45edeb291ab11d3bd4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 18:48:39 +0000 Subject: [PATCH 2/2] test(app-showcase): seed a line so the committee quorum gate opens Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016g7nYP9wdNx1GDa5XDr7Uy --- .../test/expense-approval-write-back.test.ts | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/examples/app-showcase/test/expense-approval-write-back.test.ts b/examples/app-showcase/test/expense-approval-write-back.test.ts index f63c6be2b5b..1ec2d62b2b9 100644 --- a/examples/app-showcase/test/expense-approval-write-back.test.ts +++ b/examples/app-showcase/test/expense-approval-write-back.test.ts @@ -101,12 +101,18 @@ function rows(res: any): any[] { } /** A submitted report, opened into approval the way the seed's `launchSignoff` does. */ -async function submitReport(booted: Booted, flowName: string): Promise<{ report: any; request: any }> { - const report = one(await booted.data.insert('showcase_expense_report', { +async function submitReport(booted: Booted, flowName: string, amount: number): Promise<{ report: any; request: any }> { + const created = one(await booted.data.insert('showcase_expense_report', { name: 'EXP-TEST', employee: 'Mei Phone', status: 'submitted', }, SYS)); + // One line, so the `total_amount` roll-up the committee gate reads is real. + await booted.data.insert('showcase_expense_line', { + expense_report: created.id, merchant: 'Acme Travel', category: 'travel', amount, status: 'submitted', + }, SYS); + const report = one(await booted.data.findOne('showcase_expense_report', { where: { id: created.id }, ...SYS })); + expect(Number(report.total_amount)).toBe(amount); const launched: any = await booted.automation.execute(flowName, { record: report, previous: { ...report, status: 'draft' }, @@ -147,6 +153,8 @@ const CASES = [ { flow: ExpenseSignoffFlow, node: 'dual_signoff', + // Under the $5,000 committee threshold, as the seeded EXP-2001 is. + amount: 1500, // per_group: one from EACH group finalizes. approvers: ['position:manager', 'position:auditor'], rejecter: 'position:auditor', @@ -154,16 +162,18 @@ const CASES = [ { flow: CommitteeQuorumFlow, node: 'committee', + // At or above the $5,000 the committee's start gate requires (EXP-DEMO: 8,900). + amount: 8900, // quorum: any 2 of 3 finalizes. approvers: ['position:manager', 'position:legal'], rejecter: 'position:finance', }, ] as const; -describe.each(CASES)('#25469 — $flow.name writes its decision back onto the report', ({ flow, node, approvers, rejecter }) => { +describe.each(CASES)('#25469 — $flow.name writes its decision back onto the report', ({ flow, node, amount, approvers, rejecter }) => { it('approve: the report reads `approved` once the approval finalizes, not before', async () => { const booted = await boot(flow); - const { report, request } = await submitReport(booted, flow.name); + const { report, request } = await submitReport(booted, flow.name, amount); const first = await decide(booted, request.id, 'approve', approvers[0]); expect(first.finalized, 'one approval already finalized the request').toBe(false); @@ -184,7 +194,7 @@ describe.each(CASES)('#25469 — $flow.name writes its decision back onto the re it('reject: the report goes back to `draft`, a value its status field declares', async () => { const booted = await boot(flow); - const { report, request } = await submitReport(booted, flow.name); + const { report, request } = await submitReport(booted, flow.name, amount); const res = await decide(booted, request.id, 'reject', rejecter); expect(res.finalized).toBe(true); @@ -205,7 +215,7 @@ describe.each(CASES)('#25469 — $flow.name writes its decision back onto the re expect((preFix.nodes as any[]).some((n) => n.type === 'update_record')).toBe(false); const booted = await boot(preFix); - const { report, request } = await submitReport(booted, flow.name); + const { report, request } = await submitReport(booted, flow.name, amount); await decide(booted, request.id, 'approve', approvers[0]); const second = await decide(booted, request.id, 'approve', approvers[1]); expect(second.finalized).toBe(true);