Skip to content

Commit 63b33e6

Browse files
os-zhuangclaude
andauthored
fix: 四处静默失真 — datasource 映射、meta type 归一化、value-shape 扫描、未接线的 lint 规则 (#4462, #4432, #4455, #4449) (#4520)
* fix(cli,lint): run validateFormLayout, and close the rule registry from the other side (#4449) `validateFormLayout` was implemented, unit-tested, exported and given published rule ids — and no command ever called it. A whole-repo search found the implementation, the barrel export line and its own unit test, and nothing else: the rule ran on zero stacks for as long as it existed. Two changes: * register it in `AUTHORING_RULES` as `advisory` on all three commands. It walks structured metadata only (no lazy dependency), so `os validate`, `os build` and `os lint` pay nothing measurable for it. * add the reverse closure to the wiring guard. Every invariant #4409 shipped starts FROM a registry and looks at the commands, which cannot see a rule that never entered a registry — the same blind spot as #4402's name list, one layer up. The guard now subtracts both registries from the `validate*` / `lint*` symbols on `@objectstack/lint`'s public barrel; the difference must be empty or ledgered with a reason in `UNWIRED_RULE_LEDGER`, which ships empty because today's difference was exactly this one rule. The new tests fail without the registry entry: the closure reports `validateFormLayout` as unwired, and the liveness test asserts the entry's own `run` adapter returns both findings for a stack that earns them — membership alone is not evidence a rule produces output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD * fix(spec): a stored reference holding an embedded record is not a valid id (#4455) `os migrate value-shapes` is the evidence half of the ADR-0104 D1 per-deployment gate, and the scan's own header names the case it exists for: "a `location` stored as `{latitude, longitude}` or a `lookup` holding an expanded record object". The second case was never detected. `ReferenceIdValueSchema` was `z.string().min(1)`, and in a SQL deployment a legacy embedded reference reaches storage as JSON TEXT — a non-empty string. So a deployment carrying exactly the values the gate exists to find ran the scan, was told it was clean, and closed the gate with `--apply`; and because the scan deliberately imports the write-path predicate, the write path was equally blind, so the value also survived future writes. `ReferenceIdValueSchema` now rejects a value whose first non-space character is `{` or `[`, in the expanded form too — `$expand` produces an object, never its serialization. Deliberately narrower than an id charset. `FileReferenceIdValueSchema` can bound its alphabet because a `sys_file` id is minted by the platform and nothing else; a reference id is whatever the target object's key holds, including an external key an ADR-0015 federated datasource supplies. So this rejects the shape that is provably not an id and leaves the alphabet to the object that owns it — `CB0-2026-0001`, `SFDC:001xx…` and `ops/eu-west/tenant-7` stay valid, and the tests pin that. Regression coverage is at the GATE, not just the schema: the scan test plants the serialized embedded record, asserts it is counted, and asserts `valueShapeScanPassed()` is false — the deployment may not record the flag — then asserts the same value is a write rejection under strict, so the scan and the validator still answer with one predicate. Reaches authors through the ADR-0104 warn-first path until a deployment opts into strict, so nothing starts rejecting writes on upgrade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD * fix(metadata-protocol): one canonical type key at the /meta boundary (#4432) #3985 taught the per-type gates to accept both spellings of the `/meta` type segment. It did not FOLD them, so `/meta/actions/x` and `/meta/action/x` addressed two namespaces and the layers below disagreed about which one an item lived in — `SysMetadataRepository` folded to singular on its own, while the authorization tier above it (`isOverlayAllowed`, `isArtifactBacked`), the registry heal below it (`restoreArtifactRegistryView`) and the list hydration all read the caller's spelling. The damaging half was the hydration. `getMetaItems` registered overlay rows back into the SchemaRegistry under `request.type`, so one plural-spelled read minted a PLURAL registry entry; from the next read on `listItems('actions')` was no longer empty, the singular fallback that had been supplying every code-authored action stopped running, and one overlay row hid the entire code-authored listing — on a spelling no DELETE addresses, so it outlived the delete that was meant to lift it and left listing and dispatch disagreeing about a removed item. `saveMetaItem`, `getMetaItem`, `getMetaItems`, `getMetaItemLayered`, `getMetaItemCached` and `deleteMetaItem` now fold the type to its canonical singular as their first act. Reads of data AT REST keep the other-spelling fallback: rows written under a plural `type` before this fix are real and nothing rewrites them on upgrade. What changed is that nothing WRITES or REGISTERS a non-canonical key any more. Regression tests fail without the fold: a plural-spelled read mints a phantom `actions` registry entry and the second read drops the code-authored actions, and `getMetaItem` echoes back the caller's spelling so a client can round-trip it into a second namespace. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD * fix(objectql,service-datasource,runtime): a datasourceMapping rule is routing, not a hint (#4462) Measured on `main` during the v17 verification: map an object to a Postgres datasource with a bad URL and the boot SUCCEEDS, `/ready` answers 200, the datasource name appears in ZERO log lines, the write returns 201 — and the row is physically in the DEFAULT store. The operator finds out by opening the database they declared and finding it empty. Two causes, one per layer, and each is what makes fixing the other correct: * `ObjectQLEngine.getDriver` step 2 read `mapped && drivers.has(mapped)`, so a MATCHED mapping rule naming a datasource with no live driver fell silently through to the default driver. It now throws — `DatasourceUnavailableError` when the connect layer recorded a verdict (#3828), otherwise an error naming the object, the datasource and the two remedies. `default` still resolves onward: the default driver keeps its natural name (#3826), so `drivers.has('default')` is false by construction and step 5 IS how routing to it works. * ADR-0062 D2's phase-1 note deliberately excluded "mapped" from the auto-connect gate, to keep `examples/app-crm` byte-for-byte unchanged. That note priced only one side. Gate (d) now fires when a mapping rule routes at least one object to a datasource, and a `declared-auto` failure is FATAL with an operator-readable reason — the same call gate (b) already makes for an explicit `object.datasource` binding, correct for (d) now that routing no longer supplies a fallback. `OS_ALLOW_DRIVER_CONNECT_FAILURE` still degrades. The mapped-object list comes from the engine's own matcher (`ObjectQLEngine.resolveMappedDatasource`, newly public) via `connectDeclared({ mappedObjects })`. The connection service never re-derives rule matching: two matchers drifting by one clause would connect a datasource routing never uses, or route to one nothing connects — the defect again. `examples/app-crm`'s mapping is DELETED, and that is what keeps the example unchanged rather than what breaks it. Its `namespace: 'crm'` rule never matched (`namespace` is deprecated; no object sets it) and its `default: true → crm_primary` rule routed everything to an unconnected `:memory:` datasource, i.e. to the default store by fall-through. Honouring it would move the whole app — platform objects included — onto a database empty on every boot. Verified against a real boot on a private port, not only in unit tests: * unchanged CRM example boots healthy; crm_primary/crm_analytics stay `unvalidated` (metadata-only) exactly as before; * with a mapping to `postgres://…@127.0.0.1:1/nonexistent_db`, boot exits 1 with "1 object(s) are routed to it by a datasourceMapping rule (crm_account) and have no fallback datasource — their reads/writes would otherwise land in a DIFFERENT database than the one they declare ⇒ fail-fast per ADR-0062 D5"; * under OS_ALLOW_DRIVER_CONNECT_FAILURE=1 the degraded-boot banner carries the same sentence and the mapped object's seeds fail instead of silently populating the default store. ADR-0062 D2 carries the amendment; the docs page and the data skill now state that a mapping rule is routing and fails the boot when it cannot be honoured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent fd3013a commit 63b33e6

20 files changed

Lines changed: 977 additions & 53 deletions
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/objectql": minor
3+
"@objectstack/service-datasource": minor
4+
"@objectstack/runtime": minor
5+
---
6+
7+
A `datasourceMapping` rule is routing, not a hint — an object mapped to an
8+
unreachable datasource no longer silently reads and writes the DEFAULT store
9+
(#4462).
10+
11+
**Observable behavior change; read this before upgrading.** Measured on `main`
12+
during the v17 verification: map an object to a Postgres datasource with a bad
13+
URL and the boot succeeds, `/ready` answers `200`, the datasource name appears in
14+
**zero** log lines, `POST /api/v1/data/<mapped object>` returns `201` — and the
15+
row is physically in the default store. The operator finds out by opening the
16+
database they declared and finding it empty. ADR-0062 D2's phase-1 note called a
17+
mapping-only datasource "decorative" to keep an example byte-for-byte unchanged;
18+
what that bought was a silent data-placement bug.
19+
20+
The fix is a pair, and each half is what makes the other correct:
21+
22+
1. **Routing stops falling through** (`@objectstack/objectql`). `getDriver` step
23+
2: a mapping rule that MATCHES and names a datasource with no live driver now
24+
throws — `DatasourceUnavailableError` when the connect layer recorded a
25+
verdict, otherwise an error naming the object, the datasource and the two
26+
remedies. `default` still resolves onward: the default driver keeps its
27+
natural name (#3826), so step 5 is how routing to it works.
28+
2. **ADR-0062 D2 grows gate (d)** (`@objectstack/service-datasource`,
29+
`@objectstack/runtime`). A datasource a mapping rule routes at least one
30+
object to is auto-connected at boot, and a boot-time connect failure is
31+
**fatal** with an operator-readable reason — the same call gate (b) already
32+
makes for an explicit `object.datasource` binding, now correct for (d)
33+
because half 1 removed the fallback. `OS_ALLOW_DRIVER_CONNECT_FAILURE` still
34+
degrades the boot instead, as for every other fatal connect.
35+
36+
The mapped-object list is resolved by the boot path from the engine's own
37+
matcher (`ObjectQLEngine.resolveMappedDatasource`, newly public) and passed to
38+
`connectDeclared({ mappedObjects })`; the connection service never re-derives
39+
rule matching. Two matchers drifting by one clause would connect a datasource
40+
routing never uses, or route to one nothing connects — the defect again.
41+
42+
**What to do if this breaks your boot.** It means a `datasourceMapping` rule in
43+
your stack points at a datasource that cannot be connected. Either fix the
44+
datasource configuration, or delete the rule — the second is what
45+
`examples/app-crm` did in this change, and it is what keeps that example's
46+
runtime behavior identical: its rules routed everything to an unconnected
47+
`:memory:` datasource, i.e. to the default store by fall-through.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
Wire `validateFormLayout` into the authoring-rule registry, and close the
6+
registry from the other direction (#4449).
7+
8+
`validateFormLayout` was implemented, unit-tested, exported from
9+
`@objectstack/lint` and given published rule ids (`form-field-unknown`,
10+
`absolute-colspan-discouraged`) — and **no command ever called it**. It ran on
11+
zero stacks for as long as it existed, so a form section referencing a field
12+
that is not on the bound object, or pinning an absolute `colSpan` under a
13+
per-surface derived column count, produced no output anywhere. It is now an
14+
`advisory` entry in `AUTHORING_RULES`, so `os validate`, `os build` and
15+
`os lint` all run it. It is a pure structured-metadata walk with no lazy
16+
dependency, so all three commands pay nothing measurable.
17+
18+
The wiring guard (#4409) could not have found this. Every one of its invariants
19+
starts FROM a registry and looks at the commands, which is blind by construction
20+
to a rule that never entered a registry — the same shape as #4402's name list
21+
guarding only the names on it, one layer up. The guard now also runs the reverse
22+
subtraction: every `validate*` / `lint*` symbol on `@objectstack/lint`'s public
23+
barrel, minus `AUTHORING_RULES``REFERENCE_INTEGRITY_RULES`, must be empty or
24+
carry a written reason in `UNWIRED_RULE_LEDGER`. The ledger ships empty: today's
25+
difference was exactly this one rule.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
One canonical type key at the `/meta` read/write/delete boundary (#4432).
6+
7+
#3985 made the per-type gates accept both spellings of the `/meta` type segment
8+
(`/meta/actions` and `/meta/action`). It did not FOLD them, so the two spellings
9+
addressed two different namespaces and the layers below disagreed about which
10+
one an item lived in. `saveMetaItem`, `getMetaItem`, `getMetaItems`,
11+
`getMetaItemLayered`, `getMetaItemCached` and `deleteMetaItem` now fold the type
12+
to its canonical singular (Prime Directive #3) as their first act, so every layer
13+
below them reads one key.
14+
15+
The damaging consequence was not the duplicate row — it was the shadowing.
16+
`getMetaItems` hydrated overlay rows back into the SchemaRegistry under the
17+
CALLER's spelling, so one plural-spelled read minted a plural registry entry;
18+
from the next read on, `listItems('actions')` was no longer empty, the singular
19+
fallback that had been supplying every code-authored action stopped running, and
20+
a single overlay row hid the entire code-authored listing — on a spelling no
21+
DELETE could address, because the delete path resolved the singular. Listing and
22+
dispatch then disagreed about an item that had been deleted.
23+
24+
Reads of data AT REST still try the other spelling as a fallback: rows written
25+
under a plural `type` before this fix are real, and nothing rewrites them on
26+
upgrade. What changed is that nothing WRITES or REGISTERS a non-canonical key any
27+
more.
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
A stored reference value that is an embedded record is no longer a valid id
6+
(#4455).
7+
8+
`os migrate value-shapes` is the evidence half of the ADR-0104 D1 per-deployment
9+
gate, and its own header names the case it exists for: "a `location` stored as
10+
`{latitude, longitude}` **or a `lookup` holding an expanded record object**". The
11+
second case was not detected. `ReferenceIdValueSchema` was
12+
`z.string().min(1)`, and in a SQL deployment a legacy embedded reference reaches
13+
storage as JSON *text* in a TEXT column — a non-empty string. So a deployment
14+
carrying exactly the values the gate exists to find ran the scan, was told it was
15+
clean, and closed the gate with `--apply`; because the scan deliberately imports
16+
the write-path predicate, the write path was equally blind and the value survived
17+
future writes too.
18+
19+
`ReferenceIdValueSchema` now rejects a value whose first non-space character is
20+
`{` or `[`, in both the stored and the expanded form (`$expand` produces an
21+
object, never its serialization).
22+
23+
The rejection is deliberately narrower than the issue's first suggestion. Its
24+
file sibling `FileReferenceIdValueSchema` can bound its charset because a
25+
`sys_file` id is minted by the platform and by nothing else; a reference id is
26+
whatever the target object's primary key holds, including an external key an
27+
ADR-0015 federated datasource supplies. So this rejects the shape that is
28+
provably not an id (`{"id":"acc_1","name":"embedded"}`) and leaves the id
29+
alphabet to the object that owns it — `CB0-2026-0001`, `SFDC:001xx…` and
30+
`ops/eu-west/tenant-7` all remain valid. Widening it further needs evidence about
31+
real external keys, not a guess.
32+
33+
Reaches authors through the ADR-0104 warn-first path (a `[value-shape]` log line)
34+
until a deployment opts into strict, so nothing starts rejecting writes on
35+
upgrade — but the scan now counts these values, and a deployment holding them can
36+
no longer close the gate.

content/docs/data-modeling/external-datasources.mdx

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,22 @@ A declared datasource auto-connects when it is **meaningfully addressed**:
108108

109109
1. it is **external** (`schemaMode !== 'managed'`), **or**
110110
2. an object **explicitly** binds to it via `object.datasource === <name>`, **or**
111-
3. it sets **`autoConnect: true`**.
111+
3. it sets **`autoConnect: true`**, **or**
112+
4. a **`datasourceMapping` rule routes at least one object to it**.
112113

113-
A `managed` datasource that nothing explicitly binds to (for example one that is
114-
only referenced by a `datasourceMapping` rule) stays *metadata-only* — visible in
115-
Setup, but not connected — so existing apps are unchanged. Use `autoConnect: true`
116-
to opt such a datasource into a live connection at boot.
114+
A `managed` datasource that nothing routes to stays *metadata-only* — visible in
115+
Setup, but not connected. Use `autoConnect: true` to opt such a datasource into a
116+
live connection at boot.
117+
118+
<Callout type="warn">
119+
**A mapping rule is routing, not a hint.** If a `datasourceMapping` rule routes an
120+
object to a datasource that cannot be connected, the boot **fails** with the
121+
connect error, and a query against that object throws rather than resolving the
122+
default store. Before v17 it fell through silently: the app booted clean, `/ready`
123+
answered `200`, and the object's rows were written to the *default* database
124+
instead of the one it declared. If you want a declared datasource that routes
125+
nothing, remove the mapping rule rather than relying on the fall-through.
126+
</Callout>
117127

118128
<Callout type="info">
119129
**Escape hatch.** An `onEnable` hook calling `ctx.drivers.register(driver)` is

docs/adr/0062-external-datasource-runtime.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,15 @@ Introduce a single service that, given a datasource definition, builds a driver
7777
Auto-connect must not change apps that today declare datasources that are *decorative* or routed via `datasourceMapping` (e.g. `examples/app-crm`'s `crm_primary`/`crm_analytics`). Gate auto-connect so a declared datasource is only connected when it is meaningfully addressed: **(a)** it is `external` (`schemaMode !== 'managed'`), or **(b)** an object/`datasourceMapping` actually routes to it, or **(c)** it sets an explicit `autoConnect: true`. A managed datasource that nothing routes to stays metadata-only (today's behavior). The `default` datasource keeps its current dedicated bootstrap. This is the load-bearing backward-compat decision.
7878

7979
> **Phase 1 implementation note (#2163) — gate (b) is "explicit `object.datasource`", not "mapped".** Implementing D2 against `examples/app-crm` surfaced a conflict between "an object/`datasourceMapping` routes to it" and the "byte-for-byte unchanged" mandate. `app-crm`'s `crm_primary` (`:memory:`, `managed`) *is* referenced by a `datasourceMapping` rule (and is the `default:true` fallback) but has **no** `onEnable` driver, so today `engine.getDriver` finds no `crm_primary` driver and its objects fall through to the `default` driver. Auto-connecting it on the strength of the mapping rule would build a fresh, empty `:memory:` driver and silently divert those objects — a behavior change. So the gate **does not** auto-connect on a `datasourceMapping` rule alone: a *managed* datasource that is only mapped (namespace/package/`default`) is treated as decorative and left metadata-only. Gate (b) fires only when an object **explicitly** binds via `object.datasource === <name>` — a binding that today *throws* when the driver is unregistered, so auto-connecting it is a strict improvement, never a change. External datasources (a) and `autoConnect:true` (c) are unaffected. See `isDatasourceAddressed()` in `@objectstack/service-datasource`.
80+
>
81+
> **Amendment (#4462) — the phase-1 note is REVERSED: gate (d) is "a mapping rule routes objects here", and mapping-only is no longer decorative.** The note above priced the trade-off with only one side on the table. The other side, measured on `main` during the v17 verification, is what a mapping to an **unreachable** datasource does today: the boot succeeds, `/ready` answers `200`, the datasource name appears in **zero** log lines, `POST /api/v1/data/<mapped object>` returns `201` — and the row is physically in the DEFAULT store. The operator discovers it by opening the database they declared and finding it empty. Weighed against that, "decorative" is not a backward-compatibility guarantee; it is a silent data-placement bug wearing one. `datasourceMapping` reads as routing to every author who writes it, and Route-ownership rule #3 ("absence must be loud; prefer failing to falling back") applies to a routing decision as much as to a mounted surface.
82+
>
83+
> The amendment is a **pair**, and each half is what makes the other correct:
84+
>
85+
> 1. **Routing stops falling through.** `ObjectQLEngine.getDriver` step 2: a mapping rule that MATCHES and names a datasource with no live driver now throws — `DatasourceUnavailableError` when the connect layer recorded a verdict (framework#3828), otherwise a "mapped for object … is not registered" error naming the two remedies. `default` is the one name that still resolves onward: the default driver keeps its natural name (#3826), so `drivers.has('default')` is false by construction and step 5 is how routing to it works.
86+
> 2. **The D2 gate grows (d).** A datasource a mapping rule routes at least one registered object to is auto-connected at boot, and a `declared-auto` failure is **fatal** — the same argument (b) already makes, now true of (d) because half 1 removed the fallback. The object list is resolved by the boot path from the engine's own matcher (`ObjectQLEngine.resolveMappedDatasource`), never re-derived in the connection service: two matchers drifting by one clause would connect a datasource routing never uses, or route to one nothing connects, which is the defect again.
87+
>
88+
> `examples/app-crm`'s mapping was **deleted** in the same change, and that is what keeps the example byte-for-byte unchanged rather than what breaks it: its `namespace: 'crm'` rule never matched (`namespace` is deprecated and no object sets it), and its `default: true → crm_primary` rule routed everything to an unconnected `:memory:` datasource, i.e. to the `default` store by fall-through. Honouring that rule would move the entire app — platform objects included — onto a database that is empty on every boot. Removing the rule states what the example actually does. The general lesson is the one #2163 half-saw: a rule the runtime ignores is not compatibility, it is an unpaid bill.
8089
8190
### D3 — Credentials resolved at connect via `SecretBinder`/`ICryptoProvider`
8291

examples/app-crm/objectstack.config.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,20 @@ export default defineStack({
5656
requires: ['ui', 'automation'],
5757

5858
// Infrastructure
59+
//
60+
// No `datasourceMapping`. These two datasources are declared to exercise the
61+
// metadata surface, not to route anything: both are `:memory:`, and every
62+
// object here has always been served by the host's `default` store. The
63+
// mapping that used to sit here (`namespace: 'crm'` + `default: true` →
64+
// `crm_primary`) was decorative — `namespace` is deprecated and no object
65+
// sets it, and `crm_primary` had no live driver, so routing fell through to
66+
// `default`. #4462 stopped routing from falling through, because that
67+
// fall-through is what silently put a mapped object's rows in a different
68+
// database than the one it declared. Deleting the rule is what keeps this
69+
// example's behavior IDENTICAL under the new posture; keeping it would move
70+
// the whole app — platform objects included — onto an in-memory database
71+
// that is empty on every boot.
5972
datasources: [CrmDatasource, CrmAnalyticsDatasource],
60-
datasourceMapping: [
61-
{ namespace: 'crm', datasource: 'crm_primary' },
62-
{ default: true, datasource: 'crm_primary' },
63-
],
6473

6574
// Internationalisation
6675
translations: [CrmTranslationBundle],

0 commit comments

Comments
 (0)