Skip to content

Commit ce7e222

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/auth-gate-disconnect-issues-6wz8ew
# Conflicts: # docs/protocol-upgrade-guide.md # packages/spec/src/migrations/registry.ts
2 parents 13c5234 + ce5242c commit ce7e222

102 files changed

Lines changed: 3212 additions & 336 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: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
---
2+
"@objectstack/runtime": minor
3+
"@objectstack/lint": minor
4+
"@objectstack/spec": patch
5+
---
6+
7+
fix(runtime,lint): `action.body` binds a handler only for `type: 'script'` (#4352)
8+
9+
`ActionSchema.body` has always described itself as "Only used when type is
10+
`script`", and its JSDoc went further — "Only meaningful when
11+
`type === 'script'`. When set, the runtime invokes the body inside the sandbox
12+
… and ignores `target`." The runtime read none of it:
13+
`actionBodyRunnerFactory` bound a handler the moment `body` parsed, and
14+
`collectBundleActions` collected any named action. A `type: 'url'` action
15+
carrying a leftover `body` was therefore registered in the action registry and
16+
executed in the sandbox — reachable through
17+
`POST /api/v1/actions/:object/:action` and through
18+
`ql.object(o).execute(name)`, and counted by the governance inventory as a live
19+
handler.
20+
21+
Declared ≠ enforced, in the shape that is hardest to debug: an author flips
22+
`type` from `script` to `url`, reasonably concludes the body is now dead code,
23+
and it keeps running with nothing anywhere saying so.
24+
25+
**Behaviour change.** `body` now runs only under `type: 'script'`:
26+
27+
| Action | Before | After |
28+
|:--|:--|:--|
29+
| `type: 'script'` + `body` | body runs | unchanged — body runs |
30+
| `type` omitted + `body` | body runs | unchanged — body runs (`ActionType.default('script')`) |
31+
| `type: 'url' \| 'modal' \| 'flow' \| 'api' \| 'form'` + `body` | body ran | **no handler is bound**; the refusal is logged |
32+
33+
Only an action that **explicitly** declares a non-`script` type *and* carries a
34+
`body` changes behaviour. An omitted `type` still means `script`, because the
35+
collectors walk raw bundle objects — a `strict: false` `defineStack` or a legacy
36+
`manifest.actions[]` never passes through `ActionSchema`, so the schema's own
37+
default has to be applied at the gate rather than assumed to have been applied
38+
already.
39+
40+
**FROM → TO.** If you have an action whose body you want to keep running, set
41+
`type: 'script'` and move the navigation/dispatch target elsewhere; if you want
42+
the target behaviour, delete the now-inert `body`:
43+
44+
```diff
45+
{
46+
name: 'open_portal',
47+
- type: 'url',
48+
+ type: 'script',
49+
target: '/portal',
50+
body: { language: 'js', source: "await ctx.api.object('lead').update(…)", capabilities: ['api.write'] },
51+
}
52+
```
53+
54+
The refusal is **not** silent — silence would only relocate the invisibility the
55+
issue is about. `actionBodyRunnerFactory` logs a warning naming the action, its
56+
declared `type`, and both fixes.
57+
58+
Authoring-time rejection of the same contradiction already shipped in #4438
59+
(`ActionSchema` rejects `body` alongside a non-`script` `type`), so what remains
60+
reachable here is data at rest published before that gate existed, plus bundles
61+
that never parsed. This release closes that half. New tests also pin that the
62+
**publish gate resolves to the rejecting schema** — through
63+
`getMetadataTypeSchema('action')` and `ObjectSchema.actions` — so a re-point of
64+
either registration cannot silently reopen the hole while the schema's own unit
65+
tests stay green.
66+
67+
`@objectstack/lint`'s `validate-action-body-writes` filters by `type` again.
68+
#4344 deliberately made that rule type-blind on the grounds that "the runtime
69+
binds a handler from `action.body` alone … checking what executes beats checking
70+
what the schema says should" — true then, and the comment predicted its own
71+
revision. Execution and declaration are the same set again, so a non-`script`
72+
body no longer produces write-set advice about writes that provably never
73+
happen; the publish gate names that metadata's real defect (`type`) with its own
74+
prescription.
75+
76+
`collectBundleActions` stays deliberately type-blind: it feeds governance
77+
surfaces that must enumerate every declared action, bound or not, and the other
78+
bind path (`engine.setDefaultActionRunner`, for Studio-authored actions) never
79+
walks it. The gate lives at the single point where a `body` becomes an
80+
executable handler, so there is no second copy of the rule to drift.
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/objectql": minor
4+
"@objectstack/plugin-auth": minor
5+
"@objectstack/plugin-audit": minor
6+
"@objectstack/plugin-security": minor
7+
---
8+
9+
feat(auth,objectql,audit,security,spec): identity-table writes carry the real actor, so `sys_member` history stops saying "system" (#4586)
10+
11+
better-auth owns every write to the identity tables (`sys_member`, `sys_user`,
12+
`sys_invitation`, …) and its ObjectQL adapter runs them `isSystem: true` **on
13+
purpose** — the route already authorized the action under better-auth's own ACL,
14+
and ADR-0092 D2 refuses user-context writes to those tables outright. The
15+
consequence was that the human who clicked *make admin* was known exactly once,
16+
in the hook layer where the session exists, and then discarded: every
17+
`trackHistory` transition on `sys_member` recorded `user_id: null` / "system",
18+
and `sys_user_permission_set.granted_by` was written null by the auto-grant.
19+
"Who made this person an org admin?" had no answer in the platform's own audit
20+
log.
21+
22+
**What changed**
23+
24+
A request-scoped attribution seam, general rather than a `sys_member` special
25+
case:
26+
27+
| Layer | Before | After |
28+
|:--|:--|:--|
29+
| `ExecutionContext` | `userId` / `actor` only | new optional `attributedUserId` — the human CREDITED for a write the system AUTHORIZED |
30+
| `HookContext` | `session`, `user` | new `provenance.attributedUserId`, split off the context beside `session` |
31+
| better-auth ObjectQL adapter | `{ isSystem: true }` | `{ isSystem: true, attributedUserId }` when a request scope is open |
32+
| audit writer | `user_id = session.userId ?? null` | falls back to `provenance.attributedUserId` when the session names nobody |
33+
| `auto-org-admin-grant` | `granted_by: null`, no `reason` | the attributed human in `granted_by`, plus a machine-provenance `reason` naming the writer and the triggering `sys_member` row |
34+
35+
Outside a request scope nothing changes: writes stay bare `{ isSystem: true }`
36+
and audit rows keep recording `null`. Absence is still never upgraded into a
37+
caller, and never written as a sentinel string (ADR-0118 D1/D2).
38+
39+
**Hard constraint — attribution is not authority**
40+
41+
`attributedUserId` is read by exactly one consumer, the audit writer, and by no
42+
security middleware. It never becomes `ExecutionContext.userId`, so it is never
43+
the subject the engine authorizes as: not RLS `current_user`, not the ownership
44+
stamp, not permission resolution. A context carrying only `attributedUserId`
45+
authorizes exactly like an empty context (ANONYMOUS), and a context carrying it
46+
beside `isSystem: true` authorizes exactly like `isSystem` alone. Re-authorizing
47+
identity writes as the human would re-adjudicate a decision better-auth already
48+
made — the second adjudication track ADR-0095 D3 closed. The constraint is
49+
pinned by tests at three layers: the engine seam
50+
(`packages/objectql/src/engine.test.ts`), the better-auth adapter
51+
(`packages/plugins/plugin-auth/src/auth-actor-attribution.test.ts`), and the
52+
live HTTP route (a plain member still cannot promote themselves).
53+
54+
**For authors and plugin developers**
55+
56+
`attributedUserId` is authorable on `ExecutionContext` and readable as
57+
`ctx.provenance?.attributedUserId` in hooks. Use it to answer *who is
58+
responsible*; keep using `ctx.session` / `ctx.user` to decide *what is
59+
permitted*. The two are separate fields precisely so the distinction cannot be
60+
blurred by accident.
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
---
2+
"@objectstack/spec": major
3+
"@objectstack/driver-sql": major
4+
"@objectstack/driver-memory": major
5+
"@objectstack/driver-mongodb": major
6+
---
7+
8+
refactor(spec,drivers)!: retire `IDataDriver.findStream` — a required method with no caller, whose two main implementations did the opposite of what it promised (#4484, ADR-0049 enforce-or-remove)
9+
10+
`findStream` was a **required** method on the driver contract — every driver and
11+
every test double had to implement it — documented as the read
12+
13+
> Optimized for large datasets to avoid memory overflow.
14+
15+
Three things were true about it at once, and each is worse in the light of the
16+
others.
17+
18+
**Nothing called it.** Not the query engine (there is no `stream` entry on it),
19+
not REST export, not import, not any bulk-read path. Repo-wide, outside the
20+
contract declaration and the three driver implementations, every single hit was
21+
a test double — and roughly twenty of those satisfied the required method like
22+
this:
23+
24+
```ts
25+
findStream() { throw new Error('not implemented'); }
26+
```
27+
28+
Twenty stubs that throw, across four packages, for years, and no test ever went
29+
red. That is not an anecdote about test hygiene; it is the proof of absence. A
30+
method whose every double throws is a method nothing reaches.
31+
32+
**Two of the three implementations inverted its one guarantee.** `SqlDriver` and
33+
`InMemoryDriver` both did this:
34+
35+
```ts
36+
const results = await this.find(object, query, options); // ← the entire result set
37+
for (const row of results) yield row;
38+
```
39+
40+
The whole table is resident in memory before the first `yield`. A caller who
41+
believed the doc comment and reached for `findStream` precisely because a result
42+
set was too large would have hit the overflow it existed to prevent, at exactly
43+
the scale where it mattered. `SqlDriver` carried a `TODO: Use Knex .stream()`
44+
admitting it.
45+
46+
**The one real implementation dropped a parameter.** `MongoDBDriver._findStream`
47+
did walk a cursor — but it was the only read in that driver never routed through
48+
`buildFindOptions`, so it hardcoded `projection: { _id: 0 }` and silently
49+
discarded `query.fields`. (#4459 unified `find`/`findOne` onto `buildFindOptions`
50+
and recorded in its TSDoc that `_findStream` was left out. This removal subsumes
51+
that divergence rather than fixing it — there is nothing left to fix it for.)
52+
53+
Rather than manufacture a caller to justify three implementations, the method is
54+
retired. If a cursor-based read is wanted, it should arrive **with** the caller
55+
that needs it, so the contract can be shaped by a real requirement instead of
56+
being reverse-engineered from a doc comment nobody could test.
57+
58+
**Migration.**
59+
60+
| Wrote | Write instead |
61+
| --- | --- |
62+
| `for await (const row of driver.findStream(obj, q)) { … }` | page `driver.find(obj, { ...q, limit, offset })` in a loop |
63+
| `findStream(…) { … }` on your own driver | delete the method (see below) |
64+
| `findStream() { throw new Error('ni'); }` in a test double | delete the line |
65+
66+
Paging `find()` is not a downgrade from what `findStream` actually did: on SQL
67+
and memory it is strictly better (bounded pages instead of one full
68+
materialisation), and the paged read is the one with an **enforced** guarantee —
69+
`IDataDriver.find` requires a total order across the whole walk, checked by the
70+
shared `PAGINATION_CASES` / `PAGINATION_UNORDERED_CASES` fixtures in
71+
`data/pagination-conformance.ts`. `findStream` never had a conformance case at
72+
all.
73+
74+
**Driver authors: nothing breaks on you.** An implementation left in place still
75+
compiles — an extra method is not an error on a class or a widened object — it is
76+
simply never reached, so deleting it is cleanup you can do whenever. The break is
77+
on the **caller** side: `driver.findStream(...)` no longer type-checks, and there
78+
were no callers.
79+
80+
**No tombstone, deliberately.** The other v17 retirements tombstone their key so
81+
authoring it fails loudly with a prescription. That would be noise here.
82+
`DriverInterfaceSchema` describes a contract that code *implements*; nothing in
83+
either repository ever ran a driver object through `.parse()`, so a
84+
`retiredKey()` there would carry its prescription to no one. The channel that can
85+
carry it is `tsc`, and `tsc` reports it where it is actionable — at a call site.
86+
The key is removed from the schema and from `IDataDriver`, and the retirement is
87+
registered as the `data-driver-find-stream-retired` semantic entry in the
88+
protocol-17 chain step (ADR-0087 D3), so `spec-changes.json`, the generated
89+
upgrade guide and the `spec_changes` MCP tool all carry it. There is no
90+
`os migrate meta` step: a driver is code, never stack metadata, so the chain has
91+
no source to rewrite.
92+
93+
**Left standing on purpose:** `DriverCapabilities.streaming`, the capability flag
94+
whose only referent was this method. It has no readers either (and the values
95+
written into it were already wrong — `SqlDriver` declared `streaming: false`
96+
while implementing `findStream`, `InMemoryDriver` declared `true` for the
97+
copy-everything version), but removing a key from the capabilities literal breaks
98+
every driver that writes it, third-party included, and the same audit should
99+
cover the other ~30 flags in one pass rather than one at a time. Tracked as
100+
#4634.

.changeset/data-event-contract.md

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/objectql": minor
3+
"@objectstack/client": patch
4+
"@objectstack/plugin-webhooks": patch
5+
"@objectstack/service-knowledge": patch
6+
---
7+
8+
fix(objectql,client): `subscribeData` callbacks receive real `DataEvent`s — the producer now fulfils the declared contract (#4626)
9+
10+
`@objectstack/spec/api`'s `DataEvent` declares top-level `id` (uuid,
11+
required), `type`, `object`, `recordId` (required), `changes?`, `before?`,
12+
`after?`, `userId?`, `timestamp`. But the producer (the ObjectQL engine)
13+
published a raw `RealtimeEventPayload` envelope with `{ recordId, after,
14+
changes }` nested under `payload` and never generated `id`/`userId`, while the
15+
client SDK force-cast that envelope into the callback (`callback(event as any
16+
as DataEvent)`). Subscribers who wrote `event.recordId` / `event.changes` —
17+
exactly what the types promised — compiled green and read `undefined` at
18+
runtime. The data-side twin of #4602.
19+
20+
Producer now fulfils the contract:
21+
22+
- `ObjectQL.insert()` / `update()` / `delete()` build a true `DataEvent`
23+
(generated uuid `id`, flattened top-level fields, `userId` from the
24+
execution context when the write names an actor) and validate it with
25+
`DataEventSchema.parse` before publishing. The transport envelope is
26+
unchanged (`RealtimeEventPayload`, with `payload` carrying the complete
27+
`DataEvent`), so subscribers keep receiving `{ type, object, payload,
28+
timestamp }` on the wire.
29+
- A batch insert publishes one event **per record** (as before), each with its
30+
own event id.
31+
- **A multi-row write (`multi: true``updateMany` / `deleteMany`) now
32+
publishes nothing.** Those driver methods return only an affected count, so
33+
there is no record for a required `recordId` to name; the engine logs a
34+
warning naming the gap instead of publishing the previous fabrication
35+
(`recordId: ''`, `after: <affected count>`), which every schema-compliant
36+
consumer had to reject. **Consequence: webhooks and knowledge sync no longer
37+
fire for bulk writes** — they previously fired once with an unusable body. A
38+
real bulk event contract is tracked in #4639.
39+
40+
Consumers validate or read the fulfilled shape instead of guessing:
41+
42+
- `@objectstack/client`'s `subscribeData` (and therefore
43+
`@objectstack/client-react`'s `useDataSubscription` /
44+
`useDataSubscriptionCallback` / `useAutoRefresh`, which delegate to it)
45+
unwraps the envelope and runs `DataEventSchema.safeParse` at the boundary.
46+
An off-contract payload is rejected loudly (handler error, callback never
47+
invoked) — never coerced or passed through. The `as any as DataEvent`
48+
double-cast is gone, and the `recordId` option now filters on the fulfilled
49+
event.
50+
- `@objectstack/plugin-webhooks`' auto-enqueuer reads the required
51+
`recordId` directly; its `recordId ?? id ?? after?.id ?? before?.id ??
52+
'unknown'` fallback chain is gone, and an off-contract event is dropped with
53+
a warning rather than delivered under the literal id `'unknown'`. Delivered
54+
webhook bodies now also carry the event's `id`/`type`/`userId`; the record
55+
itself stays nested under `after` and the envelope keys (`object`,
56+
`recordId`, `action`, `timestamp`) still win.
57+
- `@objectstack/service-knowledge`'s event sync reads the record from `after`
58+
(create/update) and the id from `recordId` (delete) for `data.record.*`.
59+
It previously indexed the envelope itself as if it were the row, and never
60+
resolved an id for deletes.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
---
3+
4+
test(drivers): a conformance run that discovers zero drivers is a failure, not an OK (#4646)
5+
6+
`scripts/check-driver-conformance.mjs` discovers driver packages from disk under a
7+
hardcoded `DRIVERS_DIR`. `listDir` swallows ENOENT and returns `[]`, and all three
8+
invariants iterate the discovered set — CONSUMED over `drivers`, RECONCILED over
9+
`LEDGER` (empty since #4405, the intended steady state), CLASSIFIED not over drivers
10+
at all. So a stale `DRIVERS_DIR` produced `OK — 0 covered cell(s)` and exit 0.
11+
12+
CI never had this exposure: `lint.yml` runs `pnpm check:driver-conformance`, which is
13+
`--self-test && audit`, and the self-test carried a driver-discovery assertion. The
14+
false green was on the bare `node scripts/check-driver-conformance.mjs` the script's
15+
own header documents as a usage.
16+
17+
Two things were wrong with leaving the guard there. It read
18+
`drivers.length >= 3 && drivers.includes('driver-sql')` — a hardcoded name and count
19+
inside the one script whose stated rule is that drivers come from disk and are never
20+
listed, so both needed hand-editing on the next driver added or package moved, which
21+
is precisely when the guard earns its keep. And its failure text ("discovers driver
22+
packages from disk") named neither `DRIVERS_DIR` nor the stale path, leaving whoever
23+
tripped it to find that themselves.
24+
25+
DISCOVERED is now a fourth invariant in `audit()`, and the message names the directory
26+
it searched. The self-test drives the invariant in both directions instead of standing
27+
in for it, and asserts nothing about which drivers exist.
28+
29+
The case-set axis cannot rot this way and is left alone: `CASE_SETS` is a declared
30+
expectation, so a vanished `spec/src/data` fails CLASSIFIED's reverse direction with
31+
one error per case-set. The driver axis is disk-discovery with nothing declared to
32+
reconcile against — that asymmetry is why zero was reachable on one axis and not the
33+
other, and it is what DISCOVERED supplies.

0 commit comments

Comments
 (0)