fix(flows): 记录变更流以平台身份运行，补齐调度流早已declare的 runAs (#684)

.changeset/record-change-flows-run-as-system.md

@@ -0,0 +1,66 @@
+---
+'hotcrm': patch
+---
+
+Give the record-change flows the execution identity the scheduled ones already
+had, so system-driven writes stop silently losing their automation — including
+an approval gate that a machine-created deal could walk straight through.
+
+A flow's `runAs` decides who its data operations execute as. Under the schema
+default `'user'`, a run that resolved **no trigger user** has no identity to
+scope to, so the engine refuses its data operations rather than run them
+unscoped. The part that was missed here is that "no trigger user" is not a
+schedule-only condition: a record-change flow is fired by a **write**, and a
+write made without a session — seed loading, an integration or webhook, or
+another `runAs: 'system'` flow's own write — carries no user into the run it
+triggers. Ten scheduled flows had already been given `runAs: 'system'`, each
+with an authored comment saying exactly that; all seven record-change flows
+were left on the default. The platform's build-time lint cannot close the gap,
+because whether a record-change trigger carries a user is only knowable at run
+time.
+
+Measured on `@objectstack/*` 17.0.0-rc.2, driving the real automation engine
+with no trigger user:
+
+- **`case_escalation`** (and its insert-time twin) died at its first data node
+  with `[runAs] refusing a data operation`. A case raised by the seed loader or
+  an integration kept its critical priority forever — a freshly seeded org had
+  never once run this automation over its own data.
+- **`opportunity_approval`** (and its twin) died the same way, at
+  `get_opportunity`, before the approval request was ever opened. A $150K
+  renewal created by the `runAs: 'system'` contract_renewal sweep therefore sat
+  at `approval_status: 'not_required'`, unlocked, with no approval on record.
+  An approval control that engages only for logged-in writers is not a control.
+- **`lead_assignment`** died at its SLA stamp, so a lead arriving from
+  web-to-lead, a CSV import or a partner integration got no follow-up date and
+  no alert at all.
+
+All seven record-change flows now declare `runAs: 'system'` with a per-flow
+rationale in source. The declaration elevates the **user-driven** runs too, so
+that is argued per flow rather than assumed: every data node in these flows is
+keyed to `{record.id}` — the row that just fired the trigger — so user scope
+adds no restriction that matters while adding a failure mode (a rep who may
+create a lead, or an agent who may raise a case, is not thereby granted edit
+rights on it), and for `opportunity_approval` the submitter's own scope is
+positively the wrong identity for a gate that exists to constrain the
+submitter. No flow in this set relies on the triggering user's restricted scope.
+
+The issue's premise held for three of the seven; the remaining four
+(`contact_welcome`, `task_urgent_alert`, `opportunity_won_alert`,
+`case_csat_followup`) were measured **not** refused, and that measurement is
+recorded rather than papered over. They are notify-only, the refusal covers
+`get`/`create`/`update`/`delete` record nodes, and `notify` dispatches through
+the messaging service without a run data context — so a user-less run of those
+completes and delivers today. They carry the declaration anyway because the
+decision worth recording is "record-change automation runs as the platform",
+not "this flow currently happens to have no data node": the day one gains a
+data node it inherits an elevation that was reasoned about, instead of
+discovering a refusal in production.
+
+Pinned two ways so the gap cannot return. `test/actions-flows-integrity.test.ts`
+enumerates record-change flows **from the compiled stack** — not a hand-kept
+import list — and requires each to declare `runAs: 'system'` with an authored
+rationale beside it. `test/flow-record-change.test.ts` runs the real engine
+with no trigger user and asserts the writes land, with the pre-fix shape
+reproduced on demand (`runAs` stripped) so the assertions cannot pass by
+producing nothing. Refs #684, ADR-0049.

src/flows/case-csat-followup.flow.ts

@@ -23,6 +23,22 @@ export const CaseCsatFollowupFlow: Flow = {
   description: 'When a case closes, wait a day then prompt the owner to collect a satisfaction rating.',
   type: 'record_change',
   status: 'active',
+  // A record-change flow fired by a SYSTEM write carries no trigger user
+  // either (ADR-0049, #1888, #3760), and here that is the NORMAL path, not an
+  // edge case: the close transition this flow waits on is written by the
+  // `close_case` screen flow, which is itself `runAs: 'system'`, so runs
+  // spawned from the Close Case button are user-less by construction.
+  //
+  // Measured honestly on 17.0.0-rc.2: this flow has NO data node (start →
+  // wait → notify → end), and a user-less run was driven end to end through
+  // the real engine — it suspends at the P1D timer and, on resume, the notify
+  // node delivers. The refusal covers get/create/update/delete only, so
+  // declaring `system` changes nothing observable today. Declared for the same
+  // reason as `contact_welcome` — see the fuller note there — and with extra
+  // force here: this flow resumes long after its trigger, so any data node
+  // added to the post-wait leg would run in whatever identity the suspended
+  // run carried, which for the common path is none.
+  runAs: 'system',
 
   variables: [],
 

src/flows/case-escalation.flow.ts

@@ -11,6 +11,24 @@ export const CaseEscalationFlow: Flow = {
   description: 'Automatically escalate high-priority cases',
   type: 'record_change',
   status: 'active',
+  // A record-change flow fired by a SYSTEM write carries no trigger user
+  // either — the user-less exposure is NOT schedule-only (ADR-0049, #1888,
+  // #3760), and the platform's own build-time lint cannot see this shape
+  // because it is only knowable at run time. Measured on 17.0.0-rc.2: a
+  // user-less run refuses the very first data node — "[runAs] refusing a data
+  // operation (object 'crm_case', event 'record-after-update')" — so a case
+  // raised by the seed loader, an integration, or another runAs:'system'
+  // flow's write silently keeps its critical priority forever.
+  //
+  // Elevating the USER-driven runs too is correct here: escalation is a
+  // service-level policy, not the writer's own edit. Both data nodes are keyed
+  // to `{record.id}` — the row that just fired the trigger — so user scope
+  // buys no restriction that matters, while it does add a failure mode: an
+  // agent may legitimately raise a case without holding edit rights on the
+  // escalation lifecycle fields (`is_escalated` / `escalated_date` / `status`).
+  // This also puts the flow on the same footing as `case_sla_monitor`, which
+  // performs the same escalation on a schedule under `runAs: 'system'`.
+  runAs: 'system',
 
   variables: [
     { name: 'caseId', type: 'text', isInput: true, isOutput: false },

src/flows/contact-welcome.flow.ts

@@ -25,6 +25,25 @@ export const ContactWelcomeFlow: Flow = {
   description: 'On new contact: prompt the owner to welcome them.',
   type: 'record_change',
   status: 'active',
+  // A record-change flow fired by a SYSTEM write carries no trigger user
+  // either (ADR-0049, #1888, #3760) — and this flow exists FOR that
+  // population: the start condition below already documents the seeded /
+  // integration-written contacts whose `owner_id` the security middleware
+  // never stamped because the write carried no session (#548).
+  //
+  // Measured honestly on 17.0.0-rc.2: this flow has NO data node today (start
+  // → notify → end), the runAs refusal covers get/create/update/delete only,
+  // and `notify` dispatches through the messaging service without a run data
+  // context — so a user-less run completes and delivers today, and this
+  // declaration changes nothing observable right now. It is declared anyway
+  // because the decision being recorded is "record-change automation runs as
+  // the platform", not "this flow currently happens to have no data node".
+  // The day a data node is added — or a start-node `config.expand`, which is
+  // the remedy the notify node's own error message recommends and which reads
+  // through the same guarded path — the elevation has already been reasoned
+  // about here rather than discovered as a refusal in production. Elevation is
+  // harmless for the user-driven runs: there is no data operation to scope.
+  runAs: 'system',
   variables: [],
   nodes: [
     {

src/flows/lead-assignment.flow.ts

@@ -31,6 +31,19 @@ export const LeadAssignmentFlow: Flow = {
   description: 'On new lead: set a rating-based follow-up SLA and alert the lead owner.',
   type: 'record_change',
   status: 'active',
+  // A record-change flow fired by a SYSTEM write carries no trigger user
+  // either (ADR-0049, #1888, #3760). This is the population the flow is FOR:
+  // leads arrive from web-to-lead, a CSV import, a partner integration or a
+  // seed load, none of which carry a user session. Measured on 17.0.0-rc.2, a
+  // user-less run refuses `sla_hot` / `sla_std` outright, so the lead lands
+  // with no follow-up SLA and no alert at all — silently, since the refusal
+  // surfaces only in the server log.
+  //
+  // Elevating the USER-driven runs too is correct: the SLA stamp is the
+  // platform's routing decision about the row that just fired the trigger
+  // (keyed to `{record.id}`), not the submitting rep's own edit, and a rep who
+  // may create a lead is not thereby guaranteed edit rights on it.
+  runAs: 'system',
 
   variables: [],
 

src/flows/opportunity-approval.flow.ts

@@ -35,6 +35,24 @@ export const OpportunityApprovalFlow: Flow = {
   description: 'Tiered approval for opportunities: manager review > $100K, director sign-off > $500K.',
   type: 'record_change',
   status: 'active',
+  // Same user-less exposure as every record-change flow (ADR-0049, #1888,
+  // #3760) — and the one with teeth. Measured on 17.0.0-rc.2: a $150K renewal
+  // created by the `runAs: 'system'` contract_renewal sweep fired this flow
+  // with no trigger user, and the run died at `get_opportunity`. The deal sat
+  // at `approval_status: 'not_required'`, unlocked, with no approval request
+  // ever opened — the gate was bypassed by the writer simply not carrying a
+  // session. An approval control that only engages for logged-in writers is
+  // not a control.
+  //
+  // Elevating the USER-driven runs too is not merely acceptable here, it is
+  // required. This gate exists to CONSTRAIN the submitter, so the submitter's
+  // own scope is the wrong identity to evaluate it under; and
+  // `mark_approved` / `mark_rejected` stamp `approval_status` on a record the
+  // approval node holds locked (`lockRecord: true`), which only a platform
+  // write may land. The approval node itself already opens its request under
+  // the platform identity while re-attaching the deciding user for
+  // attribution, so this declaration aligns the flow with the node it drives.
+  runAs: 'system',
 
   variables: [
     { name: 'opportunityId', type: 'text', isInput: true, isOutput: false },

src/flows/opportunity-won-alert.flow.ts

@@ -21,6 +21,18 @@ export const OpportunityWonAlertFlow: Flow = {
   description: 'On closed_won opportunities over $100K: notify owner + manager.',
   type: 'record_change',
   status: 'active',
+  // A record-change flow fired by a SYSTEM write carries no trigger user
+  // either (ADR-0049, #1888, #3760). Deals reach `closed_won` through
+  // machinery as well as through a rep's own save — `lead_conversion` and the
+  // `contract_renewal` sweep both write opportunities, and the latter is
+  // itself `runAs: 'system'`.
+  //
+  // Measured honestly on 17.0.0-rc.2: this flow has NO data node (start →
+  // notify → end), so the refusal does not reach it today and declaring
+  // `system` changes nothing observable. Declared for the same reason as
+  // `contact_welcome` — see the fuller note there. Nothing to scope, so
+  // elevating the user-driven runs costs nothing.
+  runAs: 'system',
   variables: [],
   nodes: [
     {

src/flows/task-urgent-alert.flow.ts

@@ -17,6 +17,21 @@ export const TaskUrgentAlertFlow: Flow = {
   description: 'On new urgent task: notify the owner.',
   type: 'record_change',
   status: 'active',
+  // A record-change flow fired by a SYSTEM write carries no trigger user
+  // either (ADR-0049, #1888, #3760), and most urgent tasks in this app are
+  // machine-written: `case_status_side_effects` opens the escalation follow-up
+  // task, and the scheduled sweeps create their own — none of those writes
+  // carries a session.
+  //
+  // Measured honestly on 17.0.0-rc.2: this flow has NO data node (start →
+  // notify → end), so the refusal does not reach it today and declaring
+  // `system` changes nothing observable. Declared for the same reason as
+  // `contact_welcome` — see the fuller note there: the recorded decision is
+  // that record-change automation runs as the platform, so a data node added
+  // later inherits an elevation that was reasoned about rather than one
+  // discovered as a production refusal. No data operation exists to scope, so
+  // elevating the user-driven runs costs nothing.
+  runAs: 'system',
   variables: [],
   nodes: [
     {

test/actions-flows-integrity.test.ts

@@ -1,5 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { readdirSync, readFileSync } from 'node:fs';
 import { describe, it, expect } from 'vitest';
 import stack from '../objectstack.config';
 
@@ -373,3 +374,95 @@ describe('case escalation trigger does not fight the close action', () => {
     },
   );
 });
+
+/**
+ * Execution identity of the RECORD-CHANGE flows (#684).
+ *
+ * `runAs` decides who a run's data operations execute as. Under the schema
+ * default `'user'` a run that resolved NO trigger user has no identity to
+ * scope to, so the engine REFUSES its data operations rather than letting them
+ * run unscoped — the fail-open ADR-0049 forbids (#1888, #3760).
+ *
+ * The trap this guard exists for is that "no trigger user" is NOT a
+ * schedule-only condition. A record-change flow is fired by a WRITE, and a
+ * write made without a session — seed loading, an integration, a webhook, or
+ * another `runAs:'system'` flow's own write — carries no user into the run it
+ * triggers. Ten scheduled flows had already been given `runAs:'system'`, each
+ * with an authored comment saying so; every record-change flow was left on the
+ * default, and on the 17.0 GA acceptance sweep that cost 12 failed runs on one
+ * boot plus a demonstrated approval bypass (a $150K renewal created by the
+ * contract_renewal sweep never opened its approval request).
+ *
+ * The platform's build-time lint (`flow-runas-unscoped`) rejects the
+ * statically-decidable shapes — schedule, time-relative, api — but explicitly
+ * NOT this one, because whether a record-change trigger carries a user is only
+ * knowable at run time. So the app owns the invariant, and this is it.
+ *
+ * Enumerated from the COMPILED STACK, not from a hand-maintained import list:
+ * a new record-change flow registered in `src/flows/index.ts` is covered the
+ * moment it is registered, which is the only way this guard cannot go stale.
+ */
+describe('record-change flows declare their execution identity (#684)', () => {
+  const recordChangeFlows = flows.filter((f) => f.type === 'record_change');
+
+  it('there are record-change flows to check (the filter still matches)', () => {
+    // Cheap canary: if `type` were ever renamed or the flows re-typed, every
+    // assertion below would pass vacuously over an empty list.
+    expect(recordChangeFlows.length).toBeGreaterThanOrEqual(9);
+  });
+
+  it.each(recordChangeFlows.map((f) => f.name))(
+    '%s declares runAs:"system"',
+    (name) => {
+      const f = flow(name)!;
+      expect(
+        f.runAs,
+        `flow '${name}' is record-change but leaves runAs at the default 'user'. ` +
+          'A write that carries no session (seed, integration, webhook, or another ' +
+          "runAs:'system' flow) fires it with no trigger user, and its data operations " +
+          'are then REFUSED at run time — silently, in the server log. Declare ' +
+          "runAs:'system' to make the elevation explicit, or, if this flow genuinely " +
+          "must act as the triggering user, exempt it here with the reasoning written down.",
+      ).toBe('system');
+    },
+  );
+
+  it('the insert-time twins inherit the parent’s identity rather than restating it', () => {
+    // Both twins are built by spreading their parent, so `runAs` rides along.
+    // Pinning it keeps a future refactor from turning the spread into an
+    // explicit field list that quietly drops it.
+    for (const [twin, parent] of [
+      ['case_escalation_on_create', 'case_escalation'],
+      ['opportunity_approval_on_create', 'opportunity_approval'],
+    ] as const) {
+      expect(flow(twin)!.runAs, `${twin} drifted from ${parent}`).toBe(flow(parent)!.runAs);
+    }
+  });
+
+  it('every record-change flow explains its elevation in the source', () => {
+    // The ten scheduled flows each carry an authored rationale beside their
+    // `runAs`. An undocumented `runAs:'system'` is indistinguishable from a
+    // copy-paste, and elevation is exactly the decision that must not be made
+    // by copy-paste: it applies to the USER-driven runs too, which today
+    // execute scoped to the triggering user.
+    const missing: string[] = [];
+    for (const file of readdirSync(new URL('../src/flows/', import.meta.url))) {
+      if (!file.endsWith('.flow.ts')) continue;
+      const lines = readFileSync(new URL(`../src/flows/${file}`, import.meta.url), 'utf8').split('\n');
+      if (!lines.some((l) => /^\s*type:\s*'record_change'\s*,?\s*$/.test(l))) continue;
+      // The DECLARATION line, not a mention of `runAs: 'system'` in prose —
+      // these rationales cite each other and every scheduled precedent, so a
+      // substring search lands inside a comment and reports the file as
+      // undocumented (it did, on four of the seven, first run).
+      const at = lines.findIndex((l) => /^\s*runAs:\s*'system'\s*,\s*$/.test(l));
+      if (at < 0) { missing.push(`${file}: no runAs: 'system' declaration`); continue; }
+      // Walk back over the contiguous comment block directly above it.
+      let comment = 0;
+      for (let i = at - 1; i >= 0 && /^\s*(\/\/|\*|\/\*)/.test(lines[i]); i--) comment++;
+      if (comment < 3) {
+        missing.push(`${file}: runAs: 'system' carries ${comment} line(s) of rationale above it, expected >= 3`);
+      }
+    }
+    expect(missing, missing.join('\n')).toEqual([]);
+  });
+});