Skip to content

Commit 40fc905

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/discovery-cache-queue-route-conflict-8fwmfc
2 parents acddfb1 + cdf4d9a commit 40fc905

123 files changed

Lines changed: 6810 additions & 3585 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: 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: 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`.
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/lint": minor
4+
---
5+
6+
fix(spec,lint): withdraw the `record:*` blocks from the react tier — no renderer read the props it published (#4413)
7+
8+
The react-tier contract published `objectName` / `recordId` on
9+
`<RecordDetails>`, `<RecordHighlights>`, `<RecordRelatedList>` and
10+
`<RecordPath>`, and no renderer read either prop. All ten `record:*` renderers
11+
take their record from `useRecordContext()`, which only the record route
12+
(`RecordDetailView`) and the metadata editor's preview (`PagePreview`) ever
13+
mount; the `kind:'react'` page renderer wraps the page in a
14+
`SchemaRendererProvider` alone. So the blocks rendered their "bind a record to
15+
preview" placeholder — or, for `record:related_list` (the one that does read
16+
`schema.objectName`), refused to fetch because the parent id never arrived. A
17+
page authored exactly to contract came back EMPTY with nothing reported
18+
anywhere, including by `os validate`, which resolved those props' field names
19+
against the object they named: lint standing guard over a binding that never
20+
ran.
21+
22+
Withdrawn rather than implemented. The contract was not merely unimplemented,
23+
it was the wrong SHAPE: per-block bindings describe four independent fetches of
24+
one record, which is exactly the coupling the shared record context exists to
25+
prevent (`record:details` drops the fields a mounted `record:highlights`
26+
registered; one inline-edit save bar commits them all under a single
27+
`ifMatch`). Honoring the props would have fossilized that (Prime Directive
28+
#12). The naming of that primitive — a record SCOPE an author wraps around the
29+
family, one fetch, shared context — is the open design question, filed as #4444.
30+
31+
`@objectstack/spec` drops the four blocks from `REACT_BLOCKS` and gains the
32+
ledger for why, plus the working replacement per type. The family is derived
33+
from `ComponentPropsMap`, so a record component added later is gated the day it
34+
lands — including the six that were never in the contract but are just as
35+
reachable through the registry-built react scope.
36+
37+
`@objectstack/lint` gains `react-block-needs-record-context` (error), which
38+
rejects them on a react page by tag and through `<Block type="record:…">`
39+
alike, quoting the block that does work: `<ListView filters={['<lookup>', '=',
40+
parentId]}>` for a related list, `<ObjectForm mode="view" recordId={…}>` for a
41+
field panel. A locally-declared component of the same name shadows the injected
42+
scope and is left alone.

‎content/docs/automation/flows.mdx‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ const approvalFlow = {
3131
type: 'decision',
3232
label: 'Check Amount',
3333
config: {
34-
// decision expressions use {braces} around variables — see Expressions in flows
34+
// decision expressions are bare CEL, like every other condition — no braces
3535
conditions: [
36-
{ label: 'High Value', expression: '{order_amount} > 10000' },
37-
{ label: 'Standard', expression: '{order_amount} <= 10000' },
36+
{ label: 'High Value', expression: 'order_amount > 10000' },
37+
{ label: 'Standard', expression: 'order_amount <= 10000' },
3838
],
3939
},
4040
},
@@ -169,8 +169,8 @@ or missing-`required` violation (#4277). A node type that publishes no
169169
label: 'Check Status',
170170
config: {
171171
conditions: [
172-
{ label: 'Approved', expression: "{status} == 'approved'" },
173-
{ label: 'Rejected', expression: "{status} == 'rejected'" },
172+
{ label: 'Approved', expression: "status == 'approved'" },
173+
{ label: 'Rejected', expression: "status == 'rejected'" },
174174
],
175175
},
176176
}
@@ -972,30 +972,40 @@ failures so one broken flow does not abort startup.
972972

973973
## Expressions in flows
974974

975-
A flow mixes **three expression dialects**, and using the wrong one is the
976-
single most common way a flow silently misbehaves. Which dialect applies is
977-
decided by *where* the expression sits — not by what it looks like:
975+
A flow mixes **two expression dialects**, and the rule is short: **every
976+
condition is CEL; braces are for values.**
978977

979978
| Where | Dialect | Write it like | Bindings |
980979
|:---|:---|:---|:---|
981980
| Start-node `condition` | **CEL** (bare, no braces) | `record.amount > 500` | `record.*`, `previous.*`, bare field names, `vars.*` |
982981
| Edge `condition` | **CEL** (bare, no braces) | `record.status == 'open'` | same as above |
983-
| Decision-node `conditions[].expression` | **Template compare** (braces required) | `{order_amount} > 10000` | flow variables by name, in `{…}` |
982+
| Decision-node `conditions[].expression` | **CEL** (bare, no braces) | `order_amount > 10000` | same as above |
984983
| Field values in `create_record` / `update_record` | **Interpolation** (braces required) | `'Follow up on {record.name}'`, `'{TODAY() + 7}'` | `{var}`, `{var.path}`, `{$User.Id}`, `{$User.Email}`, `{NOW()}`, `{TODAY()}`, `{TODAY() + 90}` (whole days) |
985984

986985
<Callout type="warn">
987-
**The two failure modes to memorize:**
986+
**The failure modes to memorize:**
988987

989-
1. **Braces missing in a decision expression** — `'order_amount > 10000'` isn't
990-
evaluated as a variable at all. It compares the *string* `"order_amount"`
991-
against `"10000"`, which is **always true**, so the flow always takes the
992-
first branch and never tells you. Write `'{order_amount} > 10000'`.
993-
2. **Braces missing in a field value** — `due_date: 'TODAY() + 7'` writes the
988+
1. **Braces missing in a field value** — `due_date: 'TODAY() + 7'` writes the
994989
literal text `TODAY() + 7` into the field. Write `'{TODAY() + 7}'`.
990+
2. **Braces put *into* a condition** — `'{record.amount} > 500'`. Conditions
991+
fail loudly rather than silently, with an error that tells you to drop the
992+
braces.
993+
</Callout>
995994

996-
The mirror mistake is putting braces *into* a CEL condition
997-
(`'{record.amount} > 500'`) — CEL conditions fail loudly rather than silently,
998-
with an error that tells you to drop the braces.
995+
<Callout type="info">
996+
**Decision-node expressions used to be compared as text** (#4414, #4336), so
997+
`'order_amount > 10000'` compared the string `"order_amount"` against `"10000"`
998+
and was **always true**, while `'{lead_record.status} == "converted"'` was
999+
**always false** — the brace form substitutes a whole flow variable by name, and
1000+
a field access on an object variable is not one. Both reported `success`. They
1001+
are bare CEL now, so the spellings in the table above are the correct ones and
1002+
`lead_record.status == 'converted'` resolves the field.
1003+
1004+
The `{var}` form still works where it always did — `{amount} > 100`,
1005+
`{status} == 'active'` — but the two ways it used to answer `false` without
1006+
saying so are now **loud errors** naming the reference: a `{…}` hole that
1007+
matches no flow variable, and a substituted value that is neither a boolean, a
1008+
number, nor part of a comparison (#4336).
9991009
</Callout>
10001010

10011011
CEL conditions that fail to evaluate raise an error and stop the run — they

0 commit comments

Comments
 (0)