Skip to content

Commit 47be844

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-6619-fold-error-maps
2 parents 0c247c0 + 9b86cf6 commit 47be844

179 files changed

Lines changed: 31161 additions & 1978 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: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
---
2+
"@objectstack/objectql": minor
3+
"@objectstack/spec": patch
4+
---
5+
6+
feat(objectql): dispatch `before*` hooks per matched row on a predicate bulk write (#5574, #5846)
7+
8+
A `multi: true` update or delete now dispatches `beforeUpdate` / `beforeDelete`
9+
**once per matched row**, on a single-record-shaped context carrying that row's
10+
`id` and `previous` — the same move #5038 made for the `after*` phase, held to
11+
the same yardstick. ADR-0058 Addendum II (maintainer ruling B, 2026-08-06) is
12+
the contract; `packages/spec/src/data/bulk-write-hook-conformance.ts` states it
13+
as clauses D1–D7, and its `delivered` flags flip with this change.
14+
15+
**The harm this fixes.** `ctx.previous` was never bound in the before phase of a
16+
predicate write, so every guard written the way guards are written —
17+
`if (ctx.previous?.locked) throw` — passed **silently** on every batch. The
18+
failure direction is fail-OPEN and the optional chaining that makes it silent is
19+
exactly what an AI writes. One measured deployment had all 15 of its guard hooks
20+
bypassed by a single batch edit, including writing `null` into a `readonly: true`
21+
field that the single-id path refuses.
22+
23+
**Two visible behaviour changes, both loud.**
24+
25+
- **Guards now fire per row on predicate writes.** A `beforeUpdate` /
26+
`beforeDelete` hook on an object targeted by a `multi: true` write runs N times
27+
instead of once, each time with that row's `previous` bound. Zero matched rows
28+
is zero dispatches. A hook that throws refuses the whole batch before anything
29+
is written. The payload stays **batch-scoped** (D3): every per-row context
30+
carries the one payload, so a rewrite applies to every matched row whichever
31+
row's dispatch made it, rewrites accumulate in dispatch order, and no predicate
32+
write is ever split into N single-row writes — one `updateMany`, one affected
33+
count (#4639), one aggregate event. A rewrite *conditioned* on the row is
34+
therefore out of contract: it widens to the whole batch rather than scoping
35+
itself. Per-row `previous` is supplied so a guard can REFUSE, not so a rewrite
36+
can be aimed.
37+
- **The `input.id` reroute lever is retired and now refuses.** The dispatch
38+
ladder is resolved **before** the before phase — it has to be, since per-row
39+
contexts are built from the matched row set — so the id slot can no longer
40+
steer the write. Rather than ignore an assignment (a silent no-op) or honour
41+
it blindly, the write is rejected with `HookTargetRebindError`
42+
(`ERR_HOOK_TARGET_REBIND`), whose message names the retired capability and the
43+
three supported replacements. Recorded as ADR-0058 Amendment II.1. Precisely:
44+
45+
| | CLEARED id | REBOUND to another id |
46+
|---|---|---|
47+
| `update()` by-id | refused | refused |
48+
| `delete()` by-id | refused | **honoured, unchanged** (#5272's re-read) |
49+
| either, per-row | refused (D4) | refused (D4) |
50+
51+
Clearing is uniform because it worked by falling through to the predicate
52+
branch, and that branch is now chosen before any handler runs. Rebinding is
53+
not uniform, deliberately: the case against honouring it is that the write
54+
lands on a row whose pre-image and rules were never evaluated, and on
55+
`delete()` that is simply not true — #5272 already re-resolves the new target
56+
before `afterDelete` or the summary recompute sees it. `update()` has no such
57+
mechanism and building one would be the "silently pick re-resolution instead"
58+
the ruling forbids. Retiring the delete-side repoint is its own question,
59+
filed as #6752 rather than ridden in on an ordering change.
60+
61+
**Also in this change.**
62+
63+
- **One read, reused (D7).** The matched row set is read ONCE per predicate
64+
write, with the write's own composed AST, and serves per-row validation
65+
(#3106), the `readonlyWhen` strip (#3042) and both per-row dispatches.
66+
- **One ceiling, both phases (D6).** `MAX_BULK_PER_ROW_HOOK_ROWS` (10 000) now
67+
governs `before*` as well as `after*`, checked **before the first dispatch**, so
68+
an over-ceiling batch runs zero handlers and writes nothing — a refusal, never
69+
a downgrade to one dispatch. The engine's open-coded ceiling and refusal
70+
message are replaced by the spec module's `resolveBulkPerRowHookBudget`, so the
71+
number and the wording have one definition again.
72+
- **`update()` binds `previous` before the before phase (#5846 (a)).** The by-id
73+
path reads its prior row ahead of the dispatch, matching `delete()`'s shape
74+
since #5272, so both phases share one read. objectql's
75+
`sys_fetch_previous_update` builtin is **retired**: it existed to bind
76+
`previous` for the before phase behind `if (input.id && !ctx.previous)`, and
77+
that guard is now permanently false. A by-id update on a kernel used to read
78+
the same row three times; this removes one and makes the engine's read the
79+
single producer.
80+
- **`HookConditionLimitation` is retired** (ADR-0049 enforce-or-remove), with
81+
`isPredicateBulkWrite` and the `predicateBulkWrite` flag. Both members
82+
(`bulk_write_previous_unbound`, `bulk_write_stored_state_unavailable`)
83+
described a batch-scoped `before*` dispatch that no longer exists, leaving them
84+
with neither producer nor reachable consumer. A `previous`-reading `before*`
85+
condition on a bulk write now **evaluates as authored**, per row, instead of
86+
rejecting the batch. `HookConditionError` itself is unchanged — an unevaluable
87+
condition still aborts the operation (#4775).
88+
89+
**Migrating.** A handler that cleared `ctx.input.id` — or rebound it on an
90+
`update()` — must instead write through `ctx.api` / `ctx.ql` for the row it
91+
means, have the caller pass `{ multi: true, where: … }`, or throw to refuse the
92+
write. A `beforeDelete` handler that repoints the target is unaffected. A `beforeUpdate` hook
93+
with side effects on an object that receives bulk writes should expect to run
94+
per row; a batch-wide effect belongs in a payload rewrite, which is still
95+
batch-scoped.
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
---
2+
"@objectstack/formula": minor
3+
---
4+
5+
fix(formula): the CEL pushdown compiler parses through the canonical front end, so `DEFAULT_LIMITS` finally apply to RLS/sharing predicates (#6132)
6+
7+
`cel-to-filter.ts` — the ONE canonical CEL → `FilterCondition` pushdown compiler
8+
(ADR-0058 D1/D2/D6), consumed by the RLS path (`plugin-security`'s
9+
`RLSCompiler`), the sharing seeder (`plugin-sharing`), and the analytics SQL
10+
backend — kept a **private, limitless** parse environment of its own:
11+
12+
```ts
13+
new Environment({ unlistedVariablesAreDyn: true, enableOptionalTypes: true })
14+
```
15+
16+
no `limits`, no stdlib, no `rewriteNullableTernary`. That made the pushdown path
17+
the one place on the platform that answered a *different* question from
18+
`celEngine.compile()` about what parses. Measured: a 300-term addition, a
19+
60-level parenthesis nest and a 200-element list literal all parsed there while
20+
the interpreter refused each one outright (`Exceeded maxAstNodes (256)` /
21+
`maxDepth (32)` / `maxListElements (64)`). Escalated: an 80-term conjunction, a
22+
40-level nest and a 200-element `$in` all reached **real pushdown SQL**,
23+
silently — and `isSupportedRlsExpression`, the ADR-0056 D4 authoring gate, was a
24+
thin wrapper over the same limitless environment, so it was no independent check
25+
either.
26+
27+
It now parses through `parseCelToAstWithReason`#4812's canonical entry, with
28+
`DEFAULT_LIMITS`, the stdlib and the #3306 null-guard rewrite. "What parses" has
29+
one answer again.
30+
31+
**Within the limits nothing moves, and that is measured, not asserted.** Across
32+
the 710 sources of the pushdown corpus that both front ends accept, the only AST
33+
difference is `rewriteNullableTernary`'s `dyn(…)` wrap on the three null-guard
34+
ternaries — and a ternary faults on its own `?:` node before the lowerer
35+
descends into a branch, so verdict *and* detail come out byte-identical. Pinned
36+
in `cel-to-filter-parse-convergence.test.ts`, which rebuilds the old environment
37+
to compare against.
38+
39+
**Over the limits, behaviour changes — in two dated steps.**
40+
41+
- **Now, during `17.0.0-rc.x` (`rc-grace`):** an over-limit predicate **still
42+
compiles** — nothing that enforces today stops enforcing on this upgrade — and
43+
emits one WARN per predicate naming the bound that was exceeded
44+
(`maxAstNodes` / `maxDepth` / `maxListElements` / …), the platform's value for
45+
it, and what the predicate itself measures (cel-js's own accounting: the
46+
smallest bound it parses under), plus what will happen at GA.
47+
- **At v17.0.0 GA (`fail-closed`):** the same predicate is **refused**
48+
`{ ok: false, reason: 'parse-error', detail: 'Exceeded maxAstNodes (256)' }`
49+
and the RLS path turns that into `RLS_DENY_FILTER`, i.e. zero rows, fail
50+
closed. A sharing rule with such a condition is not seeded.
51+
52+
**The flip is one line.** `CEL_PUSHDOWN_LIMITS_MODE` in
53+
`packages/formula/src/cel-pushdown-limits.ts` — the single dated switch,
54+
shipping as `'rc-grace'`, to be set to `'fail-closed'` at the v17.0.0 GA release
55+
(i.e. when this package's version leaves `17.0.0-rc.x`). Both positions are
56+
exercised in CI today, in `@objectstack/formula` and in
57+
`@objectstack/plugin-security` (where the `RLS_DENY_FILTER` outcome lives), so
58+
the GA half is proven before it ships rather than after. Two tests are written
59+
to go red on that line so the flip cannot be silent.
60+
61+
**If you author RLS or sharing predicates:** a predicate over any of these
62+
bounds is already refused everywhere else on the platform (`os build`,
63+
`os validate`, the interpreter). Split it, or move the logic into a hook/action
64+
body (`ScriptBody { language: 'js' }`), before upgrading past the rc line. The
65+
WARN names the predicate and its measure so you can find them.
66+
67+
**New public surface**, for consumers that must *report* a refusal rather than
68+
merely react to one:
69+
70+
- `parseCelToAstWithReason(source, opts?)` — the reason-carrying sister entrance
71+
to `parseCelToAst`. Same front end, same verdict, but it distinguishes
72+
`'parse'` (not valid CEL) from `'bounds'` (valid CEL, over budget) and names
73+
the exceeded limit, its platform value, and the source's measure. Graded by
74+
the same by-class/by-code classifier `celEngine.compile` uses (#6223) — never
75+
by error prose. `parseCelToAst` is unchanged and still collapses every refusal
76+
to `null`.
77+
- `CelParseResult`, `CelBoundsOverrun`, `CelLimitKey`, `ParseCelToAstOptions`.
78+
- `CEL_PUSHDOWN_LIMITS_MODE`, `celPushdownLimitsMode()`,
79+
`setCelPushdownLimitsModeForTests()`, `CelPushdownLimitsMode`.
80+
81+
`@objectstack/lint` needs no change, at either position of the switch. Its two
82+
enforceability gates read `isSupportedRlsExpression` and `compileCelToFilter`,
83+
both downstream of this switch, and both suites pin "the lint verdict IS the
84+
consumer's verdict" in both directions — so authoring-time reporting flips with
85+
the runtime by construction. An over-limit sharing `condition` is in fact
86+
already an authoring **error** today (`expression-invalid`, from the general
87+
expression rule, quoting `Exceeded maxAstNodes (256)`), because that rule has
88+
always gone through the canonical front end.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): `os doctor`'s ledger-failure row names the directory it actually read (#6643)
6+
7+
Removes divergence surface; not a live defect. `DEFAULT_INSTALLED_PACKAGES_DIR``@objectstack/cloud-connection`'s export, the single authority on what the installed-package ledger directory is called — exists in every version ever shipped and has never changed value, so the literal this change deletes currently agrees with it. What it buys is that the agreement stops being a coincidence nobody would notice breaking.
8+
9+
The residue of #5996, which fixed the same restatement one row over (`installedPackageLedgerSkippedEntriesCheck`) and enumerated the rest rather than widening in place:
10+
11+
- `installedPackageLedgerFailureCheck` takes the resolved `dir` — already carried on the reading since #5996 — and quotes it. Its `fix` used to open with a re-hardcoded ``.objectstack/installed-packages/`` "under the project root", which was the consumer restating a value only the producer decides, and a vaguer answer than the one doctor was holding: the reading's `dir` is `cwd`-joined and absolute. Under `--verbose` the row now reads ``The ledger is `/srv/app/.objectstack/installed-packages`;`` and drops the now-redundant project-root hedge. The parameter is required, so the row cannot quietly fall back to a guess.
12+
- `os package install`'s post-install hint keeps its literal, now with the reasons written down. That sentence describes the **remote** runtime host's directory: the CLI never touches that disk, the host's directory is configurable (`MarketplaceInstallLocalPlugin` builds `new LocalManifestSource(config.storageDir)`, so the export is only its default), and the install response carries no `storageDir` to quote. Resolving it locally would state this machine's default as an observed fact about another one — so the literal stays, marked as the description of a convention rather than a consumer read.

.changeset/eighty-donuts-tickle.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
fix(cli): `os login --json` is a parseable NDJSON stream (#6531)
6+
7+
`os login --json` produced output that no consumer could read in any shape. The
8+
device flow wrote its RFC 8628 device-authorization payload compact and, once
9+
the token poll resolved, the result payload 2-space indented — two JSON
10+
documents on one stdout. Driven against a live device endpoint, that stream
11+
failed `JSON.parse(<entire stdout>)` with `Unexpected non-whitespace character
12+
after JSON at position 200`, and read as NDJSON it failed on 5 of its 6 lines,
13+
because the second document spanned five of them. The same two-document shape
14+
appeared on the failure path, where an error payload could follow a
15+
device-authorization record that had already been written.
16+
17+
`os login --json` is now a **newline-delimited JSON stream**: one compact
18+
document per line, on every path — the device-authorization record, the
19+
`--email`/`--password` result, the already-logged-in notice, and the
20+
`{"success":false,"error":"…"}` failure record alike. Every line parses on its
21+
own, and the verification-URL record still arrives *before* the user
22+
authorizes, which is what makes the device flow usable from a script at all.
23+
24+
This is the CLI's **one declared exception** to "`--json` means exactly one JSON
25+
document on stdout" (#6217), and it is declared rather than silent: the
26+
`--json` flag's `--help` text says so, and so do the CLI reference page and the
27+
device-flow section of the authentication docs. Parse this command's stdout
28+
line by line.
29+
30+
Bumped as a patch: no interface is added or removed and nothing that previously
31+
worked stops working. The device-flow output was unparseable before, so it had
32+
no consumers to break; the only other observable change is that the
33+
email/password result is compact rather than indented, which `JSON.parse` reads
34+
identically. Human-mode output is untouched.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
Enforce `ActionDescriptor.supportsPause` at the engine boundary: an executor whose
6+
`execute()` returns `suspend: true` while its descriptor declares `supportsPause: false`
7+
is now refused instead of pausing the run (#6667, from #5703).
8+
9+
`supportsPause` used to be read only at authoring time — the designer palette, the
10+
registration warning, and the `check:resume-authority-declared` CI gate, all of which key
11+
on `supportsPause: true` and so were silent on exactly this mismatch. The pause it let
12+
through was already broken, just later and elsewhere: a type that declares no pause
13+
declares no `resumeAuthority` either, and since #5561 an unclaimed pause is fail-closed,
14+
so the run parked on a durable continuation that the generic resume route then refused
15+
with `PERMISSION_DENIED` — a message naming `resumeAuthority`, not the `supportsPause`
16+
that actually caused it. The refusal fails the run where the mistake was made, writes no
17+
continuation, and names the one-line fix.
18+
19+
Behaviour change for third-party executors in that state (no built-in is: all six pausing
20+
built-ins declare `supportsPause: true`). The refusal is guard-class, so a `fault` edge
21+
does not route it — a wrong declaration is not a condition a re-run can fix. Two shapes
22+
are deliberately untouched: declaring `supportsPause: true` and never suspending is legal
23+
(a capability, not an obligation), and an executor that publishes no descriptor at all
24+
declares nothing to enforce — its pauses stay governed by the #5561 resume gate.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
'@objectstack/plugin-security': minor
3+
---
4+
5+
Explain and enforcement now resolve ONE authorization aggregation (#6352).
6+
7+
`buildContextForUser()` — the explain API's reconstruction of an arbitrary user's
8+
context, behind `explain(request, callerContext)` and the `userId` parameter — was
9+
a hand-written second implementation of `@objectstack/core`'s `resolveAuthzContext`
10+
aggregation. Its agreement with enforcement was guaranteed by two comments saying
11+
it mirrored the resolver ("mirroring the runtime resolver's semantics", "we compute
12+
it here with the IDENTICAL rule") and by nothing else: no assertion anywhere in the
13+
repo compared the two.
14+
15+
It did not agree. Measured over identical rows, the mirror dropped:
16+
17+
| input | resolver | explain mirror |
18+
|---|---|---|
19+
| `sys_member` role positions (ADR-0095 D3) | `org_admin`, … ||
20+
| position-bound permission sets (`sys_position_permission_set`) | resolved ||
21+
| the `everyone` anchor's bound sets (ADR-0090 D5) | resolved ||
22+
| `platform_admin` position projection (ADR-0068 D2) | projected ||
23+
| `systemPermissions` / `posture` / `email` / `ai_seat` | resolved ||
24+
25+
The user-visible consequence: permission sets are resolved BY NAME from
26+
`context.positions ∪ context.permissions`, and a set carried by a POSITION only
27+
becomes a name inside the resolver. So for any user whose grants arrive through a
28+
position — the ordinary way an org grants access — the explain panel resolved fewer
29+
sets than enforcement and reported a denial the runtime never made. A security UI
30+
that says "you have no access" about access you have is worse than no panel.
31+
32+
`buildContextForUser` now calls `resolveUserAuthzGrants` (core's userId-driven
33+
resolver core, already the same entry point `runAs:'user'` automation runs use) and
34+
adds presentation only: the ADR-0091 expired-grant and `delegated_from` annotations
35+
the resolver correctly discards, and `hasPlatformAdminGrant`, which is now read
36+
back off the resolver's own posture verdict instead of recomputed. The returned
37+
context additionally carries `systemPermissions`, `org_user_ids`, `posture`,
38+
`tabPermissions` and `email` — additive; no field was removed or renamed.
39+
40+
Pinned by a parity suite that runs both implementations over the same fixture rows
41+
(org role projection, position-bound sets, the `everyone` anchor, both
42+
`platform_admin` polarities, `organization_admin``TENANT_ADMIN`, ADR-0091
43+
windows) and asserts each case's concrete expected output, so the pin cannot pass
44+
by both sides resolving to nothing. Restoring the mirror turns 9 of those cases
45+
red.

0 commit comments

Comments
 (0)