Skip to content

Commit 0d822c3

Browse files
committed
Merge origin/main into the #4439 branch
#4453 landed the same class of fix from the other end: `evaluateCondition` now sniffs the dialect from the source instead of from the caller, so a bare string is CEL unless it holds a `{var}` hole. Three overlaps to reconcile: - The flows guide's "Expressions in flows" section was rewritten on both sides. Kept main's — it is more complete about what the `{var}` dialect now does — and folded in the one fact it cannot know: a decision's `conditions[].expression` is on the expression ledger as a predicate, so a braced spelling there is a build failure, not the `{var}` dialect. Main's "the `{var}` form still works" is true of a start node's plain-string condition and would have been wrong for a decision. - `FlowNodeSchema`'s `@example` likewise. Kept both branches from main and dropped its `// default` annotation on the `true` catch-all: since #4414 the default path is the `isDefault` out-edge, not a catch-all branch. - The decision executor's explicit `dialect: 'cel'` envelope is now redundant for the case it was added for, but it is what keeps run time agreeing with the validators: the sniff would route a braced predicate to the template dialect and run it, while `registerFlow` / `objectstack validate` reject exactly that spelling. Comment rewritten to say so — the old one described a path that no longer exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q8as8yR67v41xEdomiTba9
2 parents e12c2e1 + 83cf2d3 commit 0d822c3

167 files changed

Lines changed: 9815 additions & 3721 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: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): allow the aggregate bulk dispatch key `_selectedIds` through the action param gate (objectui#3139)
6+
7+
A list view's `bulkActionDefs` entry can now opt into an aggregate single-call
8+
dispatch (`execution: 'aggregate'`, objectui 17.1): the renderer invokes the
9+
named object action ONCE for the whole selection, injecting every selected
10+
record id as `params._selectedIds: string[]`, so a single call can produce one
11+
aggregate artifact (zip of QR codes, merged PDF, batch print job).
12+
13+
`ACTION_PARAM_BUILTIN_KEYS` gains `'_selectedIds'` so the ADR-0104 strict
14+
param gate does not 400 an aggregate dispatch against an action that declares
15+
params — like `recordId`/`objectName`, the key is dispatcher-injected and can
16+
never be authored as a declared param. Pure widening: actions declaring no
17+
params were never validated, and no authored bag legitimately carried this
18+
key. The `bulkActionDefs` describe now documents the aggregate contract
19+
(server reads `params._selectedIds`, results are all-or-nothing, `batchSize`
20+
does not apply, set `maxRecords` for expensive aggregates, and toolbar
21+
url/api actions can interpolate `${ctx.selection.ids}`).
22+
23+
The showcase's Task → Bulk Actions view carries the specimen:
24+
`showcase_recalc_selection` dispatches the recalc endpoint once for the whole
25+
selection via the endpoint's new `_selectedIds` batch branch, next to the
26+
per-record fan-out fixtures.
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
fix(cli): every author-time rule that can gate runs on all three commands (#4409)
6+
7+
`os validate`, `os build` and `os lint` each hand-wired their own subset of the
8+
author-time rules. Nothing connected the three lists, so "which rules run here?"
9+
was answerable only by diffing three 800-line files by eye — and the answer
10+
drifted every time a rule landed. The audit found 23 of 26 rules running on some
11+
strict subset, nine of them able to emit `error`.
12+
13+
The worst direction was the least obvious. `os build` — the command that
14+
PUBLISHES — was the weakest gate of the three: a flow whose expression approver
15+
does not parse (`approval-expression-invalid`) built and published green, and
16+
only `os lint` stopped it, while CI usually runs the other two. `os lint`
17+
disagreed in *both* directions at once, running one gating rule neither other
18+
command ran and missing six that both of them ran, which is worse than no
19+
pre-flight — the remaining options are re-verifying everything or learning to
20+
distrust the signal.
21+
22+
This is the same failure mode's fifth appearance (#3583, #3782, #4384/#4394,
23+
#4402). Each earlier repair removed an instance and left the MODE: a rule's
24+
command coverage was whatever its author remembered to type, and forgetting was
25+
silent. #4402's guard could not catch the rest — it filtered on the current
26+
member names of one suite, so a rule hand-wired into two commands from outside
27+
that suite passed it without a word. A name list only guards the names on it.
28+
29+
**The registry.** `AUTHORING_RULES` declares all 26 rules as data: tier
30+
(`gating`/`advisory`), which stack tier they read (pre-parse `normalized` vs
31+
`parsed`), which commands run them, and a written reason for the one narrowing.
32+
All three commands consume it through `runAuthoringRules()`, so adding a rule is
33+
a one-line edit that reaches every command at once. The three command files
34+
shrink by ~1000 lines between them.
35+
36+
**The ratchet.** The wiring guard is no longer a name list: a `gating` rule on
37+
fewer than three commands fails, a narrowed rule with no reason fails, a command
38+
that calls or imports a registry rule directly fails, and an `advisory` claim is
39+
checked against the rule's own source — so a gate cannot wear an advisory label
40+
to buy itself partial coverage. That last check is the one #3760 needed, having
41+
promoted a `lintFlowPatterns` rule from advisory to gating with nothing anywhere
42+
asking whether its coverage should follow. Remaining direct calls are listed
43+
with reasons, and a stale entry fails too, so the ratchet cannot rot into a
44+
permanent permission slip.
45+
46+
**The verdict, not just the wiring.** A separate test plants one defect per
47+
previously-blind gating rule and asserts all three commands gate on it, plus the
48+
issue's own repro driven end-to-end through the real CLI: exit 1 on all three
49+
where it was 1/0/0.
50+
51+
Two behaviour changes fall out of reporting every failing rule in one run
52+
instead of exiting at the first failing gate: an author with three unrelated
53+
problems now sees all three in one pass, and `--strict` covers every advisory
54+
rather than the roughly half that happened to be printed inline.
55+
56+
Also closes the same hole one gate over: `collectAndLintDocs` failed `os build`
57+
and never ran on `os validate`, invisible because the parity guard keyed on the
58+
`lint*`/`validate*` naming convention and that gate is called `collect*`. The
59+
guard now names each shared non-registry gate explicitly instead of
60+
pattern-matching for them.
61+
62+
Cost is not what argued against any of this. The heavy dependencies
63+
(`typescript` ~9 MB, `sucrase`) are already lazy and load only when a stack
64+
carries the metadata that needs them, and the heaviest rule of the set has run
65+
on all three commands as a reference-integrity suite member since #4340 without
66+
anyone noticing. The one narrowed rule, `lintUniqueDeclarations`, is scoped
67+
because `os lint` already reports it through `lintDataModel` — coverage
68+
recorded, not coverage missing.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
'@objectstack/client': patch
3+
---
4+
5+
docs(client): drop the retired `validateOnly` batch option from the README (#4052)
6+
7+
The Batch Options section still documented `validateOnly` as a working dry-run —
8+
"validate records without persisting changes" — but the key was retired in #4052
9+
precisely because nothing ever read it. Every batch surface (`updateManyData` /
10+
`deleteManyData` / `batchData`) persisted regardless, so a caller who sent it to
11+
preview a mutation got that mutation **executed**.
12+
13+
`BatchOptionsSchema` has carried a `retiredKey(...)` tombstone since #4052, so the
14+
schema already refuses the key loudly. The README was the last place still
15+
promising it — declared-but-not-enforced in prose rather than in code, aimed at
16+
exactly the readers who cannot see the tombstone.
17+
18+
Released as a patch rather than declared release-nothing because `README.md` is in
19+
this package's `files`: the corrected text only reaches the people who hit the
20+
problem — readers on npmjs.com — if the package ships.
21+
22+
Replaced with a pointer to `docs/protocol-upgrade-guide.md`
23+
(`batch-options-validate-only-retired`). No behaviour change; there is no batch
24+
dry-run today. Write-path validate-only was evaluated in #4372 and closed as not
25+
planned — no current consumer justifies the surface.
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/service-datasource': minor
4+
---
5+
6+
`datasource.config` is now validated against its driver's contract (#4410)
7+
8+
`config` was the one authorable slot on a datasource with no gate at all. The
9+
schema's own comment claimed "the driver's own `configSchema` is what validates
10+
it" — nothing did: both bundled driver specs set `configSchema: {}`, no code read
11+
the field, and the per-driver zod schemas were not even exported from the
12+
package. So `config: { hostname: 'db.internal' }` (the key is `host`) was
13+
accepted in silence and the datasource connected to `localhost` while the parse,
14+
the save and the connection probe all reported success.
15+
16+
`DatasourceSchema` now parses `config` — and each `readReplicas` entry — against
17+
the contract for the declared driver, and `DatasourceAdminService`
18+
(create/update/test, the Setup wizard's path) applies the same check. Both read
19+
one registry in `@objectstack/spec/data`, which also projects each contract to
20+
JSON Schema for `DriverDefinitionSchema.configSchema` and the Studio connection
21+
form, so the form offers exactly the fields the validator accepts.
22+
23+
New exports from `@objectstack/spec/data`: `PostgresConfigSchema`,
24+
`MysqlConfigSchema`, `SqliteConfigSchema`, `SqliteWasmConfigSchema`,
25+
`MongoConfigSchema`, `MemoryConfigSchema`, plus `resolveDriverId`,
26+
`getDriverConfigSchema`, `getDriverConfigJsonSchemaById` and
27+
`validateDriverConfig`. A driver the platform ships no contract for (a plugin's
28+
`com.vendor.snowflake`) keeps an unvalidated `config`.
29+
30+
**Migration.** A config that was silently ignored now fails with the correction
31+
in the message. The renames:
32+
33+
| Wrote | Write instead | Driver |
34+
| --- | --- | --- |
35+
| `user` | `username` | postgres, mysql, mongo |
36+
| `connectionString` / `dsn` | `url` | postgres, mysql, mongo |
37+
| `uri` | `url` | mongo |
38+
| `file` / `path` / `database` | `filename` | sqlite, sqlite-wasm |
39+
| `hostname` | `host` | postgres, mysql, mongo |
40+
| `searchPath` | `schema` | postgres |
41+
42+
And the relocations — keys that were never driver config:
43+
44+
| Wrote in `config` | Write instead |
45+
| --- | --- |
46+
| `min` / `max` / `idleTimeoutMillis` / `connectionTimeoutMillis` | the datasource's own `pool` block |
47+
| `schemaMode` | next to `driver`, on the datasource |
48+
| `readOnly` | `capabilities: { readOnly: true }` |
49+
| `ssl: { ca, cert, key, rejectUnauthorized }` | the datasource's own `ssl` block — inside `config`, `ssl` is the on/off boolean shorthand |
50+
51+
Two memory-driver keys are **removed**: `indexes` and `maxRecordsPerObject`.
52+
`InMemoryDriverConfig` has no field for either — the driver keeps no indexes and
53+
evicts nothing — so both were inert. Drop them; for real indexing use a driver
54+
that indexes.
55+
56+
A postgres, mysql or mongo datasource must now name a connection target
57+
(`database`, or a `url` that carries it). An empty `config` used to mean "the
58+
client's own localhost default", which is the same defect in its most complete
59+
form.
60+
61+
**Also fixed, because the contract can only be enforced where it is honoured.**
62+
These keys were declared and read by nothing; they now reach the driver:
63+
64+
- `datasource.pool` is honoured by every SQL driver (it was declared, carried
65+
into the connection spec, then overwritten with a hardcoded `{ min: 0, max: 5 }`),
66+
and maps onto the Mongo client's `minPoolSize` / `maxPoolSize`.
67+
- `datasource.schemaMode` reaches the driver. It was dropped between the
68+
datasource record and the connection spec, so a `schemaMode: 'external'`
69+
database — one ObjectStack must never run DDL against — was constructed as
70+
`managed`.
71+
- `datasource.ssl` reaches the SQL clients, certificates and all. It stopped at
72+
the record — nothing put it on the connection spec — so a TLS block configured
73+
nothing, which is exactly what its own schema comment warns about ("a TLS
74+
setting that never took effect looked identical to one that did").
75+
- postgres `schema` (knex `searchPath`), `applicationName` and `statementTimeout`.
76+
- mongo `password`, `authSource` and `options`. A mongo datasource carrying a
77+
`config.password` previously composed its URL with an **empty** password.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/metadata-protocol": patch
4+
"@objectstack/runtime": patch
5+
---
6+
7+
fix(spec,metadata-protocol,runtime): discovery stops advertising routes for the kernel-internal cache/queue/job slots (#4318)
8+
9+
The metadata-protocol discovery builder declared `/api/v1/cache`, `/api/v1/queue`
10+
and `/api/v1/jobs` — three paths that existed nowhere else in the repository: no
11+
dispatcher domain, no adapter mount, no plugin registration, and the shipped
12+
providers (`service-cache`/`-queue`/`-job`) are in-process contracts that will
13+
never mount one. Every default boot therefore advertised a route inside the same
14+
`ServiceInfo` whose `handlerReady: false` said the opposite — a single record
15+
contradicting itself (ADR-0076 D12).
16+
17+
These slots are route-less now, like `realtime` — but unlike `realtime` an
18+
unmarked real implementation stays `available`: the slot's contract is
19+
in-process, so "no HTTP surface" is not reduced capability for it. `handlerReady`
20+
is reported `false` on both discovery builders — for a route-less slot it is not
21+
a proxy for anything, it is the fact itself (the dispatcher used to claim
22+
`handlerReady: true` here for an unmarked occupant, a handler that does not
23+
exist). The explanatory message is written once, as
24+
`inProcessServiceMessage(slot)` in `@objectstack/spec/system`, so the two
25+
builders cannot drift apart.
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
"@objectstack/objectql": major
3+
"@objectstack/spec": patch
4+
"@objectstack/driver-mongodb": patch
5+
"@objectstack/driver-sql": patch
6+
---
7+
8+
fix(objectql,driver-mongodb)!: `findOne` must say which record it wants, and executes every option it declares (#4419)
9+
10+
`findOne` reads a single row, which makes its predicate the only thing between
11+
the caller and *an arbitrary record*. When the predicate is missing the result is
12+
not `null` — it is the object's **first row**: a real, plausible-looking record
13+
with nothing to do with the request, which the `if (!row)` check every call site
14+
already has cannot catch, and which then propagates into whatever is computed
15+
next. Reported downstream: line items defaulting their price from the first
16+
product in the catalog rather than the selected one, and "is this deal already
17+
closed?" answered against an unrelated record while the write that followed
18+
correctly targeted the intended id. A throw would have been caught in
19+
development; a `null` would have been caught by the null-check. A valid-looking
20+
wrong record defeats both.
21+
22+
**Breaking — `findOne` now refuses a query that selects nothing in particular.**
23+
24+
FROM → TO:
25+
26+
| Was | Now write | Meaning |
27+
|---|---|---|
28+
| `findOne(o)`, `findOne(o, {})`, `findOne(o, { where: {} })` | `findOne(o, { where: … })` | the record matching this predicate |
29+
| | `findOne(o, { search: 'Acme' })` | the record this search finds |
30+
| | `findOne(o, { orderBy: [{ field: 'created_at', order: 'desc' }] })` | the FIRST record in this order — the newest |
31+
| | `find(o, { limit: 1 })` | any row will genuinely do, said at the call site |
32+
33+
One-line fix: add the `where` you meant, or `orderBy` if you meant "the newest
34+
one", or switch to `find(o, { limit: 1 })` if any row will do. The error names
35+
all four. `find` and `count` are unchanged — returning or counting every row is
36+
an honest answer; only `findOne`'s implicit "just one of them" turns a missing
37+
predicate into a confidently wrong record. The guard reads the CALLER's
38+
predicate, before RLS/sharing middleware injects its own: a tenant filter
39+
narrows which rows are visible, it does not make "whichever comes first"
40+
something the caller asked for.
41+
42+
**Two silent drops that produced the same wrong record are fixed with it.**
43+
44+
- **`findOne({ search })` applies the search.** The ADR-0061 `search`
45+
cross-field `$contains` expansion lived inline in `find` and nowhere else,
46+
while `find` and `findOne` are checked against the SAME legal-key set — so
47+
`search` passed the gate, rode onto the AST, and reached a driver. No driver
48+
reads `ast.search`. The read therefore ran with no predicate at all and
49+
`limit: 1` did the rest. The expansion is now one method both call.
50+
- **`MongoDBDriver.findOne` applies `orderBy`, `fields` and `offset`.** It
51+
translated `query.where` and dropped the rest, so `findOne({ orderBy })` did
52+
not return the newest record — it returned whichever document the scan reached
53+
first. `find` and `_findStream` in the same driver had always handled all
54+
three. This one matters beyond Mongo: the guard above tells an unpredicated
55+
caller to reach for `orderBy`, and an escape hatch one backend ignores is not
56+
an escape hatch. No ordering is IMPOSED when the caller supplies none — both
57+
drivers keep that carve-out (#4363), and `SqlDriver`'s comment about Mongo
58+
"never sorting" is corrected, since it cited the dropped parameter as
59+
agreement.
60+
61+
**And a gate so the class does not come back.** A drift pin walks
62+
`ENGINE_OPTION_KEY_SETS.findOne` and requires each declared key to have an
63+
observable effect — on the AST the driver receives, on the driver options, or in
64+
an explicit "not executed, and here is why" entry (only `limit`, which the
65+
contract's `limit: 1` overrides). `search` sat declared-but-unexecuted through
66+
two rounds of hardening because nothing asked that question.
67+
68+
Together with #4346 (`filter``where` folds on every entry point) and #4400
69+
(unknown option keys throw), a read parameter the engine does not execute now
70+
fails at the call site instead of quietly changing the answer.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/service-automation": minor
3+
---
4+
5+
fix(automation): `evaluateCondition` decides the dialect from the source, not from the caller (#4336)
6+
7+
`AutomationEngine.evaluateCondition` picked its engine by asking whether an
8+
`{ dialect, source }` **envelope** was present. A condition handed to it as a
9+
plain string therefore never reached the CEL engine: it fell through to the
10+
legacy `{var}` template path, which substitutes brace holes and then compares
11+
whatever text is left — **as text**. Nothing errored, and the run was recorded
12+
as `success`, with the failure direction depending on the predicate:
13+
14+
| Handed in | Actually evaluated | Result |
15+
|:---|:---|:---|
16+
| `existingTask == null` | `'existingTask' === 'null'` | always **false** — gate never opens |
17+
| `record.rating >= 4` | `'record.rating' >= '4'``'r' > '4'` | always **true** — branch pinned open |
18+
19+
#4414 fixed the one built-in that was reaching this — the `decision` executor
20+
now wraps `conditions[].expression` in a CEL envelope before calling. This
21+
fixes the **evaluator**, so the next caller does not have to remember: the
22+
dialect is now read from the source, and a condition is CEL unless it actually
23+
contains a `{var}` hole. `evaluateCondition` is public API, so a
24+
plugin-registered node executor evaluating its own predicate was getting the
25+
table above with nothing to warn it.
26+
27+
**The legacy `{var}` dialect keeps working** where it always did —
28+
`{amount} > 100`, `{status} == active`, `{a.b} == 7` — and gains the two things
29+
it was missing:
30+
31+
- **A quoted literal compares as its contents.** `{status} == 'active'` used to
32+
compare `active` against `'active'` — quotes included — and was false for
33+
every value of `status`. It is the spelling the flow docs showed, and quoting
34+
a string literal is what every other predicate surface requires.
35+
- **It no longer answers `false` when it could not resolve something.** A `{…}`
36+
hole matching no flow variable (`{lead_record.status}``get_record` stores
37+
the whole row under one name, so that key never exists) and a substituted
38+
value that is neither a boolean, a number, nor part of a comparison are
39+
refused with the source and the offending reference attached. Both used to be
40+
a silent `false`, which ADR-0032 §1c forbids: a predicate that cannot be
41+
evaluated is a fault, never a quiet branch decision.
42+
43+
Braces inside an explicit `dialect: 'cel'` envelope remain the #1491 brace-trap
44+
and still throw — stating the dialect is the author saying "this is CEL". The
45+
sniff reads the source outside string literals, so `record.label == '{pending}'`
46+
stays CEL and compares the field.
47+
48+
**Tightening to know about:** a bare string that is not valid CEL now raises
49+
where it previously string-compared to some answer. That includes the
50+
host-language payloads the safety tests use (`process.exit(1)`,
51+
`require("fs")…`) — nothing executed before and nothing executes now, since CEL
52+
has no `process`, no `require` and no arrow functions, but the failure is a
53+
reported fault instead of a silent `false`.

0 commit comments

Comments
 (0)