Skip to content

Commit 804192a

Browse files
committed
Merge origin/main into claude/issue-16751-flow-node-list-recordsof
One conflict, in `packages/lint/src/non-record-object-entry.test.ts`'s `RESIDUAL_THROWS`. Both sides emptied it from opposite ends: `main` (the producer repair) removed the two `flows[].nodes[].config.body.nodes` rows and kept the two `flows[].nodes` ones; this branch (the consumer repair) removed all four. The merged table is `{}` — `main`'s two remaining rows are exactly the ones this branch's coercion retires — and the sweep is re-run rather than reasoned about. The docblock is resolved to carry BOTH accounts rather than either alone. The producer's declared `FlowNodeParsed[]` did lie about what it returned, and the two consumer readers did dereference without a guard; measured on the tree before either fix, the graph-shaped arms threw from the consumer frames, and they are green now for two independent reasons. Neither repair makes the other unnecessary, and the note says so, so a later reader cannot conclude one of them was redundant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU
2 parents ea8564d + 0bb2318 commit 804192a

47 files changed

Lines changed: 2938 additions & 316 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`os explain` stops teaching shapes the spec rejects: all seven remaining broken catalog entries are corrected, and the sweep's xfail ledger is now empty.
6+
7+
The catalog in `packages/cli/src/commands/explain.ts` is hand-maintained and does not derive from the spec, so its examples drifted behind the schemas they claim to demonstrate. The sweep landed for #14811 parses every entry's `example` against its real schema and pinned each failure as an `it.fails` xfail naming a card. This corrects all six that carried one, disposes of the seventh (an entry with no type at all), and promotes every xfail to a plain assertion — leaving the ledger empty, so the identical error cannot return silently.
8+
9+
- **`object`** — select `options` are sampled as objects. A bare `['open', 'closed']` was rejected twice over (`expected object, received string`); each option is `{ label, value }`, where `value` is the stored lowercase machine identifier.
10+
- **`field`** — the prose half of the same defect, which the sweep structurally cannot see: the optional-property table documented `options` as `type: 'string[]'`, and that row is where the `object` example's error came from. It now names `SelectOption[]` and spells the member shape out, and the entry's example demonstrates the real option list instead of a plain text field.
11+
- **`view`** — the entry taught the wrong LEVEL, not a drifted key name. `ViewSchema` is the per-object view CONTAINER (`list` / `form` / `listViews` / `formViews`); a flat list-view literal is rejected wholesale. The tables now document the container, and the example shows a single view's own keys inside a slot.
12+
- **`agent`**`tools` was removed in `@objectstack/spec` 17 with no key its value moves to (ADR-0064: an agent reaches exactly the tools its surface-compatible skills declare). The entry now teaches `skills`, `model` as the configuration OBJECT it is, and the required `label` / `instructions` it had listed as optional or omitted. Two rows naming keys the schema does not have (`objects`, a top-level `temperature`) are gone.
13+
- **`app`** — navigation items carry `id` plus the discriminant's own target key (`objectName` / `dashboardName`), not a bare `object` / `dashboard`. The table's `logo` and `defaultRoute` rows were the same class of error — neither is an `AppSchema` key, and `logo` is now rejected by name in favour of `branding`.
14+
- **`dashboard`** — widgets bind a `dataset` and select `dimensions` / `values` by name (ADR-0021); the pre-ADR-0021 inline analytics shape (`object` + `groupBy` + `aggregate`) was removed, and `'chart'` is not a widget type at all — the enum names the concrete mark. `label` and `widgets` move to required, where the schema has them, and the non-existent dashboard-level `layout` row is dropped for the per-widget `layout: { x, y, w, h }`.
15+
- **`trigger`** — not a wrong example but an entry with no type to check against. ADR-0088 §1 retired the `trigger` metadata kind and records that its enum comment referenced a `TriggerSchema` that never existed. The entry becomes an explicit redirect, the same shape the `workflow` entry already uses, pointing at the two delivered mechanisms the ADR names: a `hook` for synchronous in-transaction data-layer logic, a `record_change` flow for asynchronous automation. Its `docsPath` moves off a page that does not exist.
16+
- **`action`**`object` is `objectName`, `confirmation` is `confirmText`, and a `type: 'flow'` action names its flow in `target` (there is no `flow` key, and none was suggested, which is why the entry could not simply be renamed). The required table also advertised a `"button"` action type that is not in the enum, and a `url` key the schema does not have.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/client": patch
3+
---
4+
5+
The README's namespace tour calls `approvals.approve` / `approvals.reject` with the decision object they declare, and `auth.register` with the field its schema requires.
6+
7+
`approve` and `reject` take `(requestId: string, decision?: { actorId?: string; comment?: string; attachments?: string[] })`. The tour passed the comment as a bare string — `approve(requestId, 'LGTM')` — which a TypeScript reader hits as `TS2559` and a JavaScript reader does not hit at all: the string goes out as the request body where the route reads the decision object's fields, so the approval is recorded and its **reason is silently dropped**. In an approvals surface a lost reason is not a typo. The calls now read `{ comment: 'LGTM' }` / `{ comment: 'Incomplete' }`, the spelling the docs site's Client SDK page already carried.
8+
9+
Type-checking the whole fence against the package's own built `dist/index.d.ts` found one more call in the same defect class — a live method given the wrong argument shape. `auth.register` takes `RegisterRequest`, whose schema declares `name: z.string()` as required (and pins the rejection of a request without it); the tour passed only `{ email, password }`, failing `TS2345`. It now passes `name` as well, again matching the Client SDK page. All 35 calls in the fence type-check clean against the built declarations after this change.
10+
11+
No behaviour changes and no source change: this is the README, and `files` ships `README.md` inside the tarball, so correcting it moves what `@objectstack/client` publishes — it is the package's npm front page.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
`collectFlowGraphs` now honours the `readonly FlowNodeParsed[]` it declares: a member of a region's node list that is not a record is dropped from the `FlowGraph` it hands out, instead of being passed through verbatim.
6+
7+
An ADR-0031 container keeps a whole sub-graph inside `FlowNodeSchema.config`, a deliberately open `z.record`, so `collectFlowGraphs` re-derives those inner node lists at run time and checks them with `Array.isArray` — which proves the LIST and never its MEMBERS. An empty item in a YAML `nodes:` list under a `loop` body deserialises to `null`, and that `null` reached `graph.nodes` on every returned graph, at every depth. No caller could prevent it: this is an array the walk picks up itself, so no coercion at a call site ever holds it. Filed as #16752.
8+
9+
- **What changed.** The walk filters what it hands out and skips what it descends into, through one predicate. Array identity is preserved when nothing is dropped, so a well-formed flow allocates nothing new.
10+
- **What deliberately did NOT change.** The declared input type is untouched — widening it to tolerate malformed members was refused on the anti-AI-error axis, and this is the opposite move: the producer now keeps the promise it already made. The schema refusal that owns a malformed region still fires, unchanged; this walk runs inside `FlowSchema`'s parse, where a thrown `TypeError` would escape `safeParse`, so the repair is a drop and a skip and never a throw. `FlowGraph.path` still indexes the raw authored list, so a Zod issue stays anchored where the author wrote the node.
11+
- **Visible consequence.** As with the sibling repairs that read their lists through a record filter, a dropped member renumbers the ones behind it *within* `graph.nodes` — a difference in the index, never in whether a node was judged, and only in a list that was already malformed.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
`ObjectTitleCompleteness.status` now documents which INPUT each grade describes. No predicate, no grade computation and no accept set moves — the classification is unchanged and correct; what changes is what the declaration tells the reader it means.
6+
7+
`explicit` was documented as "an explicit pointer (`nameField`/`displayNameField`) is set". That is accurate about the predicate, and that is what made it dangerous: on a body served by a `/meta` read exit a pointer is present whether or not the author wrote one, because the exit replays the registry's object-materialization seam (`materializeServedObjectOnto`, which runs `provisionPrimary` in designate-only mode). So `explicit` there means "a pointer is present", never "the author designated this" — and the old wording invited the second reading at every call site.
8+
9+
The corrected declaration says three things the old one left to inference:
10+
11+
- **Provenance decides the grade.** On a served body `explicit` carries no authorship information, and `derived` is unreachable except where that replay withheld the designation. On a body captured before the write seam — an authored definition as written, which is what `os build` / `os lint` hand this predicate through `@objectstack/lint`'s `validateRecordTitle``explicit` really does mean the author wrote a pointer.
12+
- **Authorship is not recoverable afterwards, deliberately.** The write-side inverse `stripProvisionedPrimaryFrom` removes the pointer exactly when it is byte-identical to what the derivation would produce, and says so in its own words: the two are indistinguishable by construction, the same bytes. Neither the served document nor the stored row can answer "did the author designate this?".
13+
- **The consequence for callers.** Never build a check on `explicit` that needs the authored answer without first proving the input is a pre-write body.
14+
15+
`objectTitleCompleteness` itself carries the one-sentence form of the same warning, because a caller hovering the function is on a different path from a caller hovering the grade.
16+
17+
The correct words already existed in this repo — on the WRITE seam, where nobody reading the grade goes. This moves their substance to the grade.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/cli": patch
3+
"@objectstack/lint": patch
4+
---
5+
6+
`objectstack build` now refuses to lower a hook/action body that calls `.create(`, and the shared write-pattern ledger stops advertising the verb. Three layers used to disagree about `ctx.api.object('x').create({ … })`, and the loudest one was wrong.
7+
8+
- The spec contract `IScopedObjectRepository` (`packages/spec/src/contracts/scoped-context.ts`) declares `insert` and names `create` as measured-and-deliberately-excluded.
9+
- The QuickJS sandbox installs exactly `insert / update / delete / updateMany / deleteMany / upsert` as the `ctx.api.object()` write leaves — no `create`. An L2 body calling `.create()` therefore threw `TypeError: not a function` on its **first run**, and under a hook's default `onError: 'abort'` that throw aborted the triggering write, with a message naming no member.
10+
- The extractor ledger nonetheless advertised `.create({…})` as legal `api-crud-literal` syntax and mapped it in `API_WRITE_METHODS`, so `hook-body-write-unknown-field` graded the payload as a live write and stayed silent when the field existed — a clean bill of health for a call that cannot run. Build time said nothing at all.
11+
12+
What changes:
13+
14+
- **`@objectstack/cli`**`.create(` joins `FORBIDDEN_PATTERNS` in the hook/action body extractor, beside `.sudo(` and for the same reason (a member real on the in-process `ScopedContext` / `ObjectRepository` and absent from the VM). The refusal names `.insert({ ... })` as the spelling the sandbox actually has. Behaviour is the `forbidden-token` fallback every other entry has: the callable is still registered and still shipped through the back-compat `.mjs` bundle, so a handler keeps running in-process where the host `create()` alias exists — `objectstack build` merely declines to *also* emit it as a body that cannot run. Under `--strict-body` it is a hard failure, correctly. The rule is receiver-loose like `.sudo(` (`const repo = ctx.api.object('x'); repo.create(…)` is refused too) with one carve-out: `Object.create()` is a real sandbox global and is **not** affected.
15+
- **`@objectstack/lint`**`create` is withdrawn from `HOOK_BODY_WRITE_PATTERNS`' advertised `api-crud-literal` syntax and from `API_WRITE_METHODS`, on the hook and action surfaces alike. `hook-body-write-unknown-field` / `action-body-write-unknown-field` no longer grade a `.create()` payload; `hook-api-update-readonly-field` keeps its existing `create` exclusion, whose *reason* is updated — it is no longer "the call throws, so a silently-dropped finding would be false" but "the shape can no longer reach this rule at all".
16+
17+
**Migration.** If a hook or action body calls `ctx.api.object('x').create({ … })`, spell it `ctx.api.object('x').insert({ … })` — the same host method, the one the sandbox installs and the only insert verb the contract declares. The host-side `ObjectRepository.create()` alias is untouched and stays reachable from in-process handlers and actions.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
Three published prose carriers in `packages/spec` now state what the tree does, instead of a premise the same package's own code contradicts. No predicate, schema arm, key or export moves; every accept set is byte-identical.
6+
7+
All three reach consumers: `@objectstack/spec` ships `src/**/*.zod.ts` in its published `files[]`, so a TSDoc comment in one of these files is a published byte, and one of the three is a `.describe()` that additionally lands in the JSON Schema and the generated reference page.
8+
9+
- **`FILE_REFERENCE_TYPES` said the stored schema "deliberately admits both until D3 lands".** ADR-0104 D3 wave 2 landed: `valueSchemaFor` returns `FileReferenceIdValueSchema` ALONE for `form === 'stored'` and the id-or-object union only for `'expanded'`, and `field-value.test.ts` pins both directions. The docblock now states the shipped contract — the stored value is an opaque `sys_file` id, the inline metadata object is the expanded READ form — and names where legacy stored values are actually admitted: the warn-first ADR-0104 value-shape path in `@objectstack/objectql`'s record validator, never this schema. The sentence promised a future that had already arrived, which is exactly how it kept reading as current.
10+
- **`CreateManyDataResponseSchema.droppedFields` justified its aggregated shape with "schema-uniform, so every row drops the same set".** Ruling C moved the static-`readonly` strip INSIDE `engine.insert`, after the `beforeInsert` hooks, and exempts keys a hook itself wrote — tracked per row (`rowHookWrittenKeys`). A hook that stamps a protected key on some rows and not others therefore makes those rows drop different sets, so the premise is false. The shape is unchanged and stays right for the reason the producer already gives: this response is `{ object, records, count }` and has no per-row slot to hang a drop set on, so a union is the only view it can represent. The description now says that, and tells a reader how to read a name in the array — "at least one row dropped this field", not "every row did". Callers needing row precision still use the per-row `insertMany`/`batch` results.
11+
- **`FieldReferenceSchema`'s `@example` spelled the #14104 shape as the relation path `duty.grace_days`.** Nineteen lines below it, in the same block, the Execution support prose states that SQL push-down compiles same-table columns only and refuses a dotted path with `INVALID_FILTER`. An author copying the example gets a filter that passes in memory — `matchesFilter` walks dot paths — and 400s on SQL. The example is now the same-table spelling both execution paths compile, matching the wording the hand-written query-syntax guide already publishes; the block's own prose remains the statement of what a dotted path does.
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
fix(spec): `isValueDomainMember` refuses an off-vocabulary domain instead of failing OPEN on `Object.prototype` names
6+
7+
`DOMAIN_MEMBERSHIP` is an object literal, so it inherits `Object.prototype`, and
8+
`isValueDomainMember` indexed it with no own-property guard. Measured against the
9+
built artifact (`dist/shared/index.mjs`) on the repo's Node 22 baseline (v22.22.2),
10+
an off-vocabulary `domain` did one of two wrong things — and one of them was a
11+
membership FALSE POSITIVE out of a predicate whose whole job is to refuse
12+
non-members:
13+
14+
| `domain` | before | after |
15+
|:--|:--|:--|
16+
| `iana_time_zone` (in vocabulary) | `true` for `UTC` | `true` for `UTC` — unmoved |
17+
| `toString` | `'[object Object]'` — a truthy **string** | `false` |
18+
| `valueOf` | a truthy **object** | `false` |
19+
| `constructor` | a truthy **object** | `false` |
20+
| `__proto__` | threw a `TypeError` | `false` |
21+
| `nope`, `''` | threw a `TypeError` | `false` |
22+
23+
**Why it is reachable.** "Unreachable in-repo" is not "unreachable". The parameter
24+
is typed `ValueDomain` and every in-repo call site names a member, but
25+
`isValueDomainMember` is **published** on `@objectstack/spec/shared` (it is in
26+
`packages/spec/api-surface/shared.json`). A plain-JS consumer, or any caller
27+
handing over a domain string read from **metadata** rather than written in source,
28+
reaches it with no type checking at all — and metadata-sourced strings are exactly
29+
where `constructor` and `toString` show up.
30+
31+
**This narrows and widens nothing, measured rather than asserted.** Every accepted
32+
`domain` is an own key of the record, so no value that was accepted before is
33+
refused now; the three real domains answer from their own definitions, unmoved.
34+
The change is one `Object.prototype.hasOwnProperty.call` guard — the same spelling
35+
the `iso_4217_currency` definition in the same module already uses — returning
36+
`false` for a domain that is not an own key. A **null-prototype record** was the
37+
other shape available and was not taken: it converts the truthy answers into
38+
throws rather than into `false`, and it costs the `Readonly<Record<ValueDomain, …>>`
39+
annotation that makes a vocabulary member added without a definition fail to
40+
compile.
41+
42+
**Unknown domain answers `false`; it does not throw.** `false` is the narrowing
43+
reading — it refuses more and accepts nothing new — whereas a thrown refusal would
44+
change published behaviour for callers who today receive a truthy value. This is
45+
the same third branch a sister ruling settled for the same defect family: list
46+
reject / own-member value / prototype-resolvable ⇒ reject.
47+
48+
The pin that existed did not cover this, and the fix is as much about its
49+
POPULATION as about the guard: the totality pin asserted the return `typeof` was
50+
`boolean` but iterated `ValueDomainSchema.options` **only** — exactly the domains
51+
that behave. The new pins put `toString`, `valueOf`, `constructor`,
52+
`hasOwnProperty`, `isPrototypeOf`, `propertyIsEnumerable`, `__proto__` and plainly
53+
absent words into the population, and a third pin holds that population honest by
54+
asserting every one of them is still outside the vocabulary.

0 commit comments

Comments
 (0)