Skip to content

Commit 86d90db

Browse files
claudeclaude
committed
Merge remote-tracking branch 'origin/main' into claude/issue-16356-showcase-loop-parallel
2 parents ca9db83 + 10da5c4 commit 86d90db

18 files changed

Lines changed: 1794 additions & 48 deletions

‎.changeset/analytics-sqldialect-declared-vocabulary.md‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
fix(analytics)!: `AnalyticsServiceConfig.sqlDialect` declares its three-name accept set, and a host that answers outside it is told once (#16206)
6+
7+
<!-- adr-0087: not-required (runtime-interface-only packages/services/service-analytics/src/analytics-service.ts#AnalyticsServiceConfig) The narrowed member is one hook on a service CONSTRUCTOR CONFIG — a published runtime TypeScript interface with no metadata surface. It has no Zod schema, no `packages/spec` declaration and no stored representation, so `objectstack migrate meta`, `spec-changes.json` and the generated upgrade guide have nothing to rewrite; the affected party is a TypeScript host and the channel that reaches every one of them is the compiler at their own composition site. No metadata key is added, removed, renamed or re-shaped, and `packages/spec` is untouched by this diff. -->
8+
9+
**BREAKING** for a TypeScript host that declares its `sqlDialect` hook as returning
10+
`string`: the hook's declared return is now the three canonical dialect names or
11+
`undefined`, so such a composition stops compiling until the host's own annotation
12+
says which names it can answer. Shipped as `minor` under the repo's launch-window
13+
convention, in which breaking-ness is carried by this banner and the disposition
14+
above rather than by the bump level. Runtime behaviour for every host is unchanged:
15+
the same three names were the only ones that ever did anything.
16+
17+
## What was wrong
18+
19+
`AnalyticsServiceConfig.sqlDialect` — the hook a host answers to say which SQL
20+
dialect backs an object — was typed as free `string`, while `normalizeSqlDialect`
21+
has only ever recognised `sqlite`, `postgres` and `mysql`. Nothing said so, and
22+
nothing told a host that answered otherwise.
23+
24+
So a host that owns a SQLite datasource and answers the spelling its own stack uses
25+
— knex's canonical `sqlite3`, or `better-sqlite3`, both of which `driver-sql` itself
26+
lists in `SQLITE_EMIT_CLIENTS` — was read as `unknown`. And because `sqlDialectFor`
27+
is tiered "cannot answer, do not block", **a wrong answer and no answer were the
28+
same answer**: the host that tried hardest to help got the residue arm, silently.
29+
30+
## What it does now
31+
32+
- **The vocabulary is declared**, on the type and in the docblock, as
33+
`AcceptedSqlDialect` — `sqlite` | `postgres` | `mysql` — so a host reading the
34+
config learns the accept set without running anything. The type and the runtime
35+
membership set are generated from one `const` tuple, so a future widening cannot
36+
land in one and miss the other.
37+
- **A non-empty answer outside the set is diagnosed**: one `warn` naming the object,
38+
the answer and the accepted set. It is emitted **once per distinct unrecognised
39+
spelling** — the failure's identity — so the line count is bounded by the host's
40+
own hook and never grows with query volume.
41+
- **`undefined` stays silent and legal.** The hook is optional and "cannot answer,
42+
do not block" is a supported composition, not a misconfiguration. A pin holds both
43+
halves, because a diagnostic that also shouted at hosts who wired nothing would be
44+
a worse defect than the one being fixed.
45+
- **The accept set is NOT widened.** Teaching this package `driver-sql`'s knex
46+
aliases would be a second copy of that driver's table, and an unrecognised
47+
spelling is sometimes deliberate (`mariadb`, #11756). The answer is still read as
48+
`unknown`; only the silence changed.
49+
- **The plugin bridge translates the driver's own residue.** `SqlDriver.dialectName`
50+
carries a fourth name, `unknown`, meaning "I cannot say"; handed on verbatim it
51+
would have presented a correctly-behaving driver as a host answering out of
52+
contract. It now arrives as `undefined`, this hook's own spelling for the same
53+
thing. The dialect the compilers end up with is unchanged either way.
54+
55+
## Measured, and worth reading before relying on the residue arm
56+
57+
Driven on sql.js through a host answering `sqlite3`, against the shared
58+
`FILTER_TEXT_CASES` fixture, with a host answering `sqlite` as the control: **five of
59+
the six case-EXACT cases come back with the wrong rows** — every case that
60+
discriminates on ASCII case. `{ name: { $contains: 'acme' } }` answers `['1','2']`
61+
where the table says `['2']`, and the negated form DROPS a row that belongs in the
62+
result. That is #15684's fold, live on the arm this population lands on, and it is
63+
reported rather than fixed here: closing it is that card's business, not this one's.

‎.changeset/blank-node-condition-refused-at-registration.md‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
---
2+
"@objectstack/service-automation": minor
3+
---
4+
5+
fix(service-automation)!: a whitespace-only `config.condition` is refused at `registerFlow`, the rule the edge door has carried since #15807 (#17322)
6+
7+
<!-- adr-0087: not-required (already-registered flow-edge-condition-evaluated-slot-source-required) this is a second face of the decision that entry already carries — an evaluated slot requires a non-blank `source`, refused with EVALUATED_EXPRESSION_SOURCE_REQUIRED — applied to the other structural condition slot by importing the same schema rather than by deriving a second rule; no key is renamed, retired or given a new meaning here. ⚠️ That entry's `surface` and `acceptanceCriteria` name only `edges[].condition`, so they need widening to `config.condition` for a consumer replaying the chain; that file is in packages/spec, outside this card's package, and is filed as a follow-up rather than edited here. -->
8+
9+
**BREAKING** in the accept-set sense, landing in the launch window as `minor`
10+
(the lockstep convention: `major` is refused by `check-changeset-no-major`, and
11+
breaking-ness is carried by this banner plus the ADR-0087 disposition): a flow
12+
node's `config.condition` — a `decision` node's predicate, and on a `start` node
13+
the **trigger gate** — is now refused at `registerFlow` when its source is blank
14+
after trimming, where it used to register clean and answer a **silent `false`**
15+
at every evaluation.
16+
17+
Two doors, the same authored value, two fates until now. `FlowEdgeSchema.condition`
18+
composes `EvaluatedExpressionInputSchema` (#15807), so `' '` on an edge is
19+
refused at `FlowSchema.parse`, by name. A node's `config` is an open
20+
`z.record(z.string(), z.unknown())`, so the same value passed through verbatim,
21+
reached `AutomationEngine.evaluateCondition`'s empty-source arm — `exprStr.trim()
22+
=== ''` — and returned `false`, under a comment that names that arm as being for
23+
an **unauthored** condition. `' '` was authored. The branch never ran, forever,
24+
with nothing said at any layer.
25+
26+
```yaml
27+
nodes:
28+
- { id: gate, type: start, config: { objectName: lead, triggerType: record-after-update, condition: ' ' } } # the flow was gated shut
29+
- { id: branch, type: decision, config: { condition: { dialect: cel, source: ' ' } } } # the same blank, through the envelope key
30+
```
31+
32+
> An expression in an evaluated slot needs a non-blank `source`: the expression
33+
> engine evaluates `source` (the canonical persisted form of phase M9.1) and
34+
> cannot evaluate `ast` alone, so an envelope carrying only `ast`, or a `source`
35+
> that is blank after trimming, would validate and register and then fault at
36+
> run time. Write `{ dialect: 'cel', source: '…' }`.
37+
38+
- **The rule is imported, not re-derived.** `registerFlow`'s structural pass runs
39+
the condition's source through `EvaluatedExpressionInputSchema` itself, so the
40+
node door and the edge door cannot drift into two notions of "blank" or two
41+
sentences for it — the property the #15662 campaign built the shared refusal
42+
for. Nothing is exported from this package to carry it, and no new export was
43+
added.
44+
- **Applied to the SOURCE, not to the whole value**, deliberately: the union
45+
would also refuse an envelope with no `dialect` or with a dialect outside its
46+
enum, and this slot admits both (`structuralConditionRefusal`'s docblock,
47+
#4336). The narrowing is exactly the blank population and nothing else — a
48+
`cron` envelope with a real source still earns its own pre-existing verdict,
49+
and a bare string with a `{…}` brace trap still earns #1491's.
50+
- **`evaluateCondition` is unchanged and still answers `false`.** It is the
51+
shared evaluator and a public method on an exported class, so its throw
52+
behaviour is itself a contract; and a stored flow reaches it whatever the
53+
producer refuses. This change is at the producer only.
54+
- **`structuralConditionRefusal` is unchanged.** A string is still a well-shaped
55+
condition; the new refusal sits behind the shape one and in front of the CEL
56+
one, and answers the evaluated-slot sentence rather than
57+
`STRUCTURAL_CONDITION_SHAPE_REFUSAL`.
58+
59+
**What an author does with a refused condition.** A whitespace-only condition was
60+
never a predicate — the engine answered `false`, so the branch never fired, and on
61+
a `start` node the flow never triggered. **Remove the `condition` key** if the node
62+
was meant to be unconditional, or **write the expression** if it was meant to
63+
branch. ⚠️ Those two are not interchangeable: a refused condition never fired,
64+
while an absent `condition` on a decision node is an unconditional branch that
65+
always fires and an absent one on a start node is a gate that always opens.
66+
Deleting the key to clear the refusal inverts the node rather than preserving it.
67+
Every condition with a non-blank source is unchanged, and nothing is renamed or
68+
retired.
69+
70+
**A flow ALREADY STORED in `sys_metadata` stops running entirely — the whole flow,
71+
not just the branch.** Stored flows are deliberately not canonicalized by
72+
`applyConversionsToStoredItem` (`spec/src/conversions/stored.ts`, and the same
73+
skip in `metadata/src/loaders/database-loader.ts`'s `rowToData`); they canonicalize
74+
at `registerFlow`, and each of the three boot paths in
75+
`service-automation/src/plugin.ts` wraps that call in `try`/`catch`, logs one
76+
`warn` naming the flow, and continues. So a node condition that used to answer a
77+
silent `false` while the rest of the flow ran now takes the flow down with it: it
78+
is never registered, its trigger is never armed, and the announcement is that one
79+
warn line — `[Automation] failed to register flow` at boot, `[Automation]
80+
cold-boot flow bind: failed to register flow` at the kernel:ready bind,
81+
`[Automation] flow re-sync: failed to register flow` on a re-sync. The warn line
82+
is also the locator: the refusal names the node and the slot, e.g. `node 'gate'
83+
(start) condition`. A stack authored in config files has a second door,
84+
`objectstack validate` — see the note below for what that door does **not** yet
85+
say.
86+
87+
**A repo-wide census on this branch found zero authored `config.condition` values
88+
of this shape**, against a lit control: a textual probe over all 8,123 tracked
89+
source files found **461** non-blank `condition:` string literals and **zero**
90+
blank-after-trim ones in any authored flow (the four blank hits are two prose
91+
examples inside #15807's own changeset and two `packages/lint` test fixtures).
92+
There is nothing in this repository to rewrite.
93+
94+
⚠️ **Two follow-ups this change does not carry, both outside this card's package.**
95+
(1) The ADR-0087 D3 entry named above,
96+
`flow-edge-condition-evaluated-slot-source-required`, registers the decision this
97+
change is a second face of — an evaluated slot requires a non-blank `source` — but
98+
its `surface` and `acceptanceCriteria` name only `edges[].condition`. They need
99+
widening to `config.condition` so a consumer replaying the chain is told to sweep
100+
the node key too; that file is in `packages/spec`.
101+
(2) `@objectstack/lint`'s `validate-expressions` applies only
102+
`structuralConditionRefusal` to a structural condition, so `objectstack validate`
103+
still reports nothing for a blank `config.condition` that `registerFlow` now
104+
refuses — the two doors disagree until that rule is rebound as well.

‎.changeset/plugin-version-honest-grammar-claim.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
`PluginSchema.version` now describes the grammar it actually enforces instead of calling itself `"Semantic Version"`.
6+
7+
The key's regex accepts **every** SemVer 2.0.0-valid string and, additionally, eight strings SemVer 2.0.0 forbids:
8+
9+
| SemVer 2.0.0 rule | Strings this key accepts anyway |
10+
|---|---|
11+
| §2 — numeric identifiers MUST NOT include leading zeroes | `01.1.1`, `1.01.1`, `1.1.01` |
12+
| §9 — prerelease identifiers MUST NOT be empty or carry leading zeroes | `1.0.0-0123`, `1.0.0-alpha..1`, `1.0.0-alpha..`, `1.0.0-.` |
13+
| §10 — build-metadata identifiers MUST NOT be empty | `1.0.0+.` |
14+
15+
**No accepted value moved, in either direction.** The regex is byte-for-byte what it was; the `describe()` string is what changed. The leading-zero half is older than the recent widening — the original `/^\d+\.\d+\.\d+$/` admitted `01.1.1` too, because `\d+` always has — so tightening the key to the official SemVer regex would refuse plugin objects that load today, which the ruling on this key forbids. With the accept set frozen, the only side of the declared/enforced pair still free to move is the claim, and the bare `"Semantic Version"` was the false half: it named a standard this key does not implement.
16+
17+
The replacement states the shape an author can predict a verdict from — `major.minor.patch` with an optional `-prerelease` and an optional `+build` suffix — and disclaims the standard it exceeds rather than merely dropping the word. This follows `ManifestSchema.version`, which already spells `(major.minor.patch)` explicitly rather than leaning on "SemVer".
18+
19+
**What consumers see.** The `description` on `version` in the shipped `json-schema/` tree and on the generated `kernel/plugin` reference page. No `pattern`, no `type`, no accepted or rejected value changes, so a tool that validates against this schema behaves identically.
20+
21+
All eight forms are now pinned as **accepted** — in `packages/spec` (`plugin.test.ts`) and in `packages/core` (`plugin-loader.test.ts`, `plugin-contract-enforcement.test.ts`) — so the honesty is enforced rather than narrated, and a future edit that "corrects" the grammar to be standards-compliant fails those pins on purpose.
22+
23+
`@objectstack/core` is deliberately **not** listed above. Its `PluginLoader` predicate was renamed `isValidSemanticVersion` to `isSemverShapedVersion` in the same change, for the same reason, but the symbol is `private` and package-internal: measured against the built `dist/index.d.ts`, `import { isValidSemanticVersion } from '@objectstack/core'` is TS2305 (no exported member) and `loader.isValidSemanticVersion` is TS2341 (private), while a public member on the same class compiles. Nothing published moves.

‎content/docs/references/kernel/plugin.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const result = PluginSchema.parse(data);
3131
| **staticPath** | `string` | optional | Absolute path to static assets (Required for type="ui") |
3232
| **slug** | `string` | optional | URL path segment (Required for type="ui") |
3333
| **default** | `boolean` | optional | Serve at root path (Only one "ui" plugin can be default) |
34-
| **version** | `string` | optional | Semantic Version |
34+
| **version** | `string` | optional | Version: major.minor.patch, with an optional -prerelease and an optional +build suffix. Looser than SemVer 2.0.0 — leading zeroes (01.1.1) and empty identifiers (1.0.0-alpha..1) are accepted. |
3535
| **description** | `string` | optional | |
3636
| **author** | `string` | optional | |
3737
| **homepage** | `string` | optional | |

‎packages/core/src/plugin-contract-enforcement.test.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ describe('E — `version` is the NINTH enforced key, and admitting it refused no
348348
* `version` used to be filtered out of this check. It was, because the two
349349
* declarations disagreed: `PluginSchema.version` was `/^\d+\.\d+\.\d+$/`
350350
* and refused the prerelease and build-metadata forms SemVer 2.0.0 defines,
351-
* while `PluginLoader.isValidSemanticVersion` — the check the boot path has
351+
* while `PluginLoader.isSemverShapedVersion` — the check the boot path has
352352
* always run — accepted them, deliberately, pinned by `plugin-loader.test.ts`.
353353
*
354354
* #16365 settled that in `packages/spec` by WIDENING the schema onto the
@@ -376,6 +376,39 @@ describe('E — `version` is the NINTH enforced key, and admitting it refused no
376376
}
377377
});
378378

379+
/**
380+
* #17070 — the two declarations still share ONE grammar, measured over the
381+
* eight strings SemVer 2.0.0 forbids and both of them accept.
382+
*
383+
* ⭐ This is the convergence assertion for the pair, and it is the reason
384+
* #17070 could repair the CLAIM on both sides from a single card: schema and
385+
* loader are one accept set with two names on it. If a future edit moves one
386+
* spelling and not the other, this fails — and both docblocks that promise
387+
* "character for character" become false at the same moment.
388+
*
389+
* ⛔ The direction here is deliberate and frozen. #16365 ruled widen-never-
390+
* narrow, so these eight are pinned as ACCEPTED, not as a defect awaiting
391+
* cleanup; `01.1.1` loaded before either card existed. What #17070 changed
392+
* is the description on the spec key and the name of the loader's predicate
393+
* (`isSemverShapedVersion`), so that the accept set and the claim about it
394+
* finally agree.
395+
*/
396+
const SEMVER_FORBIDS = [
397+
'01.1.1', '1.01.1', '1.1.01', // §2
398+
'1.0.0-0123', '1.0.0-alpha..1', '1.0.0-alpha..', '1.0.0-.', // §9
399+
'1.0.0+.', // §10
400+
];
401+
402+
it.each(SEMVER_FORBIDS)('`PluginSchema` accepts %s — the spec half of the shared grammar', (version) => {
403+
expect(PluginSchema.safeParse({ name: 'x', version, init: () => {} }).success).toBe(true);
404+
});
405+
406+
it.each(SEMVER_FORBIDS)('and `kernel.use()` boots it — the loader half agrees on %s', async (version) => {
407+
const kernel = makeKernel();
408+
409+
await expect(kernel.use(fixture({ name: `com.example.fringe-${version}`, version }))).resolves.toBe(kernel);
410+
});
411+
379412
it('and a malformed version is STILL refused by the loader, with its own message', async () => {
380413
const kernel = makeKernel();
381414
const bad = fixture({ name: 'com.example.bad-version', version: 'v1.0.0' });

‎packages/core/src/plugin-contract.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ import type { Plugin } from './types.js';
123123
* This function used to filter `version` issues out. It did so because the two
124124
* declarations disagreed: `PluginSchema.version` was `/^\d+\.\d+\.\d+$/` and
125125
* refused the prerelease and build-metadata forms SemVer 2.0.0 defines, while
126-
* `PluginLoader.isValidSemanticVersion` — the check the loader has always run —
126+
* `PluginLoader.isSemverShapedVersion` — the check the loader has always run —
127127
* implemented the full grammar and accepted them. Enforcing the narrow spelling
128128
* would have RETIRED a pinned capability under a card that ruled on `type`, so
129129
* the disagreement was declared here rather than performed.

0 commit comments

Comments
 (0)