Skip to content

Commit 2ed6be6

Browse files
fix(engine): aggregate boot-path advisory hits into one summary line, and count them by row (#15306)
* wip(#13889): advisory aggregation scope + by-row advisory gating Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * feat(#13889): aggregate boot-path advisory hits; count by row, not by write Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * docs(#13889): changeset for advisory boot-path aggregation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * fix(#13889): honour the caller's bound in the new test double; re-anchor census lines Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * docs(#13889): correct advisory-severity prose the change falsifies Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d0ee598 commit 2ed6be6

9 files changed

Lines changed: 760 additions & 7 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/core": minor
3+
"@objectstack/objectql": minor
4+
"@objectstack/metadata-protocol": minor
5+
---
6+
7+
Advisory validation rules no longer flood the startup log, and no longer count a row twice on a clean first boot.
8+
9+
A `severity: 'warning'` (or `'info'`) validation rule is advisory: it never blocks a write, and its message is written for a person filling in a form. Evaluated across a seed load it produced one `WARN` line per row, so a clean-database first boot opened with a wall of form hints re-cast as boot diagnostics — and an app could reach "zero warnings" only by bending its data or deleting the rule.
10+
11+
Two changes, and neither moves what a rule evaluates to:
12+
13+
- **Aggregated reporting on the seed/boot path.** `SeedLoaderService.load()` now runs inside an advisory aggregation scope, and reports one summary line per rule — the rule, the object, the row count, the rule's own message and example rows — instead of one line per row. Off that path (an ordinary interactive write) nothing changes: the same per-write line is emitted verbatim. The new scope is `runWithAdvisoryAggregation` / `recordAdvisoryHit` in `@objectstack/core`.
14+
- **Advisory rules are counted by row, not by write.** An `update` whose payload touches only platform-injected system columns — the shape `claimSeedOwnership` writes when it hands seeded rows to the first admin, `{ owner_id }` — changes no business field, so it no longer re-evaluates the object's advisory rules. Previously a seeded row rang once on insert and again when the claim scan rewrote `owner_id`, so anyone counting startup warnings over-estimated by the number of claimed objects.
15+
16+
`error`-severity rules are untouched by both changes: an invariant is still enforced on every write, whoever issued it and however little it moved. Membership of the "system column" set is resolved per object by `resolveInjectedSystemColumns`, so an object that declares `ownership: 'org'` (no `owner_id`) or `systemFields: false` is judged on its own columns rather than a fixed list.

content/docs/data-modeling/validation.mdx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,11 @@ All validation types share these base properties:
8383
| Severity | Behavior |
8484
| :--- | :--- |
8585
| `error` | Prevents the record from being saved |
86-
| `warning` | Shows a warning but allows save |
86+
| `warning` | Allows the save; advisory only — logged server-side, not returned to the caller |
8787
| `info` | Informational message, no blocking |
8888

89+
Advisory rules (`warning` / `info`) are reported, never enforced: on an ordinary write each hit is logged as it happens; on a seed/boot load a run's hits are folded into **one summary line per rule**; and an `update` touching only platform-injected system columns does not re-evaluate them at all, so a row is reported once rather than once per write (#13889).
90+
8991
## Validation Types
9092

9193
### Script Validation
@@ -112,7 +114,8 @@ fix it. Until protocol 17 such a rule was logged at WARN and *skipped*, so the w
112114
went through while the rule stayed declared and enforced nothing; a validation exists
113115
to reject a write, and "the rule could not be checked" must never resolve to
114116
"allowed" (#4649). `severity` still governs blocking — an unevaluable `warning` /
115-
`info` rule is logged and does not throw.
117+
`info` rule is logged and does not throw, on the writes where advisory rules are
118+
evaluated at all (see above).
116119

117120
The record a predicate reads is the stored row overlaid with this write's payload,
118121
**total over the object's declared fields** (`null` for a declared field present in

content/docs/permissions/system-context.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ assuming `isSystem` covers it is a documented source of bugs.
193193

194194
| Assumption | Reality | Anchor |
195195
|:---|:---|:---|
196-
| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881``1883`, #3760), `flow.zod.ts:702` |
196+
| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:2032` (rationale at `:1942``1944`, #3760), `flow.zod.ts:702` |
197197
| "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) |
198198
| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10166``10183` |
199199
| "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1580` (#3493 / #6640) |

content/docs/protocol/objectql/state-machine.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ transitions: {
108108
- **On insert**, the `transitions` table is not consulted — there is no prior state to transition from. If the rule declares `initialStates`, the created value must be one of them, or the write is rejected with `invalid_initial_state` (the FSM entry point). Without `initialStates`, insert is a no-op and the starting value is constrained only by the field-level `select` option-membership check (`invalid_option`), so any declared option is a legal start.
109109
- **On update**, if the state field changed and the new value is **not** in `transitions[oldValue]`, the write is rejected. Clearing the field (writing `null`) is exempt.
110110
- The check is **lenient where it cannot reason**: if the prior state has no key in `transitions` (legacy or externally-written data, or a state you simply forgot to declare), it does not block. Only an explicit `[]` makes a state a hard dead-end.
111-
- Only a rule with `severity: 'error'` (the default) blocks the write; `warning`/`info` are logged.
111+
- Only a rule with `severity: 'error'` (the default) blocks the write. `warning`/`info` are advisory: logged per write on an ordinary write, folded into **one summary line per rule** on a seed/boot load, and not evaluated at all on an `update` whose payload touches only platform-injected system columns (#13889).
112112
- **Seed writes are exempt** (#3433). Curated seed data — package bootstrap fixtures, marketplace templates, per-org replay, all loaded by `SeedLoaderService` — is a snapshot of established facts, not a record walking its lifecycle, so it bypasses the `state_machine` rule entirely: a seed may be born mid-lifecycle (a `completed` project, a `closed_won` opportunity) and neither `initialStates` (insert) nor `transitions` (update) is enforced. Every *other* validation still runs, so a seed must still satisfy field shape, `format`, `script`, and the rest. `os lint` warns when a seeded value is not a state the machine declares, so a typo is still caught before boot.
113113
- **A "historical" data import is exempt too** (#3479). Migrating established facts — a batch of already-`closed` tickets, `closed_won` deals — is the same "snapshot, not a lifecycle event" situation. Set `treatAsHistorical: true` on the import request (default **off**) and the runner puts `skipStateMachine` on the write context, so `initialStates` doesn't reject those mid-lifecycle rows. A normal import leaves it off and still walks the FSM — the strict behavior is the default, so the exemption is always an explicit opt-in.
114114
- **`treatAsHistorical` also preserves the original audit timeline** (#3493) — **on the rows an import UPDATES** (#6640). Skipping the FSM is only half of migrating established facts; the other half is keeping *when* they happened and *who* did them. Under the same flag the write context also carries `preserveAudit`, which (1) makes `updated_at` / `updated_by` **client-preferred** — a supplied historical last-modified survives instead of being stamped with the import instant — and (2) admits a **whitelist** through the static-`readonly` write strip: the audit/timestamp family plus author-declared business `readonly` fields (`closed_at`, `resolved_by`, …) — but never the record's own primary key (`id`), which is the address of the write rather than a fact being restored (#8215). Platform-managed `system` columns outside that family (`organization_id` and other tenancy/generated columns) stay stripped — a historical import reinstates facts, it does not forge tenancy. Like the FSM exemption this is opt-in: a normal write still auto-stamps `updated_at`/`updated_by` and strips `readonly` exactly as before, and permissions / RLS / field-level security are unchanged.

packages/core/src/index.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,15 @@ export * from './utils/internal-write-response.js';
6565
// that `@objectstack/metadata-protocol`'s atomic `batchData` also uses.
6666
export * from './utils/migration-journal.js';
6767

68+
// Export the advisory-hit aggregation scope (#13889) — the seam between the
69+
// evaluator that PRODUCES an advisory hit (`@objectstack/objectql`) and the
70+
// machine load path that REPORTS a run's worth of them as one summary
71+
// (`@objectstack/metadata-protocol`'s seed loader). It lives here, on the floor
72+
// both of those already stand on, because neither can import the other's
73+
// package for it: objectql depends on metadata-protocol, so the dependency only
74+
// runs one way.
75+
export * from './utils/advisory-aggregation.js';
76+
6877
// Export the runtime filter-placeholder resolver (framework#3582)
6978
export * from './utils/filter-tokens.js';
7079

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Advisory-hit aggregation for machine load paths (#13889).
5+
*
6+
* ## The defect this closes
7+
*
8+
* A `severity: 'warning'` (or `'info'`) validation rule is ADVISORY: it never
9+
* blocks a write, and its message is written for a HUMAN IN A FORM ("At least
10+
* one related record should be selected"). The evaluator's only report channel
11+
* for one is `logger.warn`, one line per (row × violated rule).
12+
*
13+
* On an interactive write that is exactly right. On the seed / bootstrap load
14+
* path it is not: a clean-database first boot writes every seeded row through
15+
* the same evaluator, so an app whose seed legitimately contains N rows that
16+
* trip one advisory rule gets N `WARN` lines in the STARTUP LOG — a form hint
17+
* re-cast as a boot diagnostic, which reads like the boot failed. Measured
18+
* downstream (hotcrm#1203): the only ways an app could reach "zero warnings"
19+
* were to bend its data (attach a parent record it does not have) or delete the
20+
* rule (weaken a real guard). Both are worse than the noise.
21+
*
22+
* Maintainer ruling (2026-09-01, verbatim 「同意」 on option B):
23+
* 「⛔ 不改规则语义,只改日志形状」 — do not change rule semantics, change
24+
* only the shape of the log. So this module changes NOTHING about what a rule
25+
* evaluates to, who it applies to, or whether it blocks. It changes where the
26+
* REPORT goes while a machine load path is running, and nothing else.
27+
*
28+
* ## Why an ambient scope rather than a parameter
29+
*
30+
* The producer (`evaluateValidationRules`, `@objectstack/objectql`) and the
31+
* scope owner (`SeedLoaderService.load`, `@objectstack/metadata-protocol`) are
32+
* separated by the whole engine write path: the loader calls
33+
* `IDataEngine.insert/update`, and the evaluator is reached several layers
34+
* below through a contract that carries no reporting channel. Threading a sink
35+
* through would mean widening `IDataEngine`'s options — a published contract —
36+
* for a diagnostic concern. An ambient scope keeps the change inside the two
37+
* ends that care.
38+
*
39+
* `AsyncLocalStorage` rather than a module-level flag, deliberately: seed loads
40+
* are NOT boot-only. A per-org replay (`sys_organization` insert) runs a full
41+
* seed load on a LIVE server, concurrently with ordinary interactive traffic. A
42+
* plain global would capture those interactive writes' advisories into the
43+
* replay's summary — silently swallowing a report meant for someone else. ALS
44+
* scopes the capture to the load's own async context, which is the difference
45+
* between aggregating and losing.
46+
*
47+
* ## Folded on arrival, never accumulated per row
48+
*
49+
* A hit is folded into its `(object, rule)` group as it arrives, so a load of
50+
* 100 000 rows that all trip one rule holds ONE group, not 100 000 records.
51+
* The group keeps what a reader needs to act — the rule, the object, the
52+
* severity, the message, the row count, and up to {@link ADVISORY_SAMPLE_ROWS}
53+
* example rows — which is the 「详见…」 half of the ruling's summary shape.
54+
*/
55+
56+
import { AsyncLocalStorage } from 'node:async_hooks';
57+
58+
/** Example row references a group carries, so the summary can point at real rows. */
59+
export const ADVISORY_SAMPLE_ROWS = 5;
60+
61+
/** One advisory rule hit, as the evaluator reports it. */
62+
export interface AdvisoryHit {
63+
/** Object the row belongs to. */
64+
object: string;
65+
/** The declared rule's `name`. */
66+
rule: string;
67+
/** The rule's declared severity — `'warning'` or `'info'`; never `'error'`. */
68+
severity: string;
69+
/** The rule's author-written message, in the caller's locale. */
70+
message: string;
71+
/**
72+
* A reference to the row, when the write carries one.
73+
*
74+
* NOT necessarily an id: on the path this exists for — a seed INSERT — the
75+
* driver has not issued an id yet at validation time, so an id-only reference
76+
* would be empty for exactly the case the aggregation was built for. The
77+
* producer sends the best stable handle it has (`id`, else `name=<value>`,
78+
* the same way the seed loader names a row in its own errors).
79+
*/
80+
recordRef?: string;
81+
}
82+
83+
/** Every hit for one `(object, rule)` pair, folded. */
84+
export interface AdvisoryGroup {
85+
object: string;
86+
rule: string;
87+
severity: string;
88+
/** The first message seen for this group (they differ only by interpolation). */
89+
message: string;
90+
/** How many ROWS tripped this rule during the scope. */
91+
rows: number;
92+
/** Up to {@link ADVISORY_SAMPLE_ROWS} example row references. */
93+
sampleRows: string[];
94+
}
95+
96+
interface AdvisoryCollector {
97+
groups: Map<string, AdvisoryGroup>;
98+
}
99+
100+
const storage = new AsyncLocalStorage<AdvisoryCollector>();
101+
102+
/**
103+
* Group key.
104+
*
105+
* `JSON.stringify` of the pair rather than a delimiter-joined string: any
106+
* single-character delimiter is a claim about what an object or rule name
107+
* cannot contain, and a wrong claim collides two groups into one silently.
108+
* Encoding the pair makes the key injective with nothing to be wrong about.
109+
*/
110+
function keyOf(object: string, rule: string): string {
111+
return JSON.stringify([object, rule]);
112+
}
113+
114+
/**
115+
* Offer one advisory hit to the active aggregation scope.
116+
*
117+
* @returns `true` when a scope captured it — the caller must then NOT log its
118+
* own per-row line, because the scope owner reports the whole group. `false`
119+
* when no scope is active, which is the ordinary interactive case: the caller
120+
* logs exactly as it always did. A caller that ignores the return value
121+
* degrades to today's behaviour rather than losing the report.
122+
*/
123+
export function recordAdvisoryHit(hit: AdvisoryHit): boolean {
124+
const collector = storage.getStore();
125+
if (!collector) return false;
126+
const key = keyOf(hit.object, hit.rule);
127+
const existing = collector.groups.get(key);
128+
if (existing) {
129+
existing.rows += 1;
130+
if (hit.recordRef != null && existing.sampleRows.length < ADVISORY_SAMPLE_ROWS) {
131+
existing.sampleRows.push(hit.recordRef);
132+
}
133+
return true;
134+
}
135+
collector.groups.set(key, {
136+
object: hit.object,
137+
rule: hit.rule,
138+
severity: hit.severity,
139+
message: hit.message,
140+
rows: 1,
141+
sampleRows: hit.recordRef != null ? [hit.recordRef] : [],
142+
});
143+
return true;
144+
}
145+
146+
/**
147+
* Whether an advisory aggregation scope is active on this async context.
148+
*
149+
* Exported for tests and for a caller that wants to skip building a message it
150+
* is about to discard; {@link recordAdvisoryHit}'s return value is the one that
151+
* decides.
152+
*/
153+
export function isAggregatingAdvisories(): boolean {
154+
return storage.getStore() !== undefined;
155+
}
156+
157+
/**
158+
* Run `fn` with advisory hits aggregated, then hand the folded groups to
159+
* `report`.
160+
*
161+
* `report` runs in a `finally`, so a load that throws still reports what it
162+
* tripped before failing — the diagnostics of a half-finished seed are the ones
163+
* most worth having. It is called only when there is something to report, and
164+
* its own failure is never allowed to replace the caller's outcome: a reporting
165+
* bug must not turn a successful seed load into a failed one.
166+
*/
167+
export async function runWithAdvisoryAggregation<T>(
168+
fn: () => Promise<T>,
169+
report: (groups: AdvisoryGroup[]) => void,
170+
): Promise<T> {
171+
const collector: AdvisoryCollector = { groups: new Map() };
172+
try {
173+
return await storage.run(collector, fn);
174+
} finally {
175+
if (collector.groups.size > 0) {
176+
try {
177+
report([...collector.groups.values()]);
178+
} catch {
179+
// Reporting is a diagnostic, never an outcome.
180+
}
181+
}
182+
}
183+
}

0 commit comments

Comments
 (0)