fix(lint,cli): os lint 不再放行另外两个命令拒绝的 flow (#4345 后续) (#4394)

os-zhuang · claude · web-flow · commit 7fec5d63e611 · 2026-07-31T18:58:41.000+08:00
`validateReadonlyFlowWrites` 手工接进了 `os validate` 和 `os compile`,唯独 漏了 `os lint`。在 showcase 上植入一处违规(`runAs:'user'` 的 update_record 写静态 readonly 字段)实测: os lint os validate 修复前 exit 0 放行 exit 1 拒绝 修复后 exit 1 拒绝 exit 1 拒绝 这条规则是 **gate** 级(静态 readonly + 字面字段是确定的 no-op:引擎从 UPDATE payload 里剥掉它,步骤照样报成功,#2948/#3425),所以漏接不是少一条 警告 —— 而是 `os lint` 放行了一个 `os validate` 会拦下的构建。 现接入 `REFERENCE_INTEGRITY_RULES`,两处手工接线随之删除,三个命令的答案 由构造保证一致,而不是靠三个人记得。这正是 suite 设立要终结的漂移 (#3583 §5 D5),而 suite 自己的头注释一直把这条规则当作该漂移的现行证据。 两件事让这个接线从"不整洁"变成"站不住": - `validateFlowNodeWrites`(#4369)走的是**同一张** `config.fields` map,问的 是同一问题的另一半("字段存在吗" vs "字段可写吗"),而它已经在 suite 里。 一张 map、两个检查、两套命令集合。 - 两处手工接线连喂进去的输入都不一致:`validate` 喂 pre-parse 的 `normalized`,`compile` 喂 post-parse 的 `result.data`。合并到 suite 的 post-parse 输入前已实测两者对本规则等价,不丢 finding。 规则行为零变化:id、严重级、文案都不动。 Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
diff --git a/.changeset/readonly-flow-writes-joins-the-suite.md b/.changeset/readonly-flow-writes-joins-the-suite.md
@@ -0,0 +1,38 @@
+---
+"@objectstack/lint": patch
+"@objectstack/cli": patch
+---
+
+fix(lint,cli): `os lint` no longer passes a flow the other two commands refuse
+
+`validateReadonlyFlowWrites` was hand-wired into `os validate` and `os compile`
+and never into `os lint`. Measured on the showcase app with one planted
+violation — a `runAs:'user'` `update_record` writing a static-`readonly` field:
+
+| | `os lint` | `os validate` |
+|---|---|---|
+| before | **exit 0 — passed** | exit 1 — refused |
+| after | exit 1 — refused | exit 1 — refused |
+
+That rule **gates** (a static `readonly` + literal field is a certain no-op:
+the engine strips it from the UPDATE payload while the step still reports
+success, #2948/#3425), so the divergence was not a missing warning — `os lint`
+green-lit a build `os validate` stops.
+
+It now joins `REFERENCE_INTEGRITY_RULES`, and both hand-wired call sites are
+deleted with it, so the three commands share one answer by construction rather
+than by three people remembering. This is the drift the suite was created to end
+(#3583 §5 D5) and which its own header cited this rule as the standing proof of.
+
+Two things made the wiring indefensible rather than merely untidy:
+
+- `validateFlowNodeWrites` (#4369) walks the **same** `config.fields` map to ask
+  the other half of the question — "does this field exist?" against "is it
+  writable?" — and is already a suite member. One map, two checks, two different
+  command sets.
+- The two hand-wired sites did not even agree with each other on their input:
+  `validate` passed the PRE-parse `normalized` stack, `compile` the POST-parse
+  `result.data`. Verified equivalent for this rule before collapsing them onto
+  the suite's post-parse input, so no finding is lost.
+
+No rule behaviour changes: same ids, same severities, same messages.
diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts
@@ -23,7 +23,6 @@ import { validateFilterTokens } from '@objectstack/lint';
 import { validateReferenceIntegrity } from '@objectstack/lint';
 import { validateResponsiveStyles } from '@objectstack/lint';
 import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
-import { validateReadonlyFlowWrites } from '@objectstack/lint';
 import { lintFlowPatterns } from '../utils/lint-flow-patterns.js';
 import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js';
 import { lintUniqueDeclarations } from '../lint/data-model-rules.js';
@@ -618,39 +617,6 @@ export default class Compile extends Command {
         }
       }
 
-      // 3e2. [#3425] Readonly flow-write guardrail. A `runAs:user` update_record
-      //      writing a static-`readonly` field is a silent no-op — the engine
-      //      strips it from the UPDATE payload (#2948) while the step reports
-      //      success. This GATES the build (shift-left of the #3407/#3413
-      //      run-time strip warning); `readonlyWhen` writes are per-record-state,
-      //      so they are advisory, printed dimmed and never fatal.
-      if (!flags.json) printStep('Checking readonly flow writes (#3425)...');
-      const readonlyWriteFindings = validateReadonlyFlowWrites(result.data as Record<string, unknown>);
-      const readonlyWriteErrors = readonlyWriteFindings.filter((f) => f.severity === 'error');
-      const readonlyWriteAdvisories = readonlyWriteFindings.filter((f) => f.severity !== 'error');
-      if (readonlyWriteErrors.length > 0) {
-        if (flags.json) {
-          await emitJson({ success: false, error: 'readonly flow-write validation failed', issues: readonlyWriteErrors }, 0, { compact: true });
-          this.exit(1);
-        }
-        console.log('');
-        printError(`Readonly flow-write check failed (${readonlyWriteErrors.length} issue${readonlyWriteErrors.length > 1 ? 's' : ''})`);
-        for (const f of readonlyWriteErrors.slice(0, 50)) {
-          console.log(`  • ${f.where}: ${f.message}`);
-          console.log(chalk.dim(`      ${f.hint}`));
-          console.log(chalk.dim(`      rule: ${f.rule}  at ${f.path}`));
-        }
-        this.exit(1);
-      }
-      if (readonlyWriteAdvisories.length > 0 && !flags.json) {
-        console.log('');
-        for (const f of readonlyWriteAdvisories) {
-          printWarning(`${f.where}: ${f.message}`);
-          console.log(chalk.dim(`    ${f.hint}`));
-          console.log(chalk.dim(`    rule: ${f.rule}`));
-        }
-      }
-
       // 3f. [ADR-0090 D6] Access-matrix snapshot gate. Opt-in per app: when
       //     `access-matrix.json` sits next to the config, the (permission set
       //     × object) capability matrix derived from THIS build must match it
diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts
@@ -28,7 +28,6 @@ import { validateCapabilityReferences } from '@objectstack/lint';
 import { validateVisibilityPredicates } from '@objectstack/lint';
 import { validateSecurityPosture, validateOrgAxisRedLines } from '@objectstack/lint';
 import { validateFlowTriggerReadiness } from '@objectstack/lint';
-import { validateReadonlyFlowWrites } from '@objectstack/lint';
 import { lintFlowPatterns } from '../utils/lint-flow-patterns.js';
 import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js';
 import { lintUniqueDeclarations } from '../lint/data-model-rules.js';
@@ -540,41 +539,6 @@ export default class Validate extends Command {
       //     flow whose filter token the runtime refuses (#3810) for as long as
       //     this call site was the only one.
 
-      // 3e2. [#3425] Readonly flow-write guardrail. A `runAs:user` update_record
-      //      that writes a static-`readonly` field is a SILENT no-op — the engine
-      //      strips it from the UPDATE payload (#2948) yet the step reports
-      //      success. This is the shift-left of the run-time strip warning
-      //      (#3407/#3413): a static readonly + literal field is a certain no-op
-      //      → error (gates); a `readonlyWhen` field is per-record-state → advisory.
-      if (!flags.json) printStep('Checking readonly flow writes (#3425)...');
-      const readonlyWriteFindings = validateReadonlyFlowWrites(normalized as Record<string, unknown>);
-      const readonlyWriteErrors = readonlyWriteFindings.filter((f) => f.severity === 'error');
-      const readonlyWriteWarnings = readonlyWriteFindings.filter((f) => f.severity === 'warning');
-      if (readonlyWriteErrors.length > 0) {
-        if (flags.json) {
-          await emitJson({
-            valid: false,
-            errors: readonlyWriteErrors,
-            duration: timer.elapsed(),
-          });
-          this.exit(1);
-        }
-        console.log('');
-        printError(`Readonly flow-write check failed (${readonlyWriteErrors.length} issue${readonlyWriteErrors.length > 1 ? 's' : ''})`);
-        for (const f of readonlyWriteErrors.slice(0, 50)) {
-          console.log(`  • ${f.where}: ${f.message}`);
-          console.log(chalk.dim(`      ${f.hint}`));
-          console.log(chalk.dim(`      rule: ${f.rule}  at ${f.path}`));
-        }
-        this.exit(1);
-      }
-      if (!flags.json) {
-        for (const f of readonlyWriteWarnings.slice(0, 50)) {
-          console.log(chalk.yellow(`  ⚠ ${f.where}: ${f.message}`));
-          console.log(chalk.dim(`      ${f.hint}`));
-        }
-      }
-
       // 3e3. [#3782] The four authoring lints that live in the CLI itself rather
       //      than in `@objectstack/lint`. Every other gate on this command is a
       //      `@objectstack/lint` import, so these four were only ever reachable
@@ -749,7 +713,7 @@ export default class Validate extends Command {
           // the suite's warnings, though the failure path (above) and the console
           // both did. Same shape of bug as the dropped errors — computed, then
           // discarded — so it is fixed rather than reproduced under a new name.
-          warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...refWarnings, ...readonlyWriteWarnings, ...authoringLintWarnings, ...unknownKeyWarnings, ...securityAdvisories, ...capProviderWarnings],
+          warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...refWarnings, ...authoringLintWarnings, ...unknownKeyWarnings, ...securityAdvisories, ...capProviderWarnings],
           conversions: conversionNotices,
           specVersionGap: specGap,
           duration: timer.elapsed(),
diff --git a/packages/lint/src/reference-integrity-suite.test.ts b/packages/lint/src/reference-integrity-suite.test.ts
@@ -29,6 +29,7 @@ describe('reference-integrity suite — membership', () => {
       'validateHookBodyWrites',
       'validateActionBodyWrites',
       'validateFlowNodeWrites',
+      'validateReadonlyFlowWrites',
     ]);
   });
 
@@ -51,7 +52,10 @@ describe('reference-integrity suite — every member actually runs', () => {
     objects: [
       {
         name: 'crm_lead',
-        fields: { name: { type: 'text', label: 'Name' } },
+        fields: {
+          name: { type: 'text', label: 'Name' },
+          locked: { type: 'boolean', label: 'Locked', readonly: true },
+        },
         // validateSearchableFields: `budget` is not a field on crm_lead, so the
         // ADR-0061 declaration is stale — the engine drops it and searches a
         // narrower set than the object declares.
@@ -157,6 +161,12 @@ describe('reference-integrity suite — every member actually runs', () => {
       {
         name: 'lead_followup',
         type: 'record_change',
+        // validateReadonlyFlowWrites: a runAs:'user' update_record writing a
+        // static-`readonly` field. The engine strips it and the step still
+        // reports success (#2948/#3425) — a certainty, so it GATES. Walks the
+        // same `config.fields` map validateFlowNodeWrites does, which is why
+        // the two must not diverge across commands again.
+        runAs: 'user',
         nodes: [
           { id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } },
           // validateFlowTemplatePaths: `budget` is not a field on crm_lead. In a
@@ -178,6 +188,17 @@ describe('reference-integrity suite — every member actually runs', () => {
             type: 'update_record',
             config: { objectName: 'crm_lead', fields: { lead_score: 100 } },
           },
+          // validateReadonlyFlowWrites: the OTHER question about that same
+          // `config.fields` map — `locked` exists but is static-`readonly`, so
+          // under this flow's `runAs:'user'` the engine strips it and the step
+          // still reports success (#2948/#3425). A separate node from `stamp`
+          // on purpose: one node carrying both defects would let either rule
+          // go silent behind the other's finding.
+          {
+            id: 'lock',
+            type: 'update_record',
+            config: { objectName: 'crm_lead', fields: { locked: true } },
+          },
         ],
       },
     ],
@@ -204,6 +225,7 @@ describe('reference-integrity suite — every member actually runs', () => {
     // why it rides along instead of becoming its own entry.
     expect(rules).toContain('action-record-write-discarded');
     expect(rules).toContain('flow-node-write-unknown-field');
+    expect(rules).toContain('flow-update-readonly-field');
   });
 
   it('carries a gating flow-template finding through the suite (#3810)', () => {
@@ -225,9 +247,9 @@ describe('reference-integrity suite — every member actually runs', () => {
       expect(typeof f.message).toBe('string');
       expect(typeof f.hint).toBe('string');
     }
-    // Object references run first, flow-node writes last.
+    // Object references run first, readonly flow writes last.
     expect(findings[0].rule).toBe('object-reference-unknown');
-    expect(findings[findings.length - 1].rule).toBe('flow-node-write-unknown-field');
+    expect(findings[findings.length - 1].rule).toBe('flow-update-readonly-field');
   });
 
   it('returns nothing for an empty stack', () => {
diff --git a/packages/lint/src/reference-integrity-suite.ts b/packages/lint/src/reference-integrity-suite.ts
@@ -68,6 +68,7 @@ import { validateAiAgentAuthoring } from './validate-ai-agent-authoring.js';
 import { validateHookBodyWrites } from './validate-hook-body-writes.js';
 import { validateActionBodyWrites } from './validate-action-body-writes.js';
 import { validateFlowNodeWrites } from './validate-flow-node-writes.js';
+import { validateReadonlyFlowWrites } from './validate-readonly-flow-writes.js';
 
 export type ReferenceIntegritySeverity = 'error' | 'warning';
 
@@ -133,8 +134,8 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
   // anyway because it falls out of the SAME parse of the SAME source: a
   // separate member would parse every action body twice to say two things
   // about one walk, and hand-wiring it into the CLI instead is exactly the
-  // drift this suite exists to end (`validateReadonlyFlowWrites` is the
-  // standing proof — wired into `validate` and `compile`, never into `lint`).
+  // drift this suite exists to end — which `validateReadonlyFlowWrites` was
+  // the standing proof of, until it joined the suite below.
   { name: 'validateActionBodyWrites', run: validateActionBodyWrites },
   // The third surface that writes a record field set: a flow `update_record`
   // node's `config.fields`. Same question as the two rules above, but the map
@@ -143,6 +144,16 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
   // standing "prefer a flow node, it's checked" advice was the least true of
   // the three until it landed.
   { name: 'validateFlowNodeWrites', run: validateFlowNodeWrites },
+  // The OTHER question about that same `config.fields` map: not "does this
+  // field exist?" but "is it writable?" — a `runAs:'user'` update_record
+  // writing a static-`readonly` field is stripped by the engine and the step
+  // still reports success (#2948/#3425). It walks the identical map the rule
+  // above walks, so the two splitting call sites was never defensible: hand-
+  // wired into `validate` and `compile` only, it left `os lint` PASSING a flow
+  // `os validate` refuses — and this one gates, so the divergence shipped a
+  // build the other command would have stopped. Joining the suite is the whole
+  // fix; the two hand-wired call sites are deleted with it (#4345 follow-up).
+  { name: 'validateReadonlyFlowWrites', run: validateReadonlyFlowWrites },
 ];
 
 /**
