Skip to content

Commit 47f3d12

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-4775-hook-condition-fail-loud
2 parents 70a6df2 + f4847a6 commit 47f3d12

52 files changed

Lines changed: 4487 additions & 553 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
fix(service-analytics): a measure a query never reported reads 0 for a count/sum on every merge seam (#4708)
6+
7+
A dataset measure carrying its own `filter` runs as a separate grouped
8+
sub-query and is merged back onto the selected dimensions. A `GROUP BY` over a
9+
filtered row set emits **no group at all** for a dimension value the filter
10+
excludes entirely, so the measure comes back **absent**, not `0` — and
11+
`computeDerived` treats an absent operand as unknowable, so every ratio over it
12+
goes null too. The cell then renders blank, which is visually identical to "no
13+
data for this row" and means the opposite.
14+
15+
The bias runs the worst possible way: the rows that blank are the ones whose
16+
numerator matched nothing — the **worst-performing rows**. A `lead_source` that
17+
won nothing rendered as "no data" while one that won everything rendered fine.
18+
19+
The empty-group value is now filled **by aggregate kind** into every measure
20+
column the assembled grid lists but no query reported:
21+
22+
| aggregate | over an excluded group | why |
23+
|:---|:---|:---|
24+
| `count`, `count_distinct` | `0` | "how many rows matched" has an exact answer when the answer is none |
25+
| `sum` | `0` | the identity element of the empty set |
26+
| `avg`, `min`, `max` | stays `null` | genuinely undefined — there is nothing to average |
27+
28+
Filling all five with `0` would trade this lie for its mirror image, reporting a
29+
measurement nobody made, so the kinds are judged separately (via
30+
`emptyGroupValueFor`, shared with the authoring-side coherence checks).
31+
32+
**Only cells are filled, never rows.** A dimension value no query reported at
33+
all has genuinely no data and stays out of the grid.
34+
35+
**What changes beyond the measure-scoped seam.** The fill previously ran before
36+
the `compareTo` merge, and that merge *appends* a row for every bucket the
37+
PREVIOUS window had and this one does not. Every base measure on those rows —
38+
including unfiltered ones — was absent, so a lead source that sold last month
39+
and nothing this month rendered as "no data" instead of `0`: the same worst-row
40+
bias, one merge later. The fill now runs after every merge and covers all base
41+
measures plus their `<measure>__compare` columns.
42+
43+
Widgets that worked around this with `?? 0` in the consumer or a `coalesce` in
44+
the measure can drop it; the coercion belongs in the executor, which is the only
45+
layer that knows which aggregate produced the gap.
46+
47+
**New export.** `fillEmptyGroups(rows, columnAggregates)` is exported from the
48+
package root beside `mergeByDimensions`, so a host assembling a grid outside
49+
`DatasetExecutor` can apply the same aggregate-kind rule rather than
50+
reimplementing it — which is what makes this a `minor` rather than a `patch`.
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
"@objectstack/plugin-approvals": patch
3+
---
4+
5+
fix(approvals): the record lock now holds for predicate (`multi`) updates (#4778)
6+
7+
The ADR-0019 record lock — "while a record has a pending `sys_approval_request`,
8+
block edits to it" — was enforced only for updates that reach the hook with an
9+
`input.id`. The engine extracts that id from a **scalar** `where.id` alone; an
10+
operator object (`{ $in: [...] }`) or any other predicate is a multi-row write
11+
that routes to `updateMany` and arrives with no id. The hook opened with
12+
`if (!id) return`, so it read *"no row was resolved"* as *"there is nothing to
13+
authorize"* when the truth was *"nothing was ever queried"*.
14+
15+
Rewriting the very same edit as `multi: true` therefore walked straight past the
16+
lock:
17+
18+
```ts
19+
// rec_1 carries a pending approval, lockRecord is not disabled
20+
await ql.update('crm_opportunity', { amount: 999 }, { where: { id: 'rec_1' } }); // RECORD_LOCKED
21+
await ql.update('crm_opportunity', { amount: 999 }, { where: { id: { $in: ['rec_1'] } }, multi: true }); // went through
22+
await ql.update('crm_opportunity', { amount: 999 }, { where: { name: 'x' }, multi: true }); // went through
23+
```
24+
25+
No privilege was needed for that bypass — not an `admin` role, not `isSystem`,
26+
not `lockRecord: false`, not a whitelisted `approvalStatusField`. Every caller
27+
shape that can spell a predicate (SDK, ObjectQL, a flow's `update_record`) could
28+
produce it. It is the same fail-open reasoning fixed for `sys_attachment`
29+
(#4757) and `sys_comment` (#4630), in the one place where it needed no
30+
privilege at all.
31+
32+
**The hook now resolves the rows a write touches before deciding.** By-id writes
33+
are unchanged (the driver writes by primary key, so the rest of `where` must not
34+
narrow the verdict). A predicate write is decided by intersecting the caller's
35+
predicate with the records that are actually locked — which is also what keeps
36+
it cheap: the query is bounded by the object's **pending approvals**, never by
37+
the update's match set, so a mass update of 50 000 unlocked rows costs one
38+
bookkeeping probe and is allowed. An unscoped `multi` update over the whole
39+
table reaches every locked row of the object and is refused while any is held.
40+
41+
**Fail-closed, both ways.** Past 1 000 locked records — the bound the attachment
42+
and comment guards use — or if the intersection query fails, the write is
43+
refused rather than allowed: the lock could not prove the write misses a locked
44+
row. The approvals bookkeeping being unreadable at all stays the one fail-open,
45+
as before: this hook is global over every object, so a kernel without
46+
`sys_approval_request` would otherwise refuse every update in the deployment.
47+
Both the bookkeeping and the match-set resolution are read under a **system**
48+
context — a guard's own input must never be narrowed by the caller's
49+
visibility, since a locked row you cannot read is still a row you may not write.
50+
51+
**Every exemption moved with the guard**, which is the other way this class of
52+
fix goes wrong — a guard extended to more rows that carries only its deny rules
53+
turns a fail-open into a false-positive. `isSystem`, the `admin` override, the
54+
`approvalStatusField` status mirror, `lockRecord: false` and the owning run's
55+
`flowRunId` (#3456 / #3712) all decide a predicate write exactly as they decide
56+
a by-id write, each pinned by tests on both predicate shapes. Refusals now name
57+
the record and object that are locked.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
---
3+
4+
chore(devx): `pnpm check:i18n` now FAILS on an undeclared authoring key, not just on bundle drift
5+
6+
Releases nothing — the change is confined to `scripts/check-i18n-bundles.mjs` and
7+
the root `check:i18n` script. No package source, no published behaviour, and
8+
deliberately **not** the `os i18n extract` exit code (that would write an internal
9+
hygiene rule into the public CLI contract).
10+
11+
#4736 cleaned nine `scripts/i18n-extract.config.ts` files that all opened their
12+
`defineStack({ … })` with the same undeclared `name:` key. Nine, because the same
13+
mistake was copied from the first one — and nothing stopped any of them. The
14+
#4167 unknown-authoring-key lint *saw* every single one: it printed
15+
`stack.name: 'name' is not a declared stack key, so its value is dropped at load`
16+
on stderr, once per package, on every run. But the CLI exited 0 and the gate only
17+
judged bundle drift, so those nine warnings appeared inside a **fully green**
18+
`check:i18n` and were read as noise nine times. A warning that nine authors
19+
filtered out is not a control; #4736 cleaned the symptom, this closes the hole.
20+
21+
**What changed.** The gate now reads the extractor's **stderr** — which it
22+
previously let flow straight through to the terminal, seen by nobody and judged by
23+
nothing — and fails on the unknown-authoring-key signature. Coverage needs no
24+
manifest: `findConfigs` walks `packages/`, so the tenth config is gated the day it
25+
lands.
26+
27+
**The two verdicts stay separate.** Bundle drift keeps its own section and its own
28+
remedy (`--write`); the new class gets its own, naming the package, the config
29+
path, the key, and the consequence that matters — *the value is dropped at load,
30+
so whatever it was meant to configure is not in effect and never was*.
31+
Regenerating bundles does not fix it, and the message says so.
32+
33+
**The gate is proven able to go red**, not merely observed green — the failure
34+
mode of `check:react-declaration-parity` (#4690), which exited 0 with nothing to
35+
check. Two proofs: `node scripts/check-i18n-bundles.mjs --self-test` (now wired
36+
into `check:i18n`, ahead of the real run) drives both classifiers over recorded
37+
CLI output, including a case asserting neither verdict matches the other's output;
38+
and the gate was run against the nine configs restored from `ffab8033b^`, the
39+
commit before #4803 deleted the keys, where it reports all nine and exits 1.
40+
41+
Fixing an offending config means deleting the key at the producer. If a key is
42+
genuinely wanted it gets declared in `packages/spec` deliberately — not
43+
accommodated by a consumer-side fallback, and not silenced by making
44+
`ObjectStackDefinitionSchema` strict (which would mute the lint itself; see
45+
`metadata-authoring-lint.ts`).
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
feat(lint): 视图 `searchableFields` 按运行时同一套判定做构建期校验 —— 一个 lookup 笔误不再等到 400 才暴露 (#4830)
6+
7+
视图(list view)的 `searchableFields` 会被客户端逐字回显为 `$searchFields` 覆盖参数,而
8+
REST 入口闸(#4254)会用 `resolveSearchFieldResolution`(`@objectstack/spec/data`)判定
9+
该对象的可搜索集合 —— 声明一个 lookup 等「不可搜索」字段,运行时会把**整条查询** 400
10+
(`INVALID_FIELD`),列表工具栏搜索对全体角色彻底不可用。此前 `compile`/`validate` 只查
11+
字段**存在性**,这类笔误全绿放行,只能靠人肉点搜索框发现。
12+
13+
新增规则 `searchable-field-unsearchable`(error 级,新导出常量同名):对每个视图级
14+
narrowing(对象内建 `listViews``defineView``list`/`listViews`、react 页面的
15+
`<ListView searchableFields>`)按**运行时同一个函数**(`resolveSearchFieldResolution`,
16+
非复制的类型清单,杜绝再度漂移)判定 declared = enforced:
17+
18+
- 对象未声明 `searchableFields`(auto 源):视图里出现 lookup/json/hidden/审计列等
19+
auto-default 拒绝的字段 → 构建期 error,信息含类型与 400 后果,lookup 给出「镜像到本
20+
对象 text/formula 字段」的处方;
21+
- 对象已声明(declared 源):视图条目超出对象声明集合 → 构建期 error(视图只能收窄、
22+
不能放宽,ADR-0061);
23+
- 对象自身的 `searchableFields`(canonical)维持**只查存在性**:运行时 declared 分支按
24+
存在过滤、不按类型过滤,声明即被引擎执行,构建期拒绝会误伤运行时接受的元数据
25+
(ADR-0072 D1);
26+
- 注册表注入的系统列在 narrowing 中跳过判定(其运行时元数据对 linter 不可见,宁可漏报
27+
不可误报)。
28+
29+
内部核心 `checkSearchableFieldList` / `indexObjectSearchTargets`(模块级导出,未入包
30+
barrel)签名有变:索引值从 `Set<string> | null` 变为 `ObjectSearchTarget | null`,并新增
31+
可选 `role: 'canonical' | 'narrowing'`(默认 `'narrowing'`)参数。
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
---
2+
"@objectstack/spec": major
3+
---
4+
5+
refactor(spec)!: remove `activationEvents` (both keys) and the `ActivationEventSchema` vocabulary — lazy activation that no runtime ever implemented (#4657, ADR-0049)
6+
7+
`activationEvents` promised lazy plugin activation ("plugins remain dormant
8+
until an activation event fires") on two authorable surfaces —
9+
`DynamicLoadRequest.activationEvents` (`@objectstack/spec/kernel`) and
10+
`StudioPluginManifest.activationEvents` (`@objectstack/spec/studio`, the
11+
`defineStudioPlugin` input) — and **no runtime in objectstack / cloud /
12+
cloud-v1 / objectui ever read either key** (four-repo bare-name scan in #4657,
13+
re-verified at implementation time). Every plugin has always activated
14+
immediately on load/registration; cloud-v1's own ROADMAP recorded lazy
15+
activation as ❌ unimplemented (planned v0.4.0). That is ADR-0049's
16+
declared ≠ enforced shape in the semantically-lying direction: an author
17+
writing `activationEvents: [{ type: 'onMetadataType', pattern: 'flow' }]`
18+
expected deferral and got eager activation with a clean parse.
19+
20+
#4653 had just converged the two `ActivationEventSchema` declarations onto one
21+
structured `{ type, pattern }` form inside this same unreleased major; with the
22+
enforce-or-remove ruling landing on **remove**, that converged vocabulary
23+
retires before ever shipping. Composed across the two changes, a v16 author
24+
simply deletes the key in whichever form they carried.
25+
26+
Migration (FROM → TO):
27+
28+
- `activationEvents` in a `defineStudioPlugin` input / `StudioPluginManifest`
29+
value — v16 string form (`['*']`, `['onMetadataType:flow']`) or v17-rc
30+
structured form (`[{ type: 'onStartup', pattern: '*' }]`) alike →
31+
**delete the key**. There is no replacement value: eager activation is the
32+
only behaviour there has ever been, and `activate()` still runs at
33+
registration time. The strict manifest parse rejects the key (and its former
34+
VS Code-flavoured aliases `activation` / `events` / `onActivate`) with this
35+
prescription.
36+
- `activationEvents` in a `DynamicLoadRequest` value → **delete the key**.
37+
Tombstoned, not silently stripped — `DynamicLoadRequestSchema` is not
38+
`.strict()`, so a `retiredKey()` tombstone makes authoring it a `tsc` error
39+
and a parse error carrying the prescription.
40+
- `import { ActivationEventSchema, ActivationEvent } from '@objectstack/spec/kernel'`
41+
(or `/studio`) → **no replacement export** (TS2305 after upgrade). Nothing
42+
consumed the vocabulary; an exported schema with no consumer is read as a
43+
capability by whoever finds it (#3950), so the orphaned def goes with the
44+
keys.
45+
- Lazy activation is a **new capability**: if it is ever built it returns via
46+
the enforce route of ADR-0049 through a new ADR — executor first, vocabulary
47+
second — not by re-declaring inert keys.
48+
49+
Self-check (#4535 §5): TS2305 — yes, two removed exports on two entries;
50+
metadata migration — none possible or needed (`StudioPluginManifest` is TS
51+
configuration parsed by `defineStudioPlugin`, a root schema never stored in
52+
`sys_metadata`; `DynamicLoadRequest` is a runtime request shape with no
53+
caller — no stored row exists for a D2 conversion to rewrite, so the change is
54+
one ADR-0087 D3 semantic record, `plugin-activation-events-retired`); shape
55+
change — two keys removed, zero behaviour change (eager activation before and
56+
after, byte-identical).
57+
58+
The retirement kit: `retiredKey()` tombstone on the non-strict kernel schema;
59+
strict-parse `guidance` prescriptions on the studio manifest (including the
60+
three former aliases); ADR-0087 D3 semantic migration; baselines
61+
(`authorable-surface.json` — one `[RETIRED]` line, five lines dropped
62+
deliberately with the defs; `json-schema.manifest.json``kernel/ActivationEvent`
63+
and `studio/ActivationEvent` def removals; `api-surface.json`) regenerated
64+
deliberately; compiler-API export pin (`activation-events-retirement.test.ts`,
65+
zero holders across every public entry) — sabotage-verified.

.changeset/seed-env-enforced.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(seed): enforce `Seed.env` — environment-scoped datasets no longer seed everywhere
6+
7+
`Seed.env` was authorable, defaulted and type-checked, but inert. `SeedLoaderService`
8+
filtered on the **loader config's** `env`, and none of the six call sites that build a
9+
`SeedLoaderRequest` (app boot, per-org replay, hot reload, package apply, draft publish,
10+
marketplace install) ever passed one — so `config.env` was always `undefined`, the filter
11+
short-circuited, and `dataset.env` was never read. A dataset marked `env: ['dev']` seeded
12+
into production exactly as if it were marked `['prod']`, which is the dangerous direction:
13+
the rows most likely to carry that marking are demo users, fake customers and seeded
14+
credentials.
15+
16+
The loader now resolves the environment itself, at the one funnel every seeding path goes
17+
through:
18+
19+
- **Source is `NODE_ENV`** — the environment source this repo already uses everywhere
20+
(`os start` defaults it to `production`, `os dev` / `serve --dev` set `development`,
21+
vitest sets `test`). No new environment variable and no new authorable key. `production`
22+
/ `development` / `test` and the seed-enum spellings `prod` / `dev` are accepted,
23+
case-insensitively.
24+
- **An explicit `config.env` still wins**, so a host can seed "as" another environment.
25+
- **A dataset that declares no `env`** (the schema default `['prod','dev','test']`) seeds
26+
in every environment, exactly as before — no existing deployment loses rows.
27+
- **When the environment cannot be determined** (NODE_ENV unset, or a value like
28+
`staging`), the loader stays permissive and seeds everything — but logs a **warning**
29+
naming each environment-scoped dataset, the accepted `NODE_ENV` values and the
30+
`config.env` escape hatch. Fail-open is deliberate: fail-closed would also drop an
31+
`env: ['prod']` dataset on a production host that merely forgot to export `NODE_ENV`,
32+
a silent data-loss regression worse than the over-seeding it prevents.
33+
- **Skipped datasets are always named** in an `info` log, so "my demo rows are missing" is
34+
one log line to answer rather than a mystery.
35+
36+
The resolved environment is also what seed CEL expressions now bind `env` to, so a seed's
37+
`env` and the loader's filter can no longer disagree.
38+
39+
No API or schema change: `Seed.env` and `SeedLoaderConfig.env` are unchanged, and no
40+
package export was added.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
---
3+
4+
Tooling-only: `pnpm check:startup-registry-verdict` — startup registry reads may not record a verdict the boot can still contradict (#4777). Adds `scripts/check-startup-registry-verdict.mjs` + the shrink-only `scripts/startup-registry-verdict.baseline.json` (empty on landing), a `Lint & Type Check` step, and an AGENTS.md section. Releases nothing — no package changes.
5+
6+
One showcase cold start on 2026-08-03 produced three instances of one shape in three unrelated subsystems written by three people at three times: ask a registry "is X there?" while the boot is still filling it, treat the "no" as final, and **record** it — cached on the instance (#4772 plugin-auth), asserted in a `warn` (#4771 service-automation), or written to the database (#4769 objectql). The provider registers a moment later and nothing undoes the record. All three are fixed; this is what stops the class from coming back.
7+
8+
The gate matches the three-part shape, and part 3 is what makes it a rule rather than noise — a read-only probe stays completely legal, and the cures are never flagged: a probe deferred into a lazy accessor or a `kernel:ready` hook, a probe whose ordering an ADR-0116 declaration (`dependencies` / `optionalDependencies` / `requiresServices`) has already made final, and a verdict drawn at a declared seal (`sealNodeTypeVocabulary()`) all pass.
9+
10+
Its reach is stated rather than implied: it under-matches on purpose. `getService('cache')` is visible, a `resolveCacheOrFallback()` three layers down another package is not, and #4769 is invisible to it entirely — that "registry" is the `sys_migration` table in a database. This stops the bleeding; it does not cure. Whether the kernel contract should be tightened further is #4776.

0 commit comments

Comments
 (0)