Skip to content

Commit 7d323fa

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/pagination-tie-breaker-missing-p98qf0
2 parents 711cde7 + 7d80695 commit 7d323fa

25 files changed

Lines changed: 568 additions & 77 deletions

.changeset/action-body-write-set-lint.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ that carries one. An action body is the same artefact: the same
1010
`HookBodySchema` union, parsed by the same `HookBodySchema.safeParse` in
1111
`actionBodyRunnerFactory`, run in the same QuickJS sandbox. So it fails the
1212
same way — `ctx.api.object('crm_deal').update({ stag: 'won' })` inside an
13-
action succeeds, returns success to the caller, and the unknown column simply
14-
never lands. Half the surface was still blind.
13+
action reaches the driver unfiltered, and the outcome splits by driver: on SQL
14+
the stray column fails the whole call with a driver-level error far from the
15+
authoring site, and on a schemaless driver the stray key is persisted. Half
16+
the surface was still blind.
1517

1618
**New rule — `action-body-write-unknown-field` (advisory).** Wired into
1719
`REFERENCE_INTEGRITY_RULES`, so `os validate`, `os lint` and `os compile` all
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/lint": patch
3+
"@objectstack/spec": patch
4+
---
5+
6+
fix(lint): the write-set diagnostics describe what the runtime actually does (#4271)
7+
8+
`hook-body-write-unknown-field` and `action-body-write-unknown-field` told
9+
authors the undeclared column "silently never lands in the stored record".
10+
Measured on `main`, that is wrong in **both** directions. Nothing between the
11+
body and the driver filters the key — `applyMutationsToInput` is a plain
12+
`Object.assign`, and `validateRecord` walks declared fields on insert and
13+
`continue`s past a key with no field def on update — so the driver decides:
14+
15+
- **SQL** — the stray column enters the statement and the **whole write
16+
fails** with a driver-level error (`table deal has no column named stagee`).
17+
Nothing is stored, so the correctly-spelled fields of that row are lost too,
18+
and the error names a column far from the body that wrote it.
19+
- **Schemaless** (memory, MongoDB — both spread the payload without consulting
20+
the declared field set) — the stray key **is** persisted, as an undeclared
21+
column nothing downstream reads.
22+
23+
A lint that misdescribes the failure it is warning about teaches the wrong
24+
debugging instinct: an author told the value silently vanishes will not connect
25+
the driver error they actually see to the typo that caused it, and on a
26+
schemaless driver will not go looking for the stray key that is really there.
27+
All three messages now state the split, matching the "What still happens at
28+
runtime" description #4355 gave `content/docs/automation/hook-bodies.mdx`.
29+
30+
Both outcomes are pinned by a new integration test —
31+
`runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts`.
32+
Its insert cases run the full chain (real QuickJS sandbox, real hook body, real
33+
engine, real driver against a real SQLite table), so "reaches the driver
34+
unfiltered" is proved rather than asserted: if anything on that path ever
35+
learns to filter, the SQL half stops throwing and the test goes red. The rule
36+
headers, the `ScriptBodySchema` / `ActionSchema.body` notes and the two
37+
still-unreleased #4271 changesets are corrected to match. #4355 fixed the
38+
prose docs; this is the same correction on the surfaces that ship in the
39+
packages — the diagnostic an author actually reads, and a test that pins it.
40+
41+
`@objectstack/spec`: doc comments only — no schema or generated-artifact change.

.changeset/hook-body-write-set-lint.md

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,23 @@ feat(lint): L2 hook-body writes to undeclared fields warn at author time (#4271)
77

88
An L2 (`language:'js'`) hook body that writes a field the target object never
99
declares — `ctx.input.amout = 0`, `ctx.api.object('deal').update({ stag: … })`
10-
— runs clean in the QuickJS sandbox, reports success, and the unknown column
11-
simply never lands in the stored record. No diagnostic anywhere: the #4001
12-
"silent no-op manufactures false completion" failure mode at the
13-
runtime-expression layer. The read side (`hook.condition`) and the capability
14-
surface were already statically checked; the write side was the one blind face,
15-
and `hook-body.zod.ts` carried it as an **accepted gap**.
10+
— runs clean in the QuickJS sandbox and reaches the driver **unfiltered**:
11+
`applyMutationsToInput` is a plain `Object.assign`, and the write-path
12+
validator walks declared fields on insert and skips a key it has no field def
13+
for on update. What happens next depends on the driver, and neither half is
14+
acceptable:
15+
16+
- **SQL** — the stray column enters the statement and the **whole write fails**
17+
with a driver-level error (`table deal has no column named stagee`). The
18+
write is lost, and the error surfaces far from the mistake that caused it.
19+
- **Schemaless** (memory, MongoDB) — the driver spreads the payload, so the
20+
stray key **is** persisted: an undeclared column nothing downstream reads.
21+
22+
No diagnostic anywhere, and nothing at the authoring site either way — the
23+
#4001 "the mistake is invisible where it is made" family. The read side
24+
(`hook.condition`) and the capability surface were already statically checked;
25+
the write side was the one blind face, and `hook-body.zod.ts` carried it as an
26+
**accepted gap**.
1627

1728
**New rule — `hook-body-write-unknown-field` (advisory).** `@objectstack/lint`
1829
now parses each L2 body (TypeScript parser; parsed, never executed, never
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/metadata": patch
3+
---
4+
5+
fix(lint,metadata): revoke the `http.server` lint exemption — its stated reason was false (#4251)
6+
7+
`http.server` was added to `UNCONTRACTED_SLOTS` in #4321 on the ground that
8+
"no IHttpServer contract exists". The contract does exist —
9+
`packages/spec/src/contracts/http-server.ts` — and eight call sites were
10+
already resolving the slot as `getService<IHttpServer>(…)` when the exemption
11+
was written. An exemption is a claim like any other, and this one rested on a
12+
premise nobody checked: the same shape as the gaps the rule exists to find.
13+
14+
Revoked. That surfaced **9 erasures the exemption had been hiding** — 7 in
15+
files never grandfathered, 2 as count growth inside grandfathered ones, none of
16+
which the baseline could legally absorb. All typed to `IHttpServer`;
17+
`packages/metadata/src/plugin.ts` came out clean entirely, so the baseline
18+
ratchets **DOWN to 168 sites in 36 files** and loses a file.
19+
20+
Two things confirmed on the way, reported rather than changed:
21+
22+
**`http.server` and `http-server` are the same instance under two names.**
23+
plugin-hono-server and qa's node-plugin each register it twice, two lines
24+
apart; runtime's `config.server` path registers only `http.server`.
25+
`metadata/src/plugin.ts` reads both with a `??`, which is how it survived. No
26+
registration is removed here — that is a runtime-behaviour change and belongs
27+
with whoever picks the canonical name.
28+
29+
**`IHttpServer` is defined twice and the two have already diverged.**
30+
`packages/spec/src/contracts/http-server.ts` (15 importers) declares `write?()`
31+
and `end?()`; `packages/core/src/contracts/http-server.ts` (8 importers) does
32+
not. Spec's is the superset and the one the ledger points at, so it is the
33+
source; core's is a stale near-copy and should re-export it. Left for its own
34+
change — collapsing a duplicated contract is not a lint fix.
35+
36+
Also worth a note for whoever writes the wider HTTP contract: `getRawApp()` now
37+
has a **third** independent consumer (metadata's HMR routes, joining
38+
cloud-connection's two). It is deliberately absent from `IHttpServer` — the
39+
contract is framework-agnostic and the raw app is the framework's own handle —
40+
so each consumer names it locally. Three is enough evidence to decide whether
41+
that stays the right answer.
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
fix(lint): an object declaring no fields is unjudgeable, not "has no such field" (#4383)
6+
7+
`hook-body-write-unknown-field` and `action-body-write-unknown-field` reported
8+
**every** field write to an object that declares no `fields` — an external
9+
object, or a datasource-introspected schema whose columns are resolved at
10+
runtime. Measured before the fix:
11+
12+
```
13+
hook : ["hook-body-write-unknown-field / warning"] ← false
14+
action: ["action-body-write-unknown-field / warning"] ← false
15+
flow : [] ← correct
16+
```
17+
18+
`indexObjectFields` returns an **empty Set** for such an object rather than
19+
`undefined`, and both rules only asked "is this object in the stack?" —
20+
`targetSets.every((s) => s !== undefined)` and `if (!known) continue`. An empty
21+
Set is neither undefined nor falsy, so it became the answer to `has(field)`,
22+
and the answer is always `false`.
23+
24+
That field map is not empty, it is **unknown**. The distinction already existed
25+
in two other rules of the same family, each with its reason written down —
26+
`validate-searchable-fields` skip #2 and `validate-flow-node-writes` (#4369,
27+
which added the guard because it gates). Two of four had it; the drift shape
28+
#3583 and #4330 exist to remove.
29+
30+
**Fixed once, not twice.** The guard now lives in a shared
31+
`judgeableFieldsOf(index, objectName)` that returns the declared names only when
32+
they are a sound basis for a "resolves to nothing" judgement, and `undefined`
33+
for both unjudgeable cases — cross-package objects and fields-less ones. All
34+
three write-set rules route their lookups through it, so a fourth cannot repeat
35+
the omission. It is internal to the family (not re-exported from the package
36+
barrel), same as `indexObjectFields` and `IMPLICIT_FIELDS`.
37+
38+
One semantic call worth naming: a **multi-target** hook where only *some*
39+
targets are judgeable is now skipped entirely. The `ctx.input` finding fires
40+
only when a field is missing from EVERY target, and an unjudgeable target is one
41+
the field might well exist on — so judging the remainder would assert "missing
42+
everywhere" on evidence that does not cover everywhere. Consistent with the
43+
rule's stated asymmetry: prefer a missed finding to a false one.
44+
45+
No behaviour change for objects that declare fields: an unknown field on a
46+
normal object still warns exactly as before, pinned by a test placed next to
47+
each new skip so the guard cannot swallow the real finding.

eslint.config.mjs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,16 @@ const SLOT_LOOKUPS = ['resolveService', 'getService', 'getRequestKernelService']
6969
// place, not sprinkled through the code. Deleting a name from this list is how
7070
// the exemption ends once that contract gets written.
7171
//
72-
// Entries are spliced into a regex, so escape metacharacters (`http\\.server`).
73-
// `http.server` is served by three providers (plugin-hono-server, runtime's
74-
// config.server path, qa's node-plugin) and no IHttpServer contract exists;
75-
// callers read only `getPort()`.
76-
const UNCONTRACTED_SLOTS = ['protocol', 'mcp', 'kernel-resolver', 'scope-manager', 'http\\.server'].join('|');
72+
// Entries are spliced into a regex, so escape metacharacters if one returns.
73+
//
74+
// [#4251] `http.server` was here, on the stated ground that "no IHttpServer
75+
// contract exists". That was FALSE when it was written: the contract is
76+
// `packages/spec/src/contracts/http-server.ts`, and eight call sites were
77+
// already resolving the slot as `getService<IHttpServer>(…)`. An exemption is a
78+
// claim like any other, and this one rested on a premise nobody checked — the
79+
// same shape as the gaps this rule exists to find. Revoked; the slot is
80+
// accounted for like every other contracted slot.
81+
const UNCONTRACTED_SLOTS = ['protocol', 'mcp', 'kernel-resolver', 'scope-manager'].join('|');
7782

7883
// Exported so `scripts/check-slot-lookup-ratchet.mjs` can identify THIS rule's
7984
// reports among the other `no-restricted-syntax` rules, by exact message —

packages/client/src/client.batch-transaction.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
3333
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
3434
import { createRestApiPlugin } from '@objectstack/runtime';
3535
import { ObjectStackClient } from './index';
36+
import type { IHttpServer } from '@objectstack/spec/contracts';
3637

3738
describe('data.batchTransaction (live Hono, #1604)', () => {
3839
let baseUrl: string;
@@ -103,8 +104,8 @@ describe('data.batchTransaction (live Hono, #1604)', () => {
103104
await ql.syncObjectSchema('project');
104105
await ql.syncObjectSchema('task');
105106

106-
const httpServer = kernel.getService<any>('http.server');
107-
baseUrl = `http://localhost:${httpServer.getPort()}`;
107+
const httpServer = kernel.getService<IHttpServer>('http.server');
108+
baseUrl = `http://localhost:${httpServer.getPort!()}`;
108109
client = new ObjectStackClient({ baseUrl });
109110
}, 30_000);
110111

packages/client/src/client.environment-scoping.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
2020
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
2121
import { createRestApiPlugin } from '@objectstack/runtime';
2222
import { ObjectStackClient } from './index';
23+
import type { IHttpServer } from '@objectstack/spec/contracts';
2324

2425
describe('Project-scoped REST routing (live Hono)', () => {
2526
let baseUrl: string;
@@ -82,8 +83,8 @@ describe('Project-scoped REST routing (live Hono)', () => {
8283
// write fails with `no such table`.
8384
await ql.syncObjectSchema('task');
8485

85-
const httpServer = kernel.getService<any>('http.server');
86-
const port = httpServer.getPort();
86+
const httpServer = kernel.getService<IHttpServer>('http.server');
87+
const port = httpServer.getPort!();
8788
baseUrl = `http://localhost:${port}`;
8889
}, 30_000);
8990

packages/client/src/client.hono.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
55
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
66
import { createRestApiPlugin } from '@objectstack/runtime';
77
import { ObjectStackClient } from './index';
8+
import type { IHttpServer } from '@objectstack/spec/contracts';
89

910
describe('ObjectStackClient (with Hono Server)', () => {
1011
let baseUrl: string;
@@ -130,8 +131,8 @@ describe('ObjectStackClient (with Hono Server)', () => {
130131
await ql.syncObjectSchema('customer');
131132

132133
// 5. Get Port from Service
133-
const httpServer = kernel.getService<any>('http.server');
134-
const port = httpServer.getPort();
134+
const httpServer = kernel.getService<IHttpServer>('http.server');
135+
const port = httpServer.getPort!();
135136
baseUrl = `http://localhost:${port}`;
136137

137138
console.log(`Test server running at ${baseUrl}`);

packages/lint/src/reference-integrity-suite.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,9 @@ describe('reference-integrity suite — every member actually runs', () => {
6363
// validateObjectReferences: a param pointing at an object nothing declares.
6464
{ name: 'assign', label: 'Assign', params: [{ name: 'owner', reference: 'user' }] },
6565
// validateActionBodyWrites, both of its rule ids from one body:
66-
// - the L2 body persists a field crm_lead does not declare — the action
67-
// returns success and the column never lands (#4271);
66+
// - the L2 body writes a field crm_lead does not declare — on SQL that
67+
// fails the whole call at the driver, on a schemaless driver it
68+
// persists a stray key (#4271);
6869
// - and it assigns ctx.record, a snapshot the runtime never writes
6970
// back, without ever passing it anywhere (#4345).
7071
{
@@ -143,7 +144,8 @@ describe('reference-integrity suite — every member actually runs', () => {
143144
skills: [{ name: 'metadata_authoring', surface: 'build', tools: ['forecast_revenue'] }],
144145
hooks: [
145146
// validateHookBodyWrites: the L2 body writes a field crm_lead does not
146-
// declare — runs clean in the sandbox, never lands in the record (#4271).
147+
// declare — runs clean in the sandbox, then fails the whole write at a
148+
// SQL driver / persists a stray key on a schemaless one (#4271).
147149
{
148150
name: 'score_lead',
149151
object: 'crm_lead',

0 commit comments

Comments
 (0)