Skip to content

Commit 782c724

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-13986-structured-logger-volume-census
2 parents 8eb2a3b + 5bc2f27 commit 782c724

133 files changed

Lines changed: 9363 additions & 811 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: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): an `autonumber` field is `unique: 'organization'` by default; explicit `unique: false` opts out (#13894)
6+
7+
**BREAKING** emitted-shape change on `FieldSchema` (the accept set is unchanged),
8+
shipped as `minor` under the repo's launch-window convention for breaking changes.
9+
10+
An auto-number is a business identifier — a contract number, a quote number, a
11+
case number — and an identifier that may repeat is not one. Yet the platform
12+
only ever materialized a unique index where the author had written `unique`
13+
by hand: of hotcrm's nine auto-numbered identifiers, exactly one
14+
(`crm_case.case_number`, `unique: true`) carried the tenant-composite unique
15+
index, and the other eight could mint the same number twice (measured:
16+
objectstack#12394 re-issued `ACC-000009`). Maintainer ruling 2026-08-31
17+
(hotcrm#1301): the default flips.
18+
19+
- An `autonumber` field that **omits** `unique` now parses to
20+
`unique: 'organization'` — one holder per organization, materialized by the
21+
drivers exactly as `case_number`'s hand-written declaration was: the NULL-safe
22+
tenant-composite index `(COALESCE(organization_id, '__global__'), <field>)` on an
23+
organization-scoped object, a plain unique index on an object with no
24+
organization key.
25+
- Every **other** field type keeps `unique: false` as its default, at the same
26+
key position — parse output for non-autonumber fields is byte-identical.
27+
- Every **authored** spelling (`true`, `'organization'`, `'global'`, `false`)
28+
parses exactly as before, on every type.
29+
- The default is materialized at parse time (the `.overwrite()` tail of
30+
`FieldSchema`, the type-conditional precedent `deleteBehavior` set), because
31+
the drivers read the parsed `unique` value-only; the published JSON Schema
32+
therefore no longer carries `default: false` on `Field.unique` — the
33+
description states the rule, and the authorable-defaults ratchet records the
34+
move as `data/Field:unique = false → (none)`.
35+
36+
**Opting out.** Write `unique: false` explicitly on the autonumber field. That
37+
is the whole opt-out surface — no second key. It is legitimate only for a
38+
display-only sequence that nothing uses to identify the record; note that the
39+
platform's duplicate scan (`os migrate duplicates`) keeps treating every
40+
autonumber field as an identifier regardless.
41+
42+
**Migration — what an operator with existing duplicates sees.** A table that
43+
already holds duplicate auto-numbers cannot take the index. On SQLite/Postgres/
44+
MySQL the SQL driver does not fail the boot and does not skip silently: it logs
45+
on the `error` channel —
46+
47+
```
48+
[sql-driver] cannot create NULL-safe unique index 'uniq_crm_quote_organization_id_quote_number' on "crm_quote" — existing rows violate it (duplicates the previous NULL-distinct index admitted, #5030). The constraint 'organization_id, quote_number' is NOT enforced until the data is deduplicated: run "os migrate plan" for the conflicting rows (ADR-0120 D4).
49+
```
50+
51+
— and the same boot's drift pass names the conflicting key groups with their
52+
row counts:
53+
54+
```
55+
[schema-drift] crm_quote: cannot create 'uniq_crm_quote_organization_id_quote_number' as UNIQUE (COALESCE(organization_id, '__global__'), quote_number) — existing rows already violate the NULL-safe unique constraint (duplicates the old index wrongly admitted, #5030): (organization_id="__global__", quote_number="QUO-00009") × 2 rows; (organization_id="org_x", quote_number="QUO-00010") × 2 rows. The op is BLOCKED: apply re-probes and refuses, and the existing index stays in place (ADR-0120 D4). Deduplicate the listed rows, then re-run "os migrate plan".
56+
```
57+
58+
`os migrate plan` reports the same blocked `create_index` with the same groups
59+
until the rows are deduplicated; `os migrate duplicates` lists the holder row
60+
ids of any value minted across organization partitions (the seed/API split).
61+
Deduplicate — which duplicate keeps its number is a business decision — then
62+
re-run `os migrate plan` / restart, and the index materializes. An object with
63+
`tenancy.enabled: false` takes a plain unique index instead, and there the
64+
driver raises the database's own unique-violation error at boot (it names the
65+
index, not the rows) — run `os migrate duplicates` / a `GROUP BY <field> HAVING
66+
COUNT(*) > 1` to find them.
67+
68+
Two landed defects change shape on purpose under the default: a counter that
69+
re-issues a number after a burned reservation (#12394) and two counters minting
70+
for one object (#8686) used to produce a *silent* duplicate; they now produce a
71+
loud unique-violation refusal at the write.
72+
73+
<!-- adr-0087: registered autonumber-default-unique-organization -->
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/plugin-sharing': patch
3+
---
4+
5+
Fix: a sharing rule with a business-unit recipient granted nothing when the unit came from seed data — and tenant-screen the member reads that widening exposes.
6+
7+
`BusinessUnitGraphService.orgScope` screened `sys_business_unit` with a strict `organization_id` equality, while the platform's own read-side chokepoint (`SqlDriver.applyTenantScope`) is null-inclusive: `(organization_id = ? OR organization_id IS NULL)`, because a NULL organization marks a platform/seeded row every tenant may see. A sharing rule always carries the caller's organization, but a business unit written by seed data carries none — a seed cannot know the id the runtime mints at boot — so the two never matched. The seed check read the unit as "does not exist", both recipient widths (`business_unit` and `unit_and_subordinates`) expanded to zero users, and the rule stayed active having materialised no `sys_record_share` row and logged nothing. `orgScope` now applies the platform's null-inclusive screen, the same predicate `plugin-approvals` already applies to these very rows and `SharingRuleService.adminOrgScope` applies to the rule table.
8+
9+
The member reads are now tenant-screened, which they were not before. Both `expandUnitMembers` and `expandUsers` queried `sys_business_unit_member` with no organization predicate at all, under a system context that carries no tenant either, so the strict unit screen was the only thing keeping an org-stamped rule away from that unscoped query. Widening the unit screen alone would have turned a silent under-grant into a silent cross-tenant over-grant, since a seeded unit id exists identically in every tenant. The member screen is strict rather than null-inclusive on purpose: seed replay and elevated system writes both leave `sys_business_unit_member.organization_id` NULL, so a NULL there means unknown tenancy rather than platform-global, and an org-scoped rule does not grant to it. The sibling recipient widths already read their membership rows this way.
10+
11+
An active business-unit rule that expands to no recipients now warns once per rule per process, naming the rule, the object, the recipient kind, the unit and the organization. That case — a rule whose unit and membership rows were both seeded — is the one combination that still grants nobody, and it is no longer silent.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
---
4+
5+
fix(automation): `create_record` now surfaces the engine's `DUPLICATE_RECORD` code, so a `try_catch` / `fault` edge can finally tell "already there" from "the store is down" (#14419)
6+
7+
`engine.insert` (#14095) already raises `DuplicateRecordError``code: 'DUPLICATE_RECORD'` (ADR-0112) — for a unique-constraint violation, driver-independent. The `create_record` node executor threw that away: every failure, from a duplicate key to a downed connection, collapsed into one opaque string (`create_record(<object>) failed: <message>`). A flow's only two error-handling primitives, `try_catch` and a `fault` edge, saw the same shape either way — the only expressible reading of "swallow the duplicate" was "swallow everything".
8+
9+
`NodeExecutionResult` gains an optional `code?: string` field, beside the existing `errorClass`, set when the caught error carries the platform's classified `DUPLICATE_RECORD` code. `AutomationEngine` now copies it onto the `$error` run variable alongside `message` (both the direct `fault`-edge path and the `try_catch` catch-region binding, which previously reconstructed `errorVariable` from the caught exception's message alone and silently dropped it), so a flow can actually branch on `{$error.code}`:
10+
11+
```
12+
try: create_record(lead, { email })
13+
catch: { $error.code === 'DUPLICATE_RECORD' } → swallow, continue
14+
else → re-raise / route the fault edge
15+
```
16+
17+
Additive only — no existing field, message text or routing behaviour changes; an executor that never sets `code` (every one except `create_record` today) is unaffected. Deliberately scoped to `create_record` alone: `update_record` / `delete_record` collapse the same way, but `engine.update` still leaks the raw driver error (#14390, not yet fixed), so those node results have nothing structured to surface yet. `create_record` itself forwards `code` only when it equals `DUPLICATE_RECORD` — narrowly, on purpose, matching the ADR-0112 vocabulary member this repair was actually scoped to surface, not any code an as-yet-unaudited driver error might someday carry.
18+
19+
**Patch round 1 (tier contract review):** `try_catch`'s catch region reads `code` off the run-wide `$error`, but the engine only rewrites `$error` when a failing node *returns* a failure, or *throws* through a node with its own `fault` edge — and a node inside a `try_catch`'s `try` region never has one (the region's synthetic sub-flow carries only the region's own edges). A node that fails by throwing (a `timeoutMs` firing, a dying nested container) therefore used to leave `$error` exactly as an *earlier, unrelated* failure left it — its `code` included. An identity guard (`$error` must have *changed*, not merely still be present, since this attempt started) closes that; two flows now pin it: a `loop` sweeping two rows where row 1 is a genuine duplicate and row 2 times out, and a plain flow where an earlier fault-routed duplicate must not leak into a later, unrelated `try_catch`.
20+
21+
A custom `IDataEngine` implementation whose thrown error already carries `code: 'DUPLICATE_RECORD'` (without being an instance of `@objectstack/objectql`'s `DuplicateRecordError`) is treated as a duplicate too — correct under ADR-0112, since `code` is the classified envelope's public contract, not the concrete class.
22+
23+
**Known gap, filed rather than fixed here (out of this lane's scope):** `packages/spec`'s `TryCatchErrorValueSchema` — the ONE declared shape for the `errorVariable` binding shared by author, engine and run log — does not declare `code` yet, and strips it on a strict parse. `packages/spec` is single-owner (`domain:spec`); tracked as #14954.
24+
25+
<!-- adr-0087: not-required (no-migration-prescription) additive optional field; no authorable key, export removal or rename for an upgrader to migrate -->
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): `os create example` now scaffolds a manifest the protocol schema accepts
6+
7+
The `objectstack.config.ts` that `os create example <name>` wrote declared
8+
three manifest keys — `name`, `version`, `description` — and nothing else.
9+
`ManifestSchema` requires `id` (the reverse-domain package id) and `type`
10+
(`app` | `plugin` | …), and `namespace` is the mandatory prefix of every object
11+
name, which decides each object's table name and REST path. Parsed against the
12+
schema, the emitted block answered `success: false` with
13+
`invalid_type@id · invalid_value@type`.
14+
15+
`defineStack` throws on exactly that, so the project a documented command had
16+
just created refused to load on its first run — before the author had written a
17+
line. The three `os init` templates all stamped the identity block; this was
18+
the one scaffold that had drifted, and nothing noticed because no test looked
19+
at these templates as data.
20+
21+
The template now stamps what `os init` stamps: `id`, `namespace` (derived from
22+
the project name with `init`'s own `sanitizeNamespace`, so both scaffolders
23+
answer the same way for the same input), `type: 'app'` and
24+
`engines.protocol`, alongside the `version`, `name` and `description` it
25+
already carried. `engines.protocol` is stamped from `PROTOCOL_MAJOR` — the same
26+
constant `init` stamps — and ships with the same self-contained comment
27+
explaining what the range is and when to move it.
28+
29+
A pin sweeps both scaffolders: every `init` and `create` template that emits an
30+
`objectstack.config.ts` is rendered through its own emitter, loaded back, and
31+
its `manifest` parsed through the real `ManifestSchema`. The population is
32+
derived from the two template maps rather than listed, so a template added
33+
later is swept the day it is added.
34+
35+
`os create` itself is untouched — it is not removed, deprecated, or redirected
36+
at `os init`. Whether the two scaffolders should stay separate is a CLI-surface
37+
decision, not this fix.
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
---
2+
"@objectstack/driver-mongodb": minor
3+
"@objectstack/driver-turso": minor
4+
---
5+
6+
fix(drivers): `update()` on a missing id answers `null` on MongoDB and on Turso's remote face
7+
8+
**BREAKING** for TypeScript consumers — a published TYPE-surface narrowing alongside a
9+
runtime behaviour change, shipped as `minor` under the launch-window convention. Two
10+
published declared returns move: `MongoDBDriver.update()` and `RemoteTransport.update()`
11+
(both exported from their package index) now declare
12+
`Promise<Record<string, unknown> | null>` where they declared
13+
`Promise<Record<string, unknown>>`. A caller that reads fields off either result —
14+
`result.id`, `result.title` — no longer compiles until it narrows the `null` arm first.
15+
The narrowing is delivered by the compiler at every call site, and it is the honest
16+
declaration: the value that arm carries has always been reachable, it was simply being
17+
answered with a fabricated record instead.
18+
19+
`IDataDriver.update()` declares `Promise<Record<string, unknown> | null>` — the
20+
not-found arm landed with the ruling on the contract (`packages/spec` is untouched here),
21+
and it is the answer `InMemoryDriver`, `SqlDriver`, `SqliteWasmDriver` and `TursoDriver`'s
22+
local face have always given. Two implementations did not honour it. They **invented a
23+
record** instead:
24+
25+
- `MongoDBDriver.update()` ran `updateOne({ id })`, then `findOne({ id })`, and
26+
when nothing came back returned
27+
`withoutUndefinedOwnKeys({ id: String(id), ...updateData })` — a row assembled
28+
from the caller's own payload plus the `updated_at` it had just stamped, under
29+
an id that names no document.
30+
- `RemoteTransport.update()` ran `UPDATE … WHERE "id" = ?`, then
31+
`SELECT * … WHERE "id" = ?`, and when no row came back returned
32+
`{ id, ...data }` — the caller's payload with the id stapled on.
33+
34+
Both now return `null`. That is the runtime half of this change, and it is why this
35+
release is not a pure type-surface move: the value a caller receives for a missing id is
36+
different at run time, not only in the `.d.ts`.
37+
38+
This is the expensive direction of wrong, not merely the wrong answer: the
39+
fabricated row said **succeeded** where the truth was **not found**, and said it
40+
in a shape carrying the caller's own fields back, so nothing about it looked
41+
wrong. Through the engine's by-id door a REST / SDK / MCP `update` against a
42+
deleted or mistyped id answered **200 with a record that does not exist** — on
43+
these two implementations only. A caller, human or agent, read that as a landed
44+
write and did not retry, alert or roll back.
45+
46+
Two things downstream become correct rather than merely different:
47+
48+
- **One `TursoDriver`, one answer.** Its remote branch passes the transport
49+
result through `formatRemoteRow`, which already guards
50+
`row && typeof row === 'object'`, so `null` reaches the engine untouched and
51+
the two faces converge with no edit at that seam. Previously the same driver
52+
answered the same missing id two ways, chosen by `isRemote`.
53+
- **`RemoteTransport.bulkUpdate()`'s skip stops being dead code.**
54+
`if (updated) results.push(updated)` is the cross-driver convention
55+
`SqlDriver.bulkUpdate` follows; on this transport `updated` could never be
56+
falsy, so a batch over N missing ids answered N invented rows. It now answers
57+
the rows that exist.
58+
59+
`upsert()` is untouched on both drivers: an upsert never answers "not found".
60+
61+
No landed test pinned the fabricating posture on either driver, so the
62+
regression pins added here are net-new coverage rather than a changed baseline.
63+
64+
<!-- adr-0087: not-required (no-migration-prescription) No metadata key is removed, renamed or re-shaped: the moving surfaces are two driver methods' declared return types and the value they answer for an id that names no row, so there is nothing for `objectstack migrate meta`, `spec-changes.json` or the upgrade guide to project, and this changeset prescribes no rewrite. The consumer obligation is a TypeScript narrowing at the call site, delivered by the compiler. `type-surface-only` is NOT claimable here: its predicate 4 (narrowed-from-erased) is false — neither declared return was `any` at the merge base, they were the non-null `Promise<Record<string, unknown>>` — and, independently, runtime behaviour moves in the same diff, which is more than a type surface. Same disposition and reasoning as `.changeset/driver-memory-update-upsert-honest-types.md` (PR #14434), one day earlier in this same series. -->
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/metadata": minor
3+
---
4+
5+
fix(metadata): `FilesystemLoader.list()` reports only names `findFile()` / `load()` / `exists()` can resolve
6+
7+
**BREAKING (list output narrows).** Two shapes stop appearing in
8+
`FilesystemLoader.list()`, and therefore in `MetadataManager.listNames()`:
9+
10+
- **nested files**`ROOT/TYPE/crm/account.json` was listed as `account`, a
11+
name that resolves against `ROOT/TYPE/account.json` and finds nothing;
12+
- **extension-less files**`ROOT/TYPE/noext` was listed as `noext`, which
13+
resolves under no appended extension at all.
14+
15+
A third shape follows from the same rule rather than from a rule of its own:
16+
the extensions a name can be resolved under are now the ones belonging to the
17+
loader's **registered serializers**, so under the manager's default format set
18+
(`typescript` / `json` / `yaml`) a `.js` file leaves `list()` too. It was
19+
previously listed and resolvable while `loadMany()` could never return it and
20+
`load()` threw `No serializer found for format: javascript`. Register the
21+
`javascript` serializer and it is listed, resolvable and loadable together.
22+
23+
`list()`, `findFile()` and `loadManyKeyed()` now share one name-to-path
24+
derivation, so `listNames()` and `get()` give the same answer. Previously a name
25+
could sit in the list while `get()` answered `null` for it — a silent failure an
26+
author reads as their own typo.
27+
28+
Nothing changes for a tree whose metadata is laid out as `ROOT/TYPE/NAME.json`
29+
(or `.yaml` / `.yml` / `.ts`), which is the layout ADR-0008 §10 already
30+
prescribes and `metadata-fs`'s `parseItemPath()` already enforces. `.yaml`,
31+
`.yml` and `.ts` are unaffected: the extension set follows the registered
32+
serializers, not §10's `.json`-only rule, which governs the `metadata-fs` store.
33+
34+
`loadMany()` is unchanged and still returns bodies for nested and
35+
extension-less files; `findFile()` still resolves an explicitly path-shaped
36+
name such as `crm/account`, which nothing lists.
37+
38+
<!-- adr-0087: not-required (no-migration-prescription) No authorable key, Zod schema or stored row moves: this narrows one runtime loader's `list()` output. The ledger's artifacts project metadata rewrites, and there is nothing here for `objectstack migrate meta` to rewrite — a tree carrying a nested or extension-less file needs the FILE relocated into the two-segment layout ADR-0008 §10 already prescribes, which no migration prescription can express. -->

0 commit comments

Comments
 (0)