Skip to content

Commit fd79a12

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-16091-text-column-unbounded
2 parents 722880a + f377394 commit fd79a12

182 files changed

Lines changed: 9765 additions & 939 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: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@objectstack/client": minor
3+
---
4+
5+
fix(client)!: `oauth.applications.delete` resolves on the zero-byte 200 its route answers, instead of rejecting on every successful delete (#15451)
6+
7+
**BREAKING** on two independent axes, and it makes a published method usable for the first time. Before this change `client.oauth.applications.delete(id)` **rejected on every successful delete** — there was no success path a caller could observe. It ships as `minor` under the lockstep launch-window convention (`scripts/check-changeset-no-major.mjs`); the version number is not the migration signal here, this entry is.
8+
9+
<!-- adr-0087: registered client-oauth-applications-delete-void -->
10+
11+
The fifth and last method of the `oauth.*` family, and the one #14312 / PR #15445 deliberately could not close: its ruling fenced that card to *narrowing published return types*, and no declared return type could be true while the `res.json()` call stood.
12+
13+
## The defect, measured end to end
14+
15+
Real `betterAuth` + real `@better-auth/oauth-provider` over the real ObjectQL adapter on real SQLite, a real signed-up user and a real session, driven through the **real** `ObjectStackClient` with only the socket stood in for:
16+
17+
```
18+
POST /api/v1/auth/oauth2/delete-client -> 200 · 0 bytes
19+
content-type: application/json
20+
content-length: (absent)
21+
through the client, BEFORE -> REJECTED: SyntaxError | Unexpected end of JSON input
22+
the row, server-side -> ALREADY GONE (get-client answers 404 not_found)
23+
through the client, AFTER -> RESOLVED | undefined
24+
```
25+
26+
The handler returns nothing and the vendor declares the endpoint `void`. `res.json()` had nothing to parse, so the method rejected — *after* the delete had committed. A caller who did the obvious thing saw a failure, retried, and the retry failed **differently**, because the row no longer existed.
27+
28+
## What changes for a caller
29+
30+
| | before | now |
31+
|:--|:--|:--|
32+
| a successful delete | rejects `SyntaxError` | resolves |
33+
| the resolved value | `any` (unreachable — the promise never resolved) | `void` |
34+
| deleting a client that is not there | rejects `not_found` | rejects `not_found` — unchanged |
35+
| a malformed non-empty body | rejects `SyntaxError` | rejects `SyntaxError` — unchanged |
36+
37+
⚠️ **The `catch` you wrote around this call stops firing on success.** Code shaped like
38+
39+
```ts
40+
try { await client.oauth.applications.delete(id); }
41+
catch { /* the delete probably worked anyway */ }
42+
```
43+
44+
still compiles and still runs, but its catch block was executing on **every** successful delete and now executes only on a real failure. Any workaround that lived in there is now inert and can be deleted. And because the promise never used to resolve, a read off its resolved value — `(await …delete(id)).deleted` — was dead code that has never executed; it now stops compiling (TS2339), which is the compiler delivering the change at the call site.
45+
46+
## Why `void`, and not `{ deleted: boolean }`
47+
48+
"Deleted" and "was already gone" **are** distinguished by the route, but on the error channel: a missing client answers 404 `{ error: 'not_found' }`, which the client already raises as a throw. The 200 answer carries zero bytes and therefore zero information, so a synthesised `{ deleted: true }` would be a shape the wire never sends and strictly less informative than the 404 a caller already receives.
49+
50+
## Why the emptiness is detected by reading the body
51+
52+
Both shortcuts were measured against the real route and both are unusable: the status is **200**, not the `204` five other delete surfaces in this client key off, and the response carries **no `content-length` header at all** — so a header test would never fire and would leave the defect in place while looking like a fix. The body itself is the only thing that answers.
53+
54+
A non-empty body is still parsed and its failure still thrown, so **the only behaviour this change moves is the zero-byte case**: a malformed response stays loud, and the day this route grows a payload, surfacing it is a deliberate widening of the return type rather than a silent change of shape.
55+
56+
`packages/client/exported-any-returns.json` loses this method's entry in the same change — the ledger is shrink-only, so the entry goes **with** the binding. Its last `oauth.*` entry is now gone; 35 sites remain open.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
Record, on `DeleteDataRequestSchema` itself, what it is for and why the DELETE data door carries no `requestSchema` for it.
6+
7+
The schema is the request contract of `DataProtocol.deleteData()`, consumed statically through the `DeleteDataRequest` type alias and parsed at runtime nowhere — a grep that finds "exported, documented, zero `safeParse` call sites" is reading the wrong surface, and had already filed it once as a gap. Its docblock now says so; records that the absence of a `requestSchema` on `DELETE /api/v1/data/:object/:id` is a pinned decision (#3899 — the catalog entry states it in place of the key, and `plugin-rest-api.schema-refs.test.ts` goes red if one is added, because the route reads no body); and points at the compile-time check (#15866) under which a field added to the schema as required reddens the door at build instead of being silently unsent.
8+
9+
Documentation only: no shape, `.describe()` text, or export changes. `@objectstack/spec` ships the new text in its published type declarations and in the source file it publishes directly via its `src/**/*.zod.ts` entry.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
MySQL: a plain unique index over existing duplicate rows no longer takes the boot down when the index has to be carried by a hash shadow.
6+
7+
`syncDeclaredIndexes` handles a declared unique that the database refuses in one `catch`, and that `catch` has two arms: the DIRECT one, and the hash-shadow one MySQL takes when a key part is wider than the 768-char utf8mb4 ceiling. #14902 taught the direct arm that a uniqueness violation over existing rows is a durability degradation rather than a fatal — log it, name the conflicting rows and the remedy, let the boot continue. The shadow arm kept the older guard, which also required a NULL-safe organization key part, so a PLAIN unique (`tenancy: { enabled: false }`, or an explicit `unique: 'global'`) matched neither branch.
8+
9+
Measured on live MySQL 8.0.46: the boot died carrying `ER_BLOB_KEY_WITHOUT_LENGTH` — a refusal about an unkeyable TEXT column, telling the operator to declare a `maxLength` the field already declared — while the real cause was two duplicate rows it never mentioned. It named no rows and no remedy.
10+
11+
The two arms now agree, and they say different things because they mean different things. The NULL-safe arm keeps its wording (existing rows violate the NULL-safe key, duplicating what the previous void constraint admitted); the plain arm gets the direct arm's reviewed sentence, because neither of those clauses is true of a plain unique — nothing admitted the rows, and there is no NULL-safe key. Widening the guard alone would have shipped a factually false durability log, which is worse than the throw it replaces.
12+
13+
`os migrate plan` already reported this operation as `destructive` with the row report and is unchanged.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
'@objectstack/lint': patch
3+
---
4+
5+
Fix: a name-keyed `pages:` map no longer passes every source-page lint vacuously.
6+
7+
`pages` has two authoring carriers — a list, or a map keyed by page name that
8+
`normalizeStackInput` folds into a list before the schema sees it. Four rules
9+
(`validate-jsx-pages`, `validate-page-source-styling`,
10+
`validate-react-page-props`, `validate-react-pages`) read the collection through
11+
a private coercion that answered a map with an empty list, and they run on the
12+
raw `os lint` path where nothing has normalized it yet. On a map-shaped stack
13+
all four therefore returned no findings by never walking a single page: an
14+
empty source, a syntax error, an unparseable component and a Tailwind
15+
`className` were all reported as clean. They now read `collectionEntries`,
16+
which handles both carriers, and a finding on the map carrier is located by the
17+
author's own key (`pages.home.source`) rather than a synthetic index.
18+
19+
The same change removes the last sixteen private copies of the collection
20+
coercion in this package. Twelve rules — the `function` form, which had already
21+
grown the non-record filter locally in two different spellings — now read
22+
`recordsOf` from `object-graph.ts`. Two behaviour changes fall out, both on
23+
input that was already malformed: an array-typed member of `agents:` /
24+
`skills:` / `tools:` used to survive the looser local filter and draw one
25+
reference-integrity finding at a position nobody authored, and is now dropped;
26+
a member of a name-keyed `validations:` map whose value is not a record is now
27+
carried as `{ name }` rather than discarded, which reaches no check that reads
28+
it. No rule id, message or severity changes, and every finding path on the list
29+
carrier is unchanged.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
New advisory rule `field-no-consumers` (`validateFieldConsumers`): a field declared on an object that nothing in the stack reads or displays is reported as a `warning` by `os validate`, `os build` and `os lint`.
6+
7+
Until now such a field was schema-valid and passed every platform check — the declaration was inert and nothing in the toolchain said so. The rule is object-aware (the same field name on two objects gets two verdicts, resolved against the object whose declaration encloses each reference), and it distinguishes consumers from carriers: a view column, form section, page binding, flow node, dataset dimension, widget filter, formula, validation, hook or action is a consumer; a translation label, a seed value, an import-mapping column, a field-level permission grant or a flow that only writes the field is a carrier and never counts. The finding carries the verdict (`carrier-only` with the carrier paths a removal must clean, or `inert`), the roots scanned, and — when the name is also declared elsewhere — the other objects, so a per-object verdict is never mistaken for a name-level one.
8+
9+
Exempt, each derived from the spec rather than listed by hand: the registry-injected system columns an author re-declared, the record's title field (ADR-0079 `nameField` ladder), and `master_detail` fields (ADR-0035 — cascade delete, `controlled_by_parent` sharing and roll-ups read the relationship by declaration). A stack that declares no consumer root at all (objects only, or objects plus carriers) is not judged: its consumers live in another package. Test fixtures are never scanned.
10+
11+
Public surface: `validateFieldConsumers`, `FIELD_NO_CONSUMERS`, `FIELD_CONSUMER_ROOTS`, `FIELD_CARRIER_ROOTS`, and the `FieldConsumerFinding` / `FieldConsumerVerdict` / `FieldConsumerSeverity` types.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
`os lint` no longer crashes on a localized label.
6+
7+
`convention/label-case` indexed its argument (`label[0].toUpperCase()`) on a parameter annotated `string`, while every call site reaches it through `any`-typed config walking and the spec does not require a label to be a string: `I18nLabelSchema` is `z.union([z.string(), InlineLocaleMapSchema])`. On the map form `label[0]` is `undefined`, the rule threw a `TypeError`, and the throw escaped `lintConfig` into the command's catch-all — so an author who localized an app label or a list-view label could not lint the project at all. Every face exited 1 with `Cannot read properties of undefined (reading 'toUpperCase')`, naming no rule, no path and no remedy, on input `ObjectStackDefinitionSchema` parses clean.
8+
9+
The rule now checks `typeof label === 'string'` first. Two of the four carriers it walks accept the inline locale map — `apps[].label` (`AppSchema`) and a view's `list` / `listViews.*` labels (`ListViewShapeSchema`); the other two are `z.string()` and reject the map at the schema door (`objects[].label`, `objects[].fields.*.label`).
10+
11+
**Nothing about a plain string label moves.** Same warning, same message, same `fix`, same path, on all four carriers — that is pinned per carrier rather than asserted.
12+
13+
**The rule deliberately says nothing about a localized label**, rather than resolving the map and case-checking one of its entries. Case is a property of a literal, and deciding which locale entry a case verdict is taken against is a product call, not a lint call. Widening the rule that way is a separate change.
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/rest": minor
4+
---
5+
6+
feat(spec): every `metadata.endpoints.*` switch gates exactly the face its name states, and the whole-store operations get their own key `maintenance` (#15542, #15854)
7+
8+
`RestServerConfig.metadata.endpoints` declared three switches, each `describe()` naming
9+
exactly one route, and each gated a different set. The mismatch ran in **both**
10+
directions at once:
11+
12+
- **`items`** — declared "GET /meta/:type - List items of type" — also gated the
13+
whole-store family: the cross-type spec-validation sweep `GET /meta/diagnostics`, the
14+
draft list `GET /meta/_drafts`, and the **`POST /meta/_migrate-stored` write door**.
15+
An operator who switched off a listing read they considered chatty silently unmounted
16+
a migration door.
17+
- **`item`** — declared "GET /meta/:type/:name - Get specific item" — gated four
18+
*reads* (`/:type/:name`, `/references`, `/layers`, `/book/:name/tree`) and left the
19+
per-item **writes** `PUT` and `DELETE /meta/:type/:name` plus the whole history family
20+
(`/history`, `/audit`, `/diff`, `/published`, `/publish`, `/rollback`) answering to
21+
`api.enableMetadata` alone. An operator who closed the per-item surface left its
22+
writes mounted.
23+
24+
Neither is a liveness defect — all three keys were genuinely read — which is why no
25+
ADR-0049 census could ever flag them: what drifted was each key's **radius** against its
26+
own documentation.
27+
28+
**One principle now holds across the block: a switch gates exactly the face its name
29+
states, reads and writes alike.**
30+
31+
| key | mounts it gates (default prefix `/meta`) |
32+
|---|---|
33+
| `types` | `GET /meta`, `GET /meta/types` — one handler, two paths (unchanged) |
34+
| `items` | `GET /meta/:type` — and nothing else |
35+
| `item` | `GET` / `PUT` / `DELETE /meta/:type/:name`, `/references`, `/layers`, `/history`, `/audit`, `/diff`, `/published`, `/publish`, `/rollback`, and `GET /meta/book/:name/tree` |
36+
| `maintenance` | **new**`GET /meta/diagnostics`, `GET /meta/_drafts`, `POST /meta/_migrate-stored` |
37+
38+
All four `describe()` strings are rewritten to enumerate what they gate, so the
39+
generated reference page is the radius rather than a sample of it.
40+
`api.enableMetadata` remains the master switch above all four, and
41+
`GET /meta/object/:name/state/:field` — the object FSM read, addressed by object name
42+
rather than by `:type/:name` — deliberately stays under that master switch alone.
43+
44+
**BREAKING** — for a programmatic embedder that authors `RestServerConfig.metadata.endpoints`,
45+
the mounted route table moves for two of the four keys, in opposite directions:
46+
47+
- **`items: false` now removes one route instead of four.** An embedder relying on it to
48+
close `/diagnostics`, `/_drafts` and the `POST /_migrate-stored` door **regains all
49+
three** unless it also sets `maintenance: false`. That is a write door coming back, so
50+
it is the half to read twice. One line restores the old table:
51+
`endpoints: { items: false, maintenance: false }`.
52+
- **`item: false` now removes twelve routes instead of four.** An embedder relying on it
53+
to close only the per-item *reads* while keeping `PUT`, `DELETE` and the history family
54+
mounted **loses those eight**. There is no key that restores them — the per-item face is
55+
one face by this ruling — so an embedder that wants the writes keeps `item` on and
56+
closes the surface at `api.enableMetadata` or at the object's own `enable.apiMethods`.
57+
- **The exported type `MetadataEndpointsConfigParsed` narrows: `endpoints` gains a
58+
REQUIRED member `maintenance: boolean`.** `maintenance` is `z.boolean().default(true)`,
59+
so it is optional on the way *in* and always present on the way *out* — and
60+
`MetadataEndpointsConfigParsed` is `z.infer<typeof MetadataEndpointsConfigSchema>`, the
61+
OUTPUT side. Any code that builds one of these objects by hand — a test fixture, a
62+
helper returning the parsed shape, a `satisfies MetadataEndpointsConfigParsed` literal —
63+
stops compiling with `TS2741: Property 'maintenance' is missing`. This one IS
64+
compiler-carried (the ADR-0087 D8 class), which is the good case: the break is loud, it
65+
lands at build time, and no runtime behaviour depends on the author noticing a
66+
changelog. Add `maintenance: true` to restore the previous mounts, or `false` to keep
67+
the whole-store family closed. In-repo consumers of the type: none — the narrowing was
68+
measured against a probe compiled from the rebuilt declaration, not assumed.
69+
70+
Priced and accepted rather than deferred: `RestServerConfig` is reachable from **no
71+
shipped boot path** today (`os serve` fixes the config and the dev plugin passes none,
72+
#15543), so the measured population of affected authors is **zero** and the blast radius
73+
is programmatic embedders only. That is precisely why this lands now — once a boot path
74+
starts authoring the config, the same change becomes a behaviour change on live
75+
operators.
76+
77+
**ADR-0087 disposition: a D3 semantic migration, no D2 conversion.** No
78+
authored key changes shape or spelling — `items: false` still parses to `items: false`,
79+
`maintenance` is additive with `.default(true)`, and nothing is retired (`endpoints.schema`
80+
stays the #14691 tombstone it already was). There is nothing for the conversion layer to
81+
convert: a `RestServerConfig` is plugin TS configuration, never a stack collection member
82+
and never a `sys_metadata` row (the `RestServerConfig.openApi31` precedent, #4579), so no
83+
rehydration seam sees it. What changes is a mounted route table at construction time.
84+
85+
Nor is the RADIUS change compiler-carried on the AUTHORED side — and that is the half a
86+
D3 is owed for. Every authored key is an optional boolean, so `{ items: false }` still
87+
compiles and still parses and simply mounts a different table: the author is told
88+
nothing. (The parsed-type narrowing in the third BREAKING bullet above *is*
89+
compiler-carried, but it catches only code that hand-builds the OUTPUT type — it cannot
90+
reach the embedder who authored `{ items: false }` and now silently gets three routes
91+
back.) So for the change that actually moves the route table, both channels that would
92+
otherwise reach a consumer are blind, which is precisely the residue D3 exists for — the prescription is registered as
93+
`metadata-endpoints-switch-radius-repartitioned` so `objectstack migrate meta` hands
94+
it to an upgrading embedder instead of leaving it as prose in a changelog.
95+
96+
<!-- adr-0087: registered metadata-endpoints-switch-radius-repartitioned -->
97+
98+
`@objectstack/rest` is versioned alongside rather than as a passive consumer: it is where
99+
the gates live, so the route-table change is observable there and not only in the
100+
declaration.
101+
102+
Every key's radius is pinned route by route, in both directions, in
103+
`packages/rest/src/rest-config-mount-table.pin.test.ts` — the #15544 shape, which asserts
104+
each route is **absent from the mounted table** when its switch is off rather than what
105+
the switch normalizes to. A gate that grows or loses a route reddens there.

0 commit comments

Comments
 (0)