Skip to content

Commit fe57fcb

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-5701-regex-retire-icontains
2 parents 0fccac6 + 5e3c83b commit fe57fcb

87 files changed

Lines changed: 7218 additions & 427 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: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/runtime": patch
4+
---
5+
6+
feat(spec): declare the action-body `ctx.session` contract (#5697)
7+
8+
An action body reads `ctx.session` on every dispatch, and until now **nothing
9+
declared it**. `actionContext` is a bare `any` at both dispatch sites
10+
(`domains/actions.ts`, `action-execution.ts`), the sandbox seam types
11+
`ScriptContext.session` as `unknown`, and the one spec-side mention was an
12+
inline literal on `ActionHandlerContext` carrying a `[k: string]: unknown`
13+
catch-all. Declared-nowhere, produced-anyway: no schema, no gate, no generated
14+
reference page, and nothing the liveness ledger could reach.
15+
16+
That is how the surface drifted without anyone noticing. Its `roles` key carries
17+
`ExecutionContext.positions` — the ADR-0090 D3 vocabulary handed to authors under
18+
the one spelling that ADR forbids — while the hook side retired its own
19+
`session.roles` at #5050. One platform, one key name, two opposite answers.
20+
21+
**`ActionSessionSchema` (`@objectstack/spec/ui`) declares that shape as built.**
22+
23+
```ts
24+
{ userId?: string; organizationId?: string; roles?: string[] }
25+
```
26+
27+
This release changes **nothing about what the runtime produces** — it is phase 1
28+
of #5613's contract-first ruling, and declaring current reality is deliberately
29+
not the same as endorsing it:
30+
31+
- `roles` is declared **deprecated** in its `.describe()` and its JSDoc. The
32+
rename to `positions`, with a deprecation window and an ADR-0087 semantic
33+
migration, is #5613 phase 2. There is deliberately **no `positions` key yet**
34+
minting one before the migration would ship two live spellings of one value.
35+
- The schema is **not strict**, matching `HookContextSchema`: this is a runtime
36+
shape the platform hands a body, never authored, and closing it would turn a
37+
future engine-side enrichment into a parse failure for whoever parses a context
38+
they were given.
39+
40+
Three facts the declaration now states, all of them previously discoverable only
41+
by reading the builder:
42+
43+
- **Absent means the key is absent.** The builder uses conditional spreads, so
44+
`'organizationId' in ctx.session` answers `false` — not "present and
45+
`undefined`". The hook path's `input.id` on a bulk write is the opposite case
46+
(#5668); an `in` test does not port between them.
47+
- **No identity envelope yields no session at all**`undefined`, never `{}`, so
48+
a body can tell "no caller" from "an anonymous caller" (#3712). One consequence:
49+
`roles` never appears on its own.
50+
- **`organizationId` is the blessed name** for the caller's active org; the
51+
v11-removed `session.tenantId` alias (#3280 / #3290) does not come back.
52+
53+
Type-only on the runtime side, no behaviour change: `buildActionSession()` now
54+
declares `ActionSession | undefined` instead of `any | undefined`, and
55+
`ActionHandlerContext.session` is the schema's inferred type rather than an
56+
inline literal with a catch-all. A handler annotated with `ActionHandler` that
57+
read an undeclared key off `ctx.session` now gets a compile error naming it —
58+
that key was never produced. `ScriptContext.session` deliberately stays
59+
`unknown`: it is one seam over both body kinds, and hook and action sessions are
60+
different objects.
61+
62+
The declaration ships with the gate it needed —
63+
`packages/runtime/src/action-session-shape-contract.test.ts` executes the real
64+
producer and asserts a non-strict parse of the built object returns it
65+
**unchanged**, so a key the builder starts producing without declaring here is
66+
stripped and the pin goes red.
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
"@objectstack/rest": patch
4+
"@objectstack/spec": patch
5+
---
6+
7+
fix(analytics,rest): five dataset refusals declare `DATASET_INVALID` / 400 themselves, and the route's message-sniffing list shrinks to one entry (#5367)
8+
9+
`POST /analytics/dataset/query` answered `400 DATASET_INVALID` for six error
10+
families because the route recognised their **prose**, not because the errors
11+
said anything about themselves. #5352 gave the catch an ADR-0112 envelope branch
12+
(`error.code` + a 4xx `error.status`, read first) and had to leave a hardcoded
13+
list of message substrings behind it, since all six producers were still bare
14+
`throw new Error(…)`:
15+
16+
```
17+
/not declared in the dataset|not backed by a declared relationship|
18+
not supported by the v1 dataset runtime|read-scope-sql|
19+
not a selected dimension or measure|is not a subset of the selected dimensions/
20+
```
21+
22+
That made the HTTP status of six families a property of their wording.
23+
Rephrasing `dataset-compiler`'s "is not declared in the dataset's `include`" —
24+
no logic change — moved that refusal from 400 to 500, i.e. re-opened #5352 for a
25+
different family, and no test and no gate would have gone red. Prime Directive
26+
#12 permits an accommodation like that only while it is declared, loud, tested
27+
**and removable on a schedule**; #5366 delivered the first three and nothing
28+
carried the fourth.
29+
30+
**Five producers now declare their own verdict.** A new
31+
`dataset-refusal.ts` in `@objectstack/service-analytics` exports
32+
`datasetInvalidError` — the same shape as that package's existing
33+
`invalidFilterError` (`INVALID_FILTER` / 400) and `assertDimensionFields`
34+
(`INVALID_FIELD` / 400) — and five sites throw through it:
35+
36+
- `dataset-compiler.ts` — a measure whose aggregate the v1 runtime cannot lower;
37+
a dimension/measure traversing a relationship path the dataset never declared
38+
in `include`;
39+
- `dataset-executor.ts` — an `order` key that is not a selected dimension or
40+
measure; a `totals` grouping that is not a subset of the selected dimensions;
41+
- `native-sql-strategy.ts` — a join outside the dataset's declared allowlist.
42+
43+
Their five entries are gone from the route's list, which is now a single
44+
`read-scope-sql` test.
45+
46+
**`read-scope-sql` deliberately stays.** Its ten fail-closed refusals are RLS
47+
read-scope lowering failures whose inputs are an admin-authored policy and a
48+
compiler-generated join alias — not caller input — so `DATASET_INVALID` ("your
49+
request is invalid") may well be the wrong verdict and choosing the right one is
50+
a separate judgement, still tracked by #5367. Deleting the entry before that
51+
judgement lands would regress those ten from `400 DATASET_INVALID` to 500.
52+
53+
**No outward behaviour change for the five.** They answered
54+
`400 DATASET_INVALID` before and answer `400 DATASET_INVALID` now, with the same
55+
message; what changed is the mechanism, from message-matching to the producer's
56+
own declaration. The one visible difference is for a bare `Error` that merely
57+
*resembles* one of those messages: it is no longer promoted to a 400. That is the
58+
point — a phrase is no longer a classification.
59+
60+
`DATASET_INVALID` is registered in `ERROR_CODE_LEDGER` under
61+
`@objectstack/service-analytics` as well as `@objectstack/rest` (provenance, per
62+
ADR-0112 D3; the code itself is unchanged and the union does not grow), and the
63+
constructor types it as `RegisteredErrorCode` so an unregistered code is a
64+
compile error rather than a body some route rejects at runtime.
65+
66+
Coverage: `dataset-refusal-envelope.test.ts` (service-analytics) pins each of the
67+
five refusals against its real producer — the refusal SET first, green before and
68+
after, then the envelope; `analytics-dataset-refusal-envelope.test.ts` (rest)
69+
drives all five end-to-end through a real `AnalyticsService` with positive
70+
controls on both the aggregate and raw-SQL paths; and
71+
`analytics-filter-refusal-envelope.test.ts` pins the deletion in both directions
72+
— the five messages answer 400 when enveloped and 500 when bare, so re-adding a
73+
regex entry turns it red.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): an ADR-0087 rename resolves a shadowed alias by VALUE, instead of always leaving it behind (#4923)
6+
7+
Every ADR-0087 D2 conversion that renames a key (`renameKey` /
8+
`renameConfigKey`, so `object``objectName`, `filters``filter`,
9+
`flow``flowName`, the four `notify` aliases, `description``subtitle`,
10+
the datasource driver-config aliases, …) used to do **nothing at all** when the
11+
canonical key was already present. The retired spelling then stayed in the
12+
converted metadata forever — the conversion had not finished converting.
13+
14+
That was invisible while flow-node config contracts were `.strip`: the dead key
15+
was silently dropped at the execute-time parse and the node ran. Once #4001 批 9
16+
made those contracts strict it stopped being invisible — a stored `subflow`
17+
carrying `{ flowName, flow }` loads today and would be refused at execute time
18+
as a guard failure (not routable through a `fault` edge).
19+
20+
**What changes.** A rename that meets both spellings now splits on whether the
21+
two values actually disagree:
22+
23+
- **Same value** (structural equality, so two separately-authored
24+
`{ status: 'stale' }` filters count as one declaration) → the alias carries
25+
nothing the canonical key does not, so it is **deleted** and the conversion
26+
emits its usual notice. Lossless hygiene, and it makes the transform
27+
idempotent in both shape and notices.
28+
- **Different values****both keys are kept** and no notice is emitted. Two
29+
spellings holding two different values is genuine author ambiguity, and an
30+
upgrade tool that silently picked the canonical one would be editing a
31+
configuration the customer never agreed to. The surviving pair is what lets
32+
the strict node-config gates refuse with a prescription that **names both
33+
keys** and asks for a decision.
34+
35+
`notify`'s nested `source: { object, id }` lift follows the same rule: a part
36+
that repeats its flat counterpart is redundant and `source` is dropped, while a
37+
part that disagrees leaves the node **entirely** untouched so `source` reaches
38+
the strict contract intact. (The `wait` node's loose-key lift is deliberately
39+
NOT covered — it moves keys between two locations rather than resolving two
40+
spellings of one slot.)
41+
42+
**What an author sees.** Metadata that named one slot twice with the same value
43+
loses the retired spelling at load and gains one deprecation notice per removal
44+
— the same notice a plain rename already emitted, so `objectstack validate`
45+
output is unchanged in kind. Metadata that named one slot twice with *different*
46+
values is unchanged by the conversion and is now refused by the strict
47+
node-config contracts with a message naming both keys; the fix is to decide
48+
which value is right, put it on the canonical key, and delete the alias. The
49+
batch-9 prescriptions were reworded accordingly: they no longer say the
50+
surviving twin is "dead" (true only under the old rule), because a key that now
51+
reaches that parse holds a value the canonical key does not.
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
feat(objectql): the dangling-reference audit stops skipping `readonly`
6+
references and files them in their own `provenance` bucket (#4743)
7+
8+
`auditDanglingReferences` used to drop every `readonly` reference field before
9+
reading a single row. That skip rested on two grounds, and #4556 removed one of
10+
them: the platform no longer writes a NON-ID into a reference column
11+
(`sys_metadata_history.recorded_by` stored the sentinel string `'system'`; it
12+
stores `NULL` now). What the skip still covered afterwards was exactly one
13+
family — the audit-provenance fields `created_by` / `updated_by` /
14+
`organization_id` that `applySystemFields` injects, all `readonly: true`.
15+
16+
Those hold **genuine ids, and genuine ids dangle**: delete one user and every
17+
row they ever created points `created_by` at a row that is gone. "Who did this"
18+
failing to resolve is precisely the question an audit trail exists to answer,
19+
so the remaining skip was blindness rather than economy. The audit now probes
20+
them.
21+
22+
**They do not join `dangling`.** A deleted actor and a broken business foreign
23+
key are different findings with different remedies (usually nothing to do vs.
24+
re-seed the target or clear the link), and merging them would bury the second
25+
under the first. Two new report keys carry the new class, mirroring the
26+
unknown/absent split the report already makes everywhere else:
27+
28+
| Key | Means |
29+
|:--|:--|
30+
| `provenance: DanglingReference[]` | a `readonly` provenance reference that resolves to nothing — same row shape as `dangling` |
31+
| `provenanceUndetermined: number` | a provenance reference whose target could not be probed at all |
32+
33+
Both are **additive and optional in the type**, exactly like `aborted`: an
34+
existing consumer keeps compiling and keeps reading `dangling` with its meaning
35+
unchanged (a link the model *declares* is broken). Every report this module
36+
produces sets both explicitly.
37+
38+
⚠️ **Expect `provenance` to be large on the first run against an aged
39+
database.** One deleted user dangles every row they ever touched. That number
40+
is pre-existing state being reported for the first time — not damage the audit
41+
caught being done, and not a regression introduced by looking at it.
42+
43+
For the same reason `provenance` **alone does not raise the summary warning**.
44+
On a database of any age it is non-empty on every healthy run, and a line that
45+
always fires is the #4747 broken alarm again — it would train its reader
46+
straight past the run where `dangling` had something in it. The counts ride
47+
along in the payload whenever the line fires for a real finding, and the
48+
itemised rows are always in the returned report. `provenanceUndetermined` is
49+
separate from `undetermined` for the same reason: on a stack that never
50+
registers `sys_user`, every provenance value probes "cannot tell", and that is
51+
a fact about which platform tables are mounted, not about the audited data.
52+
53+
Scan order gained a third tier to keep the change from costing the signal it
54+
sits next to: security surface, then objects carrying a business reference,
55+
then the provenance-only remainder. Admitting the family means nearly every
56+
object now has an auditable field, so without the tier a bounded run would
57+
spend its row budget on tables carrying only provenance and never reach the
58+
business findings the budget was built for.
59+
60+
Part of #4743 (fact 2). The stale `assertReferencesResolve` comment in
61+
`engine.ts` (fact 1) is deliberately untouched here.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
'@objectstack/service-settings': patch
3+
---
4+
5+
Settings: an `OS_*` env override is now checked against the specifier's declared `options` table (#5204)
6+
7+
A manifest's `options` table has been enforced on the write path since #5131, but
8+
`SettingsService.get()` produced an effective value by a second route that never
9+
consulted it: an `OS_*` override was reshaped by the default's type and returned
10+
straight from the top of the cascade with `locked: true`. So the providers #5094 and
11+
#5133 retired from `mail.provider` could walk back in through the one door with no
12+
gate on it — `OS_MAIL_PROVIDER=sendgrid` reached the mail plugin unchallenged — and a
13+
plain typo such as `OS_BRANDING_THEME_MODE=drak` was served to every consumer as a
14+
normal value with normal-looking provenance, each consumer left to improvise.
15+
16+
An override whose value the table does not declare is now **ignored** rather than
17+
repaired: the value falls through to the next layer of the cascade (a stored
18+
global/tenant/user value, else the manifest default), and the read API reports that
19+
layer honestly instead of claiming `source: 'env'` for a value not in force. The
20+
rejection is logged once at `error`, naming the variable, the rejected value, the legal
21+
value set and the consequence. The same audit runs at `registerManifest`, so a
22+
misconfigured deployment learns at boot rather than whenever somebody first opens the
23+
settings page.
24+
25+
Registration **reports but never refuses**: option tables move, a pin that was legal
26+
the day it was written must not turn an upgrade into a crash-on-start.
27+
28+
Two behaviour notes for anyone relying on the old shape:
29+
30+
- Keys with no declared option table are untouched — text, boolean, number and
31+
password overrides behave exactly as before. The check applies only to
32+
`select`/`radio`/`multiselect` specifiers that declare a non-empty table.
33+
- A **rejected** override no longer pins its key against writes. `setMany` used to
34+
refuse on the mere presence of the variable; judged by presence, an ignored value
35+
would have left the key configurable by nothing at all — env value discarded, UI
36+
refused with `SETTINGS_LOCKED`, and `get()` reporting `locked: false` to a settings
37+
page whose save would then fail. An override that *is* in force still locks the key,
38+
unchanged.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): `HookContext.session` declares `positions` and `preserveAudit` (#5605)
6+
7+
Two keys the engine has been **producing** all along, that consumers have been
8+
**reading** and the docs have been **teaching**, were missing from the contract:
9+
`HookContextSchema.session` declared `userId` / `actor` / `organizationId` /
10+
`accessToken` / `isSystem` / `skipTriggers` / `skipAutomations` and nothing else.
11+
Both are now declared, per the maintainer ruling on #5605.
12+
13+
This is the mirror of the `session.roles` retirement (#5050). That key was
14+
declared-never-produced, so it was removed; these two are
15+
produced-never-declared, so they are added. Same `session` block, opposite
16+
drift, opposite fix.
17+
18+
**What was broken.** `HookContextSchema` is deliberately not `.strict()` — it is
19+
the runtime shape the engine hands a handler, and strictness there would make
20+
every engine-side enrichment a breaking change for anyone parsing a context they
21+
were given. The cost of that tolerance is that an undeclared key is **stripped
22+
in silence**:
23+
24+
- `HookContextSchema.parse(ctx)` — the exact call the generated reference page
25+
documents as the way to consume a context — returned a session with the
26+
caller's `positions` and the import's `preserveAudit` dropped on the floor.
27+
- A handler typed the way the automation docs teach, `(ctx: HookContext)`, could
28+
not read either key: `ctx.session?.positions` was a `TS2339`. The two
29+
`kernel/runtime-services` pages that teach
30+
`positions: ctx.session?.positions` compiled only because they annotate `ctx`
31+
as `any` — copying both the code and the documented annotation did not build.
32+
33+
**`positions`** (`string[]`, optional) is the ADR-0090 D3 placement vocabulary,
34+
copied verbatim from `ExecutionContext.positions` by ObjectQL's `buildSession()`.
35+
Its `.describe()` states the boundary the ruling asked for, because the boundary
36+
is the whole reason this key needed a decision rather than a patch: it is
37+
**readable context, never an authorization input**. A hook may forward it as the
38+
sharing service's evaluation context, tailor a message, or log it. A hook must
39+
not make the access decision itself by testing it — privilege is judged by the
40+
security service on the execution context (capability grants `permissions`,
41+
placements `positions`, and the derived posture). A hook re-deciding access from
42+
this array decides, somewhere with no access to the grant model, something that
43+
was already decided; that is structurally the mistake the `roles` tombstone
44+
exists to prevent, one vocabulary later.
45+
46+
**`preserveAudit`** (`boolean`, optional) is the #3493 historical-import flag:
47+
server-set, opt-in, absent on normal writes, and read by the built-in audit hook
48+
to keep a caller-supplied `updated_at`/`updated_by` instead of stamping the
49+
import instant. It has a live consumer, so it could only ever be declared.
50+
51+
Purely additive — both keys are optional, the shape stays non-strict, and no
52+
existing context, handler or stored metadata changes. Contexts are built per
53+
operation and never persisted, so there is nothing to migrate.

0 commit comments

Comments
 (0)