Skip to content

Commit 6b7d709

Browse files
claude[bot]claude
andauthored
fix(cli): os validate lowers inline handlers before its parse, so the hook write-set family judges handler-authored hooks there too (#16727)
* fix(cli): `os validate` lowers inline handlers before its parse, so the hook write-set family judges handler-authored hooks there too Every rule in the `hook-body-*` / `hook-api-update-readonly-*` family opens on `body.language === 'js'`. `os validate` parsed the normalized stack without lowering, so a hook authored as `handler: async (ctx) => { … }` carried no body there and the family returned before reading anything: `os validate` passed (exit 0, no finding) a stack `os build` refuses with `hook-api-update-readonly-field`. The body-authored control fired on every door, so the silence was the door, not the rule. `validate.ts` now runs the same `lowerCallables` call `compile.ts` runs at its step 2b — after the two pre-parse unknown-key lints, which keep reading `normalized`, and before the parse, which reads `lowering.lowered` — and hands the registry `parsed: result.data` as before. `lowerCallables` never mutates its input, so the `normalized` tier, the stats and the structural advisories are unchanged; the text face prints no new step. The e2e pin's `os validate` leg flips from a measured "not lowered" reading to a red-first intake leg, with the body-authored control unchanged beside it and a negative control proving a handler-authored hook the family has nothing to say about still passes. `lowerCallables` moves from the parity pin's BUILD_ONLY_GATES to SHARED_NON_REGISTRY_GATES; the intake ledgers in the two rule modules and the hook-bodies doc record the door as reached. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * changeset: `objectstack validate` lowers inline handlers before its parse (cli minor, lint minor) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * fix(cli): declare and pin the widening limb — `os validate` now accepts an inline action `target` callable, as `os build` always did The contract review on #16544 found the change is not a pure narrowing. `ActionSchema.target` is a string and `normalizeStackInput` never touches function values, so before this branch a plain-object config with `actions: [{ name, label, target: async (ctx) => { … } }]` was refused by `os validate` at the parse (`invalid_type` at `actions.0.target`, exit 1) while `os build`, which lowers before it parses, accepted it. The same `lowerCallables` pass now lowers it here too, so `os validate` accepts it — an accepted-set relaxation on a published command, measured through the real CLI on both sides (BASE validate exit 1 with two `invalid_type` issues; HEAD validate exit 0; build exit 0 on both) and now declared in the changeset, in the `validate.ts` comment, and pinned beside the hook legs in the e2e file with a build-parity leg next to it. Also: the stale twin sentence in `build-json-undeclared-key-parity.e2e.test.ts` ("validate lowers no handlers") is corrected, and the `validate.ts` comment now says what "mirrors `compile.ts`" is exact about (lower-before-parse) and what protects the key lints' input on both doors (non-mutation, not order). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * fix(cli): declare and pin the third widening limb — a nameless `functions` array entry, which the same pass names `anon_fn` The re-review enumerated every callable slot `lowerCallables` rewrites and found one more accepted-set relaxation on `os validate`: the `functions` ARRAY form requires `name`, and `normalizeStackInput` never touches `functions`, so `functions: [{ handler: async (ctx) => { … } }]` failed the `functions` union at the un-lowered parse (exit 1) while `lowerBody` names it `anon_fn` before `os build`'s parse. Measured through the real CLI on both sides: BASE validate exit 1 `invalid_union@functions`; HEAD validate exit 0 `valid=true`; build exit 0. Declared in the changeset's widening bullet and the `validate.ts` comment, pinned as a third INTAKE leg in the fourth `describe`. No code change; the `functions` map forms and `hooks[*].handler` are not limbs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 73053ed commit 6b7d709

9 files changed

Lines changed: 258 additions & 32 deletions
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/cli": minor
3+
"@objectstack/lint": minor
4+
---
5+
6+
`objectstack validate` now lowers hooks authored as inline `handler` functions to a metadata body before it parses, so the hook write-set rules judge them there exactly as `objectstack build` and `objectstack lint` already do.
7+
8+
The `hook-body-write-unknown-field`, `hook-body-write-unprovisioned-anchor`, `hook-body-source-unparseable`, `hook-api-update-readonly-field` and `hook-api-update-readonly-when-field` rules open on `body.language === 'js'`. A hook written as `handler: async (ctx) => { … }` carries no `body`, and `objectstack validate` parsed the normalized stack without lowering — so on that command the whole family returned before reading anything, and a stack `objectstack build` refuses with `hook-api-update-readonly-field` (exit 1) passed `objectstack validate` with exit 0 and no finding. The same statement authored as an explicit `body: { language: 'js', source }` was refused by `objectstack validate` all along, so the silence was the command's intake, not the rule.
9+
10+
`objectstack validate` now runs the same `lowerCallables` pass `objectstack build` runs before its parse — after its two pre-parse undeclared-key lints, which keep reading the un-lowered stack, and before the schema parse, which reads the lowered view — and hands the rule registry the parsed result as before. This moves what `objectstack validate` accepts in **both** directions, and both are parity with `objectstack build`:
11+
12+
- **Narrowing (hooks).** A config whose inline handler writes a `readonly: true` field via `ctx.api.object(...).update()` / `.updateById()` / `.insert()` — and does not declare `runAs: 'system'` — now fails `objectstack validate` with `hook-api-update-readonly-field` (exit 1). It already failed `objectstack build` and (since #16095) `objectstack lint` with the same finding, so nothing that builds green starts failing `objectstack validate`.
13+
- **Widening (actions, and a nameless `functions` array entry).** A plain-object config carrying an inline action `target` callable — `actions: [{ name, label, target: async (ctx) => { … } }]`, or the same on `objects[*].actions[*]` — was **refused** by `objectstack validate` before this change: `ActionSchema.target` is a string, and nothing lowered the function before the parse, so the run exited 1 with `invalid_type` at `actions.0.target` (measured through the real CLI: `valid=false errors=2 invalid_type@objects.0.actions.0.target | invalid_type@actions.0.target`). The same pass now lowers it to a ref string plus `body` on this command too, so `objectstack validate` **accepts** it (exit 0, `valid: true`) — exactly as `objectstack build` accepted it all along (exit 0 on both sides). Likewise a nameless `functions` **array** entry, `functions: [{ handler: async (ctx) => { … } }]`, which the same pass names `anon_fn`: the array form requires `name`, so `objectstack validate` refused it at the parse (measured: `valid=false errors=1 invalid_union@functions`, exit 1) and now accepts it (exit 0, `valid: true`), as `objectstack build` did (exit 0 on both sides). The `functions` map forms and `hooks[*].handler` parse either way and are not affected. These are accepted-set relaxations on a published command; they are declared here rather than inferred from the build's behaviour, and pinned beside the hook legs.
14+
- The warning-severity members of the family now report on inline handlers under `objectstack validate` too; they fail a run only with `--strict`, as every other advisory does.
15+
- The `--json` payload gains no key and the text face prints no new step: the lowering is a view for the parse and the rule registry. A handler the extractor cannot lower (a forbidden token, a module-scope identifier) has no body on any command and is reported by `objectstack lint`'s `hook-body/*` rules and `objectstack build`'s warn-and-bundle line, never guessed at here.
16+
- Nothing about what `objectstack build` accepts changes; on both axes above `objectstack validate` now agrees with it.
17+
18+
Measured on this repository's ten `objectstack.config.ts` corpus files at `6ba0db4e0` with `objectstack validate --json`, before and after: **exit code, error text and rule-id list identical on 10 of 10 — zero findings change, zero verdicts change.** Six reach the rule registry (the four example apps and the `plugin-auth` / `plugin-security` / `service-i18n` configs); two (`driver-memory`, `plugin-hono-server`) are plugin manifests, not stacks, and are refused at the schema parse — after the lowering point — with the same top-level `unrecognized_keys` on both sides; two (`app-showcase`, the `blank` template) fail at load in the measuring environment, before the lowering point, on both sides. None of the repository's handler-authored hooks writes through `ctx.api`, and none of the ten carries an inline action `target` callable, which is why the delta is zero on both axes rather than either being unreached — a corpus with neither shape cannot see either limb, so both are pinned on their own fixtures; the reach itself is pinned by the card's own fixture, with the body-authored control beside it and a handler-authored hook the family has nothing to say about still passing.
19+
20+
`@objectstack/lint` carries only the header ledger recording which intakes reach each hook rule; `objectstack validate` moves from "not reached" to "reached". Its behaviour is unchanged.

content/docs/automation/hook-bodies.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ The dropped case is the dangerous one: nothing fails, the step reports success,
268268
- `hook-api-update-readonly-field`**error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` / `.insert()` writes a field the named object declares `readonly: true`. Since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) the `insert` row of the table above is reported at build time exactly like the `update` row — same id, same severity, a message naming the verb — unless the hook declares `runAs: 'system'`. Only the static shape is judged on an insert: a `readonlyWhen` field has no prior record to lock on and the engine runs no conditional strip on INSERT, so no warning is produced there.
269269
- `hook-api-update-readonly-when-field`**warning**. The same write against a `readonlyWhen` field, which strips per record *state*. The own-hook stamp **is** the workaround here, exactly as it is for static `readonly`: since [#9107](https://github.com/objectstack-ai/objectstack/issues/9107) the conditional strip judges the *caller's* entry payload, so a value a `beforeUpdate` hook **derives** is not caller-supplied and lands even on a locked record. (Deriving is the operative word — a hook that merely echoes the caller's own value back has written nothing the strip can tell from the caller's, and it still goes.) What does **not** help is elevation: unlike the static strip, the conditional lock is **not** waived by a system context, so neither `runAs: 'system'` nor the `sudo()` a body cannot reach makes a caller-supplied value survive. On this shape, confirm the write only targets records whose predicate is `false`, or derive the field in a `beforeUpdate` hook on the target object.
270270

271-
Which hooks these rules can *see* depends on the command, because every rule in this family opens on `body.language === 'js'`. A hook authored as an inline `handler` function carries no `body`, so it is judged only where the command has first lowered the handler to a metadata body: `objectstack build` always has (it lowers before it parses — see [How the build lowers a handler](#build-pipeline)), and since [#16095](https://github.com/objectstack-ai/objectstack/issues/16095) `objectstack lint` judges that same lowered view, so an author who runs only the pre-flight is told the same thing the build would refuse. `objectstack validate` parses without lowering, so there a handler-authored hook is not seen by this family — the explicit-`body` form is. A handler the build cannot lower (a forbidden token, a module-scope identifier) has no body on any command and is reported by the lowering rules instead, never guessed at here.
271+
Which hooks these rules can *see* depends on the command, because every rule in this family opens on `body.language === 'js'`. A hook authored as an inline `handler` function carries no `body`, so it is judged only where the command has first lowered the handler to a metadata body: `objectstack build` always has (it lowers before it parses — see [How the build lowers a handler](#build-pipeline)), and since [#16095](https://github.com/objectstack-ai/objectstack/issues/16095) `objectstack lint` judges that same lowered view, so an author who runs only the pre-flight is told the same thing the build would refuse. Since [#16544](https://github.com/objectstack-ai/objectstack/issues/16544) `objectstack validate` lowers before it parses as well, so all three commands judge the same view of a handler-authored hook — a stack `objectstack validate` passes is one `objectstack build` does not refuse on this family. A handler the build cannot lower (a forbidden token, a module-scope identifier) has no body on any command and is reported by the lowering rules instead, never guessed at here.
272272

273273
Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name and an object this stack does not declare are all skipped, so the rule has no opinion on them. `.create()` is skipped too, for a reason about the **sandbox** rather than the engine: the VM-side `ctx.api.object()` installs `insert` / `update` / `delete` / `updateMany` / `deleteMany` / `upsert` and no `create` leaf, so a body calling `.create()` throws `TypeError: not a function` on its first run — a loud failure, not a silent drop — and the same payload spelled `.insert()` is what the rule judges. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425), and since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) it reports a non-`runAs: 'system'` `create_record` node's static-`readonly` write at the same **error**, again with no conditional finding on a create.
274274

packages/cli/src/commands/compile.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,8 @@ export default class Compile extends Command {
856856
// the legacy .mjs bundle. A SEPARATE key on purpose, and the reason is
857857
// parity too — the opposite way round from `unknownKeyWarnings` just
858858
// above. `{origin,reason}` extraction records have NO counterpart in
859-
// `os validate --json`: that command lowers no handlers, so there is
859+
// `os validate --json`: that command lowers too since #16544 but
860+
// EMITS nothing, so it surfaces no extraction record and there is
860861
// no cross-command list for these to join, and folding a shape only
861862
// ONE command can ever emit into the shared key would teach consumers
862863
// a shape the other command never ships. The undeclared-key findings

packages/cli/src/commands/validate.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
type ConversionNotice,
1414
} from '@objectstack/spec';
1515
import { loadConfig } from '../utils/config.js';
16+
import { lowerCallables } from '../utils/lower-callables.js';
1617
import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint';
1718
import { resolveSduiManifest } from '../utils/sdui-manifest.js';
1819
import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
@@ -204,7 +205,62 @@ export default class Validate extends Command {
204205
...lintUnknownStackKeys(normalized as Record<string, unknown>, ObjectStackDefinitionSchema),
205206
...lintUnknownAuthoringKeys(normalized as Record<string, unknown>, ObjectStackDefinitionSchema),
206207
].map(formatUnknownAuthoringKey);
207-
const result = ObjectStackDefinitionSchema.safeParse(normalized);
208+
// 2b. [#16544] Lower inline `function` handlers (Hook.handler, action
209+
// `target`, top-level `functions`) to a metadata `body` + string ref
210+
// BEFORE the parse — the same `lowerCallables` call `os build` makes
211+
// at its step 2b and `os lint` makes in `lintConfig`, not a copy.
212+
//
213+
// Every rule in the `hook-body-*` / `hook-api-update-readonly-*`
214+
// family opens on `body.language === 'js'`. A hook authored as
215+
// `handler: async (ctx) => { … }` carries no `body`, so on the
216+
// un-lowered stack the whole family returned before reading
217+
// anything, and this command passed (exit 0, no finding) a stack
218+
// `os build` refuses with `hook-api-update-readonly-field` — the
219+
// #3782 / #4409 class one door over, on the shape the reference app
220+
// uses for 39 of 39 hooks. The body-authored control fired here all
221+
// along, so the silence was the door, not the rule.
222+
//
223+
// POSITION IS LOAD-BEARING: after the two pre-parse unknown-key
224+
// lints above, which keep reading `normalized` exactly as before,
225+
// and before the parse, which now reads the lowered view. That is
226+
// `compile.ts`'s lower-BEFORE-parse order exactly; its key lints sit
227+
// AFTER its parse, so on both doors what protects the lints' input
228+
// is non-mutation, not ordering: `lowerCallables` returns a NEW
229+
// top-level object and never mutates its input, so `normalized` —
230+
// the registry's `normalized` tier below, `collectMetadataStats(
231+
// config)`, the structural advisories — is byte-for-byte what it
232+
// was; only what the parse and the registry's `parsed` tier see
233+
// changes.
234+
//
235+
// NOT A PURE NARROWING. The same pass also lowers an inline action
236+
// `target` callable (`actions[*]`, `objects[*].actions[*]`) to a ref
237+
// string plus `body`, and names a nameless `functions` ARRAY entry
238+
// (`[{ handler: fn }]`) `anon_fn`. `ActionSchema.target` is
239+
// `z.string()`, the array entry requires `name`, and
240+
// `normalizeStackInput` touches neither, so before this step the
241+
// un-lowered parse REFUSED both configs (`invalid_type` at
242+
// `actions.0.target`; `invalid_union` at `functions`; exit 1) while
243+
// `os build` accepted them all along. Both are accepted here now —
244+
// accepted-set relaxations on this command, each measured through
245+
// the real CLI on both sides and pinned in
246+
// `test/lint-hook-rules-reach-handler-hooks.e2e.test.ts`. Not
247+
// limbs: `hooks[*].handler` accepts a function un-lowered, and the
248+
// `functions` MAP forms parse either way. Parity with the build is
249+
// the intent, and it is declared rather than assumed because a
250+
// sibling's acceptance is evidence of intent, not a declaration on
251+
// this command's face.
252+
//
253+
// Nothing is emitted, so `lowering.functions` is unused here, and
254+
// the extraction refusals in `bodyExtractionWarnings` are NOT
255+
// surfaced: a handler the extractor refuses is left with no `body`
256+
// on every door, the family stays silent on it, and the refusal is
257+
// `os lint`'s `hook-body/*` rules' to report. Publishing it here
258+
// would add a key to this command's `--json` payload, which is its
259+
// own contract decision (`compile.ts` records why the key is
260+
// build's alone). No step line is printed either: the text face is
261+
// byte-for-byte what it was, and the docs transcripts stay true.
262+
const { lowered } = lowerCallables(normalized as Record<string, unknown>);
263+
const result = ObjectStackDefinitionSchema.safeParse(lowered);
208264

209265
if (!result.success) {
210266
if (flags.json) {

packages/cli/test/build-json-undeclared-key-parity.e2e.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@
4343
* This is deliberately the opposite call from `bodyExtractionWarnings`, which
4444
* sits under its own key one line below in the payload. That is not an
4545
* inconsistency: `{origin,reason}` extraction records have NO counterpart in
46-
* `os validate --json` (validate lowers no handlers), so there is no parity to
47-
* hold and a sibling key is right. The undeclared-key findings do have a
46+
* `os validate --json` (validate lowers too since #16544, but emits nothing, so
47+
* it surfaces no extraction record), so there is no parity to hold and a
48+
* sibling key is right. The undeclared-key findings do have a
4849
* counterpart, and it is already in `warnings`.
4950
*
5051
* The payload's top-level key set is pinned unchanged below for that reason:

0 commit comments

Comments
 (0)