Skip to content

Commit 69c2281

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-4728-database-loader-ddl-loud
2 parents 8eee631 + 459f925 commit 69c2281

29 files changed

Lines changed: 1634 additions & 105 deletions
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
fix(plugin-auth): 每号码 OTP 发送预算改用惰性解析的共享计数存储 —— 多节点下不再按节点数倍增 (#4790)
6+
7+
#2780 的「每号码 OTP 发送预算」(60s 冷却 + 每小时 5 条)此前**只有宿主显式提供
8+
better-auth `secondaryStorage` 时才跨节点共享**`AuthManager.getOtpSendGuard()` 唯一的
9+
存储来源就是 `AuthManagerOptions.secondaryStorage`,而标准 `serve` 组合里没有任何一处
10+
提供它(#4788 之后 `AuthPlugin` 也明确不再从 cache 服务派生它)。于是预算落在**每个进程
11+
一份**:N 个节点的部署,一个号码实际能收到的是声明值的 N 倍,而且**没有任何信号**告诉你
12+
它没兑现(ADR-0049 声明 ≠ 强制)。这里的计价单位是**真金白银的短信**
13+
14+
这是 #4772 那条限流洞的同类,但是独立的一处:#4788 修的是 better-auth 自己的 `rateLimit`
15+
计数器(走 `rateLimit.customStorage`),OTP 预算是 ObjectStack 在 `AuthManager` 里自己实现
16+
的另一套计数,行为未被 #4788 改变。
17+
18+
**修法:复用 #4788 建好的那条路径,而不是再写一份。** `rate-limit-storage.ts` 中把「惰性
19+
解析 → 绑定即宣告 → 解析不到就降级到有界的进程内存储并响亮告警」抽成
20+
`createLazyCounterStore()``createLazyCacheRateLimitStorage()` 现在就是它的一层薄封装),
21+
OTP 预算经由新的 `AuthManagerOptions.sharedCounterStore` 接同一条路径:
22+
23+
- **存储在每次发送校验时才解析**,因此 `CacheServicePlugin` 晚于 `AuthPlugin` 注册也照样
24+
绑定得上(插件启动顺序不再决定任何事)—— 这正是 #4772 冻结结论造成的那个洞;
25+
- 配了 cache 的多节点部署,每号码预算**现在真的是一份**,换节点不会重新获得冷却额度;
26+
- 没有 cache 服务的部署**仍然限额**,只是降级为进程内计数,并在第一次真正计数时打一条
27+
点名代价的 warn(「an N-node deployment can send up to N× the configured number of PAID
28+
SMS to one number」)—— 降级不是关闭,两种情况在日志里可区分(绑定打 info,降级打 warn)。
29+
30+
**刻意不引入 `secondaryStorage` 来修它**#4785):那会把会话的记录之处搬进缓存,静默废掉
31+
ADR-0069 D4 的三个会话管控。宿主自己提供的 `secondaryStorage` 对这个预算仍然优先且行为不变。
32+
33+
冷却与滚动小时窗的语义**未做任何改动**:计数依旧是按号码的时间戳滚动窗口,只是换了它所在的
34+
存储。(固定窗口计数器无法表达「距上一次发送满 N 秒」,把它改成定窗会在窗口边界放行两倍突发
35+
——用一种倍增换另一种倍增。)
36+
37+
对使用者的影响:
38+
39+
- 新增 `AuthManagerOptions.sharedCounterStore``AuthPlugin` 自动填充,一般宿主无需感知;
40+
- 新增导出 `createLazyCounterStore()``counterStoreFromKv()`
41+
- `OtpSendGuard` 新增 `resolveStore` 选项,原有的 `storage`(字符串 KV)选项保持可用。
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
feat(lint): `has(x)` 不是 null 守卫 —— 发布期直接拒绝未守卫的可空比较 (#4763)
6+
7+
CEL 的 `has(x)` 问的是**键是否存在**。自 #4649 起,谓词读到的记录对对象声明的每个
8+
字段都是**全量**的:一个声明了却存 `NULL` 的列同样"存在",所以
9+
`has(record.end_date)` 对声明字段恒为 `true`,什么也没告诉作者。于是这个读起来
10+
像守卫的写法根本不是守卫:
11+
12+
```text
13+
has(record.start_date) && has(record.end_date) && record.end_date < record.start_date
14+
```
15+
16+
它会走到 `null < null`,CEL 没有对应重载,整个谓词中断。#4761 之前中断被吞掉
17+
(规则跳过,一条 WARN),也就是说**这一形状的规则在任何含 null 值的行上从未生效
18+
**——它写在元数据里、读起来完全正确、却什么都没有强制执行。#4761 把运行时改成
19+
fail-closed 之后,当场就在我们自己的两个示例对象里抓到了它。
20+
21+
运行时拒绝是兜底,不是该学到这件事的地方:作者会在真实数据(很可能是生产数据)
22+
上收到一个 400,离写下规则可能已经过去几个月。而这个错误**仅凭元数据就可判定**
23+
——谓词的 AST 加上对象声明的字段类型,就足以判断某个操作数是否可能为 null。按
24+
AGENTS.md PD #12(在创作期拒绝,不要在消费端容忍),它属于发布闸门。
25+
26+
**新增闸门(error,直接拒绝,没有降级开关)。** `os build` / `os validate` /
27+
`os lint` 与运行时发布闸门共用的 `validateStackExpressions` 现在会拒绝这样的谓词:
28+
**声明为可空**的字段(没有 `required: true`、没有 `defaultValue`、没有默认选项、
29+
不是 autonumber)应用**排序**(`< <= > >=`)或**算术**(`+ - * / %`,含一元 `-`)
30+
运算符,而该操作数没有被同一布尔分支内支配它的 `!= null` / `== null` / `!isBlank()`
31+
显式判空所守卫。`has(x)` **刻意不**计入守卫——这正是本规则存在的理由。错误信息点名
32+
规则、操作数与修法,收尾句逐字取自 `rule-validator.ts``unevaluableRuleError`,
33+
两道闸门措辞完全一致。
34+
35+
覆盖面(有意划定,而不是含糊地覆盖一半):对象**校验规则**(含 `conditional` 规则
36+
`then` / `otherwise` 里嵌套的谓词)与**生命周期 hook 的 `condition`** ——即真正由 CEL
37+
在全量记录上求值、会 fail-closed 的两类面。共享规则条件(下推成 SQL 过滤,`NULL > x`
38+
是三值逻辑,不会 fault)、flow 的扁平作用域条件(裸标识符可能是 flow 变量)与
39+
`Field.formula`(有自己的 #3306 `guard ? value : null` 处理)不在此列。
40+
41+
**未声明**键的 `has()` 完全不受影响——那才是它的正当用途:区分"这次 PATCH 里
42+
根本没提到这个键"与"显式写了 null"。示例应用无需改动即通过新闸门。
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
---
3+
4+
chore(i18n): drop the undeclared `name:` key from all nine `scripts/i18n-extract.config.ts`
5+
6+
Releases nothing — build-time-only extract fixtures (`scripts/` is not in any
7+
package's published `files`), no runtime or published behaviour changes.
8+
9+
Every one of the nine extract configs opened its `defineStack({ … })` with a
10+
`name:` that the stack schema does not declare, so `ObjectStackDefinitionSchema`
11+
dropped the value at load and the #4167 unknown-stack-key lint reported it —
12+
once per package, on every `pnpm check:i18n` run, in a run that was otherwise
13+
fully green:
14+
15+
```
16+
defineStack: stack.name: 'name' is not a declared stack key, so its value is dropped at load — did you mean 'pages'?
17+
```
18+
19+
The lint was right and the configs were wrong: nothing has ever read a stack's
20+
top-level `name``os i18n extract` receives the *parsed* `defineStack` result,
21+
from which the key is already gone — so the nine values were inert. The fix is
22+
at the producer (#4736 decision A: delete the nine keys), not a new authorable
23+
key in `packages/spec` to accommodate one typo copied nine times.
24+
25+
Extraction output is unchanged: after the deletion a full
26+
`node scripts/check-i18n-bundles.mjs --write` regenerates all 40 bundles across
27+
the nine packages with a byte-identical result, and `pnpm check:i18n` stays
28+
green — now without the warning.

content/docs/data-modeling/validation.mdx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ export const Order = ObjectSchema.create({
5959
Condition expressions are **CEL** (evaluated by `@objectstack/formula`), not Salesforce-style formulas. Reference the incoming record via `record.<field>`, use `==`/`!=`, `&&`/`||`, and helpers like `isBlank(x)` and `has(record.field)`. A string condition is accepted as authoring shorthand and normalized to `{ dialect: 'cel', source }` at build time.
6060
</Callout>
6161

62+
<Callout type="warn">
63+
**`has(x)` is not a null guard.** Predicates see a record that is *total* over the object's declared fields, so `has(record.end_date)` is true even when the value is `NULL``has(a) && has(b) && a < b` then reaches `null < null`, CEL has no overload, and the whole rule aborts (the write is rejected fail-closed). Write `record.start_date != null && record.end_date != null && record.end_date < record.start_date` instead. Since #4763 the `has()` form is **rejected at build/publish**: an ordering or arithmetic operator over a declared nullable field needs a real `!= null` guard. `has()` over an *undeclared* key — "was this in the PATCH at all?" — is untouched.
64+
</Callout>
65+
6266
## Common Properties
6367

6468
All validation types share these base properties:

packages/lint/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"typecheck": "tsc --noEmit"
2626
},
2727
"dependencies": {
28+
"@marcbachmann/cel-js": "^8.0.0",
2829
"@objectstack/formula": "workspace:*",
2930
"@objectstack/sdui-parser": "workspace:*",
3031
"@objectstack/spec": "workspace:*",

packages/lint/src/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,16 @@ export type { WidgetBindingFinding, WidgetBindingSeverity } from './validate-wid
2727
export { validateStackExpressions } from './validate-expressions.js';
2828
export type { ExprIssue } from './validate-expressions.js';
2929

30+
// #4763 — `has(x)` reads as a null guard and is not one. The decision procedure
31+
// is exported on its own so other authoring surfaces (cloud graph-lint, the AI
32+
// authoring path) reuse ONE verdict instead of re-deriving it.
33+
export {
34+
findUnguardedNullableOperands,
35+
nullGuardMessage,
36+
NULL_GUARD_HINT,
37+
} from './validate-null-guards.js';
38+
export type { NullGuardFinding, NullGuardOptions } from './validate-null-guards.js';
39+
3040
export { validateListViewMode, LIST_VIEW_FILTERS_IN_VIEWS_MODE } from './validate-list-view-mode.js';
3141

3242
// [ADR-0078] The functional-completeness gate. All judgement lives in the shared

packages/lint/src/validate-expressions.test.ts

Lines changed: 207 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,13 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
341341
close_date: { type: 'date' },
342342
expected: { type: 'formula', formula: 'record.amount * record.probability / 100' },
343343
},
344-
validations: [{ name: 'future', expression: 'record.close_date >= today()' }],
344+
// The `!= null` guard is load-bearing since #4763: `close_date` is a
345+
// declared NULLABLE field, and an un-guarded `>=` over it faults at
346+
// runtime (`null >= timestamp` has no overload) — the null-guard gate
347+
// rejects that shape at authoring now. Soundness (this block's
348+
// subject) and null-guarding are separate verdicts; the predicate has
349+
// to satisfy both to produce zero issues.
350+
validations: [{ name: 'future', expression: 'record.close_date != null && record.close_date >= today()' }],
345351
}],
346352
});
347353
expect(issues).toHaveLength(0);
@@ -758,3 +764,203 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
758764
});
759765
});
760766
});
767+
768+
// ───────────────────────────────────────────────────────────────────────
769+
// #4763 — `has(x)` reads as a null guard and is not one.
770+
//
771+
// Scope note (constraint of the issue, pinned here so it stays a decision):
772+
// this gate walks the AUTHORED METADATA the stack carries — object validation
773+
// rules and lifecycle-hook conditions. It never reads source files, so the
774+
// deliberately-bad fixtures in `packages/objectql/src/validation/rule-*.test.ts`
775+
// (which pin the runtime's fail-closed behaviour and MUST keep the bad shape)
776+
// are structurally out of its reach.
777+
// ───────────────────────────────────────────────────────────────────────
778+
describe('null-guard gate (#4763)', () => {
779+
// Mirrors `showcase_project`: dates and money are declared but nullable;
780+
// `status` carries a default option and `name` is required, so neither can
781+
// be null and neither may ever be flagged.
782+
const project = {
783+
name: 'showcase_project',
784+
fields: {
785+
name: { type: 'text', required: true },
786+
status: { type: 'select', options: [{ value: 'planned', default: true }, { value: 'active' }] },
787+
start_date: { type: 'date' },
788+
end_date: { type: 'date' },
789+
budget: { type: 'currency' },
790+
spent: { type: 'currency', defaultValue: 0 },
791+
},
792+
};
793+
const withRule = (rule: Record<string, unknown>) =>
794+
validateStackExpressions({ objects: [{ ...project, validations: [rule] }] });
795+
796+
it('REJECTS the `has(a) && has(b) && a < b` shape over nullable declared fields', () => {
797+
const issues = withRule({
798+
type: 'script',
799+
name: 'end_after_start',
800+
condition: 'has(record.start_date) && has(record.end_date) && record.end_date < record.start_date',
801+
});
802+
expect(issues.length).toBeGreaterThan(0);
803+
expect(issues.every((i) => (i.severity ?? 'error') === 'error')).toBe(true);
804+
const joined = issues.map((i) => i.message).join('\n');
805+
// names the rule …
806+
expect(joined).toContain("validation rule 'end_after_start'");
807+
// … the operand …
808+
expect(joined).toContain('record.end_date');
809+
expect(joined).toContain('record.start_date');
810+
// … and the fix, in the runtime's own words.
811+
expect(joined).toContain("Guard it with '!= null'");
812+
expect(joined).toContain('has(x)');
813+
expect(issues[0].where).toContain("object 'showcase_project'");
814+
});
815+
816+
it('ACCEPTS the `!= null` form (the fix #4761 landed in the examples)', () => {
817+
expect(
818+
withRule({
819+
type: 'script',
820+
name: 'end_after_start',
821+
condition:
822+
'record.start_date != null && record.end_date != null && record.end_date < record.start_date',
823+
}),
824+
).toHaveLength(0);
825+
});
826+
827+
it('ACCEPTS a guarded arithmetic predicate (showcase `spent_within_budget`)', () => {
828+
expect(
829+
withRule({
830+
type: 'script',
831+
name: 'spent_within_budget',
832+
condition: 'record.budget != null && record.spent != null && record.spent > record.budget * 1.2',
833+
}),
834+
).toHaveLength(0);
835+
});
836+
837+
it('never flags a required field or one with a default (`spent`, `status`, `name`)', () => {
838+
expect(
839+
withRule({ type: 'script', name: 'spend_positive', condition: 'record.spent > 0' }),
840+
).toHaveLength(0);
841+
});
842+
843+
it('reaches the predicates nested in a `conditional` rule’s then/otherwise', () => {
844+
const issues = withRule({
845+
type: 'conditional',
846+
name: 'budget_sanity',
847+
when: "record.status == 'active'",
848+
then: { type: 'script', name: 'over_budget', condition: 'has(record.budget) && record.budget > 1' },
849+
});
850+
expect(issues.length).toBe(1);
851+
expect(issues[0].message).toContain('record.budget');
852+
expect(issues[0].where).toContain("'budget_sanity' then → 'over_budget'");
853+
});
854+
855+
// Negative-case pin: the real `showcase_account` rule pair. Both use `has()`
856+
// — legitimately, to tell "key absent from the PATCH" apart from "explicit
857+
// null" — and both compare with EQUALITY only. They must stay legal; a rule
858+
// that flags them is too broad.
859+
it('leaves `showcase_account.churn_reason_consistency` alone (legitimate `has()`)', () => {
860+
const issues = validateStackExpressions({
861+
objects: [{
862+
name: 'showcase_account',
863+
fields: { status: { type: 'select', options: [{ value: 'churned' }] }, churn_reason: { type: 'text' } },
864+
validations: [{
865+
type: 'conditional',
866+
name: 'churn_reason_consistency',
867+
when: "record.status == 'churned'",
868+
then: {
869+
type: 'script',
870+
name: 'churn_reason_present',
871+
condition: "!has(record.churn_reason) || record.churn_reason == null || record.churn_reason == ''",
872+
},
873+
otherwise: {
874+
type: 'script',
875+
name: 'churn_reason_absent',
876+
condition: "has(record.churn_reason) && record.churn_reason != null && record.churn_reason != ''",
877+
},
878+
}],
879+
}],
880+
});
881+
expect(issues).toHaveLength(0);
882+
});
883+
884+
describe('hook conditions — the third instance the issue named', () => {
885+
const hookStack = (condition: string) => ({
886+
objects: [project],
887+
hooks: [{ name: 'project_budget_alert', object: 'showcase_project', condition }],
888+
});
889+
890+
// Regression pin. `examples/app-showcase/src/data/hooks/index.ts` carried
891+
// `has(record.spent) && has(record.budget) && record.spent > record.budget`
892+
// until #4770/#4786 corrected it. This asserts the bad shape cannot come
893+
// back: it is red today, and would have been red before that fix.
894+
it('REJECTS the pre-#4786 showcase hook shape', () => {
895+
const issues = validateStackExpressions(
896+
hookStack('has(record.spent) && has(record.budget) && record.spent > record.budget'),
897+
);
898+
expect(issues.length).toBeGreaterThan(0);
899+
expect(issues[0].where).toContain("hook 'project_budget_alert'");
900+
expect(issues.map((i) => i.message).join('\n')).toContain('record.budget');
901+
});
902+
903+
it('ACCEPTS the corrected shape now on `main`', () => {
904+
expect(
905+
validateStackExpressions(
906+
hookStack('record.spent != null && record.budget != null && record.spent > record.budget'),
907+
),
908+
).toHaveLength(0);
909+
});
910+
911+
it('applies per target for a multi-object hook', () => {
912+
const issues = validateStackExpressions({
913+
objects: [project, { name: 'other_obj', fields: { budget: { type: 'currency', required: true } } }],
914+
hooks: [{ name: 'multi', object: ['showcase_project', 'other_obj'], condition: 'record.budget > 1' }],
915+
});
916+
// Only the object that declares `budget` nullable is flagged.
917+
expect(issues).toHaveLength(1);
918+
expect(issues[0].where).toContain('showcase_project');
919+
});
920+
});
921+
922+
describe('surfaces deliberately NOT covered', () => {
923+
it('leaves sharing-rule conditions alone (compiled to a SQL filter, never faults)', () => {
924+
expect(
925+
validateStackExpressions({
926+
objects: [project],
927+
sharingRules: [{
928+
name: 'big_budget',
929+
object: 'showcase_project',
930+
condition: "record.status == 'active' && record.budget > 100000",
931+
}],
932+
}),
933+
).toHaveLength(0);
934+
});
935+
936+
it('leaves flattened flow conditions alone (a bare id may be a flow variable)', () => {
937+
expect(
938+
validateStackExpressions({
939+
objects: [project],
940+
flows: [{
941+
name: 'escalate',
942+
nodes: [
943+
{ id: 'start', type: 'start', config: { objectName: 'showcase_project' } },
944+
{ id: 'd', type: 'decision', config: { condition: 'record.budget > 100000' } },
945+
],
946+
edges: [],
947+
}],
948+
}),
949+
).toHaveLength(0);
950+
});
951+
952+
it('leaves `Field.formula` expressions alone (blessed `guard ? value : null`, #3306)', () => {
953+
expect(
954+
validateStackExpressions({
955+
objects: [{
956+
...project,
957+
fields: {
958+
...project.fields,
959+
remaining: { type: 'formula', formula: 'record.budget - record.spent' },
960+
},
961+
}],
962+
}),
963+
).toHaveLength(0);
964+
});
965+
});
966+
});

0 commit comments

Comments
 (0)