Skip to content

Commit 8391dea

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-4793-batch-row-result-schema
2 parents 2a58532 + 61cc079 commit 8391dea

60 files changed

Lines changed: 5098 additions & 352 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.

.changeset/book-job-translation-app-authorwarn-keys-retired.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,14 @@ Run `os migrate meta --from 16` to rewrite existing sources automatically.
3939
migration table steered retired `errors:` authors straight into it. **That
4040
guidance entry is rewritten here**: retiring one dead key by pointing at
4141
another is the defect, not the fix.
42-
- **`app.homePageId`***its own hedge*. "If not set, usually defaults to the
43-
first navigation item" described the only behaviour there was.
42+
- **`app.homePageId`***a second source for one fact*. Not unread: objectui's
43+
console consumed it in `resolveLandingRoute()` and it was the only thing
44+
deciding where an app opened. (This entry first shipped saying otherwise;
45+
corrected in #4709, which upheld the removal.) What condemns the key is its
46+
shape — an ID cross-reference into `navigation` with no referential integrity,
47+
falling back to the first item *silently* when the id dangled. If "land
48+
somewhere other than first" is ever wanted again it belongs on the navigation
49+
item itself, not on a pointer that can miss.
4450
- **`app.areas[].order`***the sibling that works*. Nav-item `order` really is
4551
sorted; area-level order never was, and both renderers iterate the array as
4652
authored.
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: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
docs(spec): `app.homePageId` 的墓碑说清真正的退役理由 —— 「no shell ever read it」是假的 (#4709)
6+
7+
**改的是「为什么删」的表述,不是删本身。** `app.homePageId` 在 17.0.0 依旧退役
8+
(`retiredKey`:编译期 `never`、解析期报错),conversion `app-dead-authoring-keys-removed`
9+
的行为、baseline、`os migrate meta --from 16` 的处方一字未动。
10+
11+
#4667 给出的理由是「no shell ever read it」。这句是**假的**,而且与本仓自己的记录直接
12+
矛盾 —— 2026-06 的 AppSchema liveness 审计
13+
(`docs/audits/2026-06-appschema-property-liveness.md`)把 `homePageId` 明确列在 LIVE
14+
一侧,因为 objectui console 的 `resolveLandingRoute()`
15+
(`packages/app-shell/src/console/AppContent.tsx`,objectui @785b8a5d)一直在读它,而且
16+
它是**唯一**决定「app 打开时落在哪」的地方。两份文档矛盾了两个月无人发现,直到有人做
17+
cloud pin 对账时先信了这句、再去核渲染器才发现不对(#4709)。
18+
19+
真正让这个键该走的是它的**形状**,不是无人使用:它把落地页编码成指向 `navigation`
20+
ID 交叉引用,没有引用完整性 —— id 悬空时**静默**回退到第一项(objectui 的实现正是如此),
21+
于是同一件事有两个来源,而错的那个不出声。将来若要「落地页 ≠ 第一项」,正确形状是导航项
22+
自身的标记(`navigation[].landing`:单一来源、不可能悬空),并按 enforce-first 设计
23+
(先有渲染器与测试,再进 schema)。
24+
25+
墓碑文案改为诚实版本后,作者看到的处方**保持不变**:删掉这个键;要改 app 从哪里打开就
26+
重排 `navigation` 让目标项排第一;根落地由 `isDefault` 决定。同步纠正:conversion 摘要
27+
(经 `gen:upgrade-guide` / `gen:spec-changes` 重生成到 `docs/protocol-upgrade-guide.md`
28+
`spec-changes.json`)、生成文档 `content/docs/references/ui/app.mdx`
29+
`content/docs/ui/apps.mdx`、liveness ledger 的 `homePageId` note、`examples/app-showcase`
30+
里那句「has no console consumer yet」,并给 6 月审计补了一条指向 #4667/#4709 的后续注记
31+
(审计结论本身是对的,原文不动)。新增一条 pin 测试,防止「无人读过」这类假前提回潮。
32+
33+
objectui 侧那段永远进不去的 `if (homePageId)` 死分支单独清理:
34+
`objectstack-ai/objectui#3264`
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
---
2+
"@objectstack/objectql": major
3+
---
4+
5+
feat(objectql)!: a hook `condition` the platform cannot evaluate now ABORTS the operation (#4775)
6+
7+
**Breaking.** A declarative hook whose `condition` cannot be evaluated used to
8+
emit a `logger.warn` and `return false` — the hook simply did not fire. Existing
9+
hooks that have been getting by on that silent skip will now **fail the write**.
10+
That is the point of the change, not a side effect: those conditions were never
11+
enforcing anything, and the failure is how you find out.
12+
13+
## What changed
14+
15+
"The condition said no" and "the platform could not work out what the condition
16+
says" used to collapse into one outcome, and that one outcome carries **opposite**
17+
risks depending on the hook:
18+
19+
- a `before*` guard ("hold this write when the condition is met") swallowed into
20+
`false` **lets through** a write it was declared to stop;
21+
- an `after*` audit ("leave a trace when the condition is met") swallowed into
22+
`false` **drops** a row nobody will go looking for, because nobody knows it
23+
should exist.
24+
25+
So an unevaluable condition is `declared ≠ enforced`, and it is now resolved the
26+
way #4649 already resolved it for validation predicates one module over: reject
27+
loudly, naming the hook and the key that would not resolve. The rejection is a
28+
`HookConditionError` (exported), carrying `hook` / `object` / `event` /
29+
`condition` / `reason` / `fault` / `missingKey` machine-readably.
30+
31+
`before*` and `after*` take the **same** direction, knowingly: a typo in an
32+
`afterUpdate` audit condition fails the write it was only watching. One rule, one
33+
answer — the platform does not grow a hidden second rule that makes the failure
34+
direction depend on the event name.
35+
36+
A condition that never **compiled** aborts too. Its old treatment
37+
(`condition ignored`) was the worse half of the swallow: the gate disappeared
38+
entirely, so a declared guard let every write through and an audit fired on all
39+
of them. It is reported at invocation rather than at bind time, so one broken
40+
hook cannot wedge boot for an app nobody is writing to.
41+
42+
## What did NOT change
43+
44+
- A condition that evaluates **FALSE** is still just a skip, and the write still
45+
succeeds. Only *unevaluable* is new.
46+
- `onError` (`abort` / `log`) is untouched and is deliberately **not** in this
47+
path. It governs a handler that threw; the condition gate runs before the
48+
handler is ever reached. Routing a condition fault through it would let
49+
`onError: 'log'` resurrect the exact silent skip this change abolishes, and
50+
would mint a third set of semantics for one word. `retryPolicy` and `async`
51+
are outside it for the same reason.
52+
53+
## Predicate (`multi: true`) bulk writes (#4800)
54+
55+
A bulk write matches N rows and fires the hook **once**, so `previous` is unbound
56+
and `record` is the bare payload — there is no single prior record, and
57+
materialising declared fields to `null` would state something false about all N.
58+
Fail loud takes **no exception** here, but the message is a diagnosis rather than
59+
a riddle: it names the hook, says *this is a predicate bulk write and there is no
60+
single prior record*, and gives the route that works (rewrite without `previous`,
61+
or target the write at one record by id).
62+
63+
It deliberately does **not** offer "use a record-change flow trigger instead":
64+
that trigger subscribes to these same lifecycle hooks, so on a bulk write it
65+
fires once with `previous` undefined too — verified against
66+
`trigger-record-change` and the engine, not assumed. Pointing at it would have
67+
made this very message the next `declared ≠ delivered`.
68+
69+
An **undeclared** key on a bulk write still gets the ordinary typo message — that
70+
one really is a misspelling, and calling it a batch problem would send the author
71+
to fix a field that is spelled correctly.
72+
73+
## Migrating
74+
75+
Run your app and watch for `HookConditionError`. Each one names the hook and the
76+
key. The usual causes, in order of frequency:
77+
78+
- **a misspelled or retired field** — fix the condition, or declare the field;
79+
- **an unguarded `null` comparison** (`record.spent > record.budget`) — guard
80+
with `!= null`. Note `has(x)` does **not** do this: a declared field holding
81+
`null` is still PRESENT, so `has(x)` is `true` and the ordering comparison
82+
still faults;
83+
- **`previous` on a bulk write** — rewrite without `previous`, or write by id;
84+
- **a bare identifier** (`done == true`) — hook conditions are `record`-scoped,
85+
so write `record.done == true`. Flow/automation conditions, which flatten
86+
fields to top level, are a different surface and are unaffected.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
"@objectstack/types": minor
3+
"@objectstack/verify": minor
4+
"@objectstack/cli": patch
5+
---
6+
7+
fix(verify): resolve the enterprise organizations package from the HOST APP (#4700)
8+
9+
`bootStack(app, { multiTenant: true })` — and therefore `objectstack verify
10+
--multi-tenant` — could never load `@objectstack/organizations`. Node ESM
11+
resolves a bare `import()` against the **importer's own realpath**, which for
12+
`packages/verify` is inside the framework workspace, while the enterprise
13+
package is cloud-private and only ever lives in the verified app's
14+
`node_modules`. Every real host app fell into the catch and was told to
15+
"Install/link it in this workspace" — about a package it had already installed.
16+
Same defect class as cloud#1013, which fixed `objectstack serve`; #4699 fixed
17+
that one call site and this issue tracked the two the sweep left behind.
18+
19+
**New: `@objectstack/types/node`.** The host-app resolver (`createHostRequire` /
20+
`createHostImporter`) moved out of `packages/cli/src/utils/import-from-host.ts`
21+
— where `@objectstack/verify` and the dogfood suite could not import it without
22+
inverting the dependency direction — into a **node-only subpath export** of
23+
`@objectstack/types`. One behaviour, one source; the CLI now consumes it and its
24+
private copy is deleted.
25+
26+
It is a subpath and **not** the root export because `@objectstack/types` is a
27+
dependency of `@objectstack/hono` ("edge-compatible REST API server for
28+
Cloudflare Workers, Deno, Bun, and Node") and of the plugin layer a `LiteKernel`
29+
boots on Workers. The root entry reaches zero `node:` builtins, and a Workers
30+
bundle breaks on `node:module` even when nothing calls it. `tsup` emits the two
31+
entries as separate self-contained bundles (`splitting: false`), and a test
32+
walks the root's import graph and fails on the first reachable `node:`
33+
specifier, so the isolation is enforced rather than merely intended. Same
34+
arrangement `@objectstack/metadata` already ships for its `./node` subpath.
35+
36+
**New: `BootOptions.hostRoot`** (optional, defaults to `process.cwd()`) names
37+
the app whose `node_modules` supplies those optional packages — for a harness
38+
booting an app that is not the working directory.
39+
40+
**The dogfood multi-org gates had never run.** Two suites probed availability
41+
with the same bare `import()` and so were **constant-false** — not "false
42+
because absent" but false by construction, in every environment including the
43+
cloud CI whose comment claimed it ran them. The #1994 cross-tenant RLS proof and
44+
the attachments cross-tenant isolation block had therefore never executed while
45+
the suite reported green (Prime Directive #10, test-suite edition). They now
46+
resolve like the runtime does, and `OS_TEST_MULTI_ORG_ENABLED=1` declares that a
47+
run is expected to ship the package — turning a silent skip into a loud failure,
48+
so a run can no longer pass by quietly not running the gates it exists for.

0 commit comments

Comments
 (0)