Skip to content

Commit abdb620

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-6259-batch-alias-row-retirement
2 parents d4e15b6 + 8b82686 commit abdb620

84 files changed

Lines changed: 5294 additions & 216 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: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
'@objectstack/spec': major
3+
'@objectstack/service-automation': patch
4+
'@objectstack/plugin-approvals': patch
5+
---
6+
7+
refactor(spec)!: retire `ActionDescriptor.isAsync` — a second spelling of `supportsPause` that nothing ever read (#6748, ADR-0049)
8+
9+
<!-- adr-0087: registered action-descriptor-is-async-retired -->
10+
11+
**FROM → TO:** `isAsync: true` → delete the key; declare `supportsPause: true` (plus the
12+
`resumeAuthority` its pauses need) and return `suspend: true` from `execute()`.
13+
`isAsync: false` → delete the key; there was never anything to preserve.
14+
15+
`ActionDescriptor.isAsync` declared "suspends the flow awaiting an external reply" and no
16+
execution path read it. Measured fresh before removal across all three repos — objectstack,
17+
objectui and cloud — with zero property reads: every hit was the declaration itself, a
18+
generated baseline, one of five shipped descriptors WRITING it, a fixture pinning the
19+
shape, or prose. Declaring it never made a node suspend; omitting it never stopped one.
20+
21+
This is the remove leg of the ADR-0049 disposition its sibling took the other way. The two
22+
keys said the same thing — "this node type can suspend the run" — and #6667 split them by
23+
evidence: `supportsPause` became an enforced fact (`AutomationEngine` now refuses a
24+
suspension whose type does not declare it, at the one seam every suspension passes
25+
through), while `isAsync` had no consumer to grow into. Keeping both would leave the
26+
platform publishing two names for one capability with only one of them honoured — and
27+
`screen` declared BOTH, so a plugin author copying it had no way to tell which.
28+
29+
The retirement kit:
30+
31+
- **Tombstone, not deletion** (`retiredKey()`): `ActionDescriptorSchema` is not `.strict()`,
32+
so a plain delete would let existing descriptors parse clean and lose the key in silence
33+
(the ADR-0104 shape). Authoring `isAsync` now fails `tsc` at the descriptor literal and
34+
fails the parse inside `defineActionDescriptor()` — with the prescription in the message.
35+
- **ADR-0087 D3 `SemanticMigration`** (`action-descriptor-is-async-retired`) plus the exact
36+
`RETIRED_KEYS_BY_MAJOR` entry. No D2 conversion, deliberately: a descriptor is published
37+
from an executor's TypeScript and never stored in stack metadata, so there is no source
38+
for `os migrate meta` to rewrite — the `EnhancedApiError.fieldErrors` disposition.
39+
- The five shipped writers stop writing it (`screen`, `map`, `wait`, `approval`,
40+
`approval_revise`); the descriptors they publish lose the key, which is why the two
41+
runtime packages appear here.
42+
- Generated baselines (`authorable-surface/automation.json` gains `[RETIRED]`,
43+
`authorable-defaults/automation.json` loses the default line), `spec-changes.json`, the
44+
upgrade guide and the reference docs regenerated.
45+
46+
No runtime behaviour changes — that impossibility is the reason for the removal. The same
47+
commit also corrects `supportsPause`'s TSDoc, which still described itself as a declaration
48+
no execution path reads; #6667 made that false (#6749).
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
<!-- adr-0087: not-required (no-migration-prescription) this change retires NO key. The endpoint vocabulary is byte-identical and only the unknown-key POSTURE moves, from strip to reject. Nothing exists for `objectstack migrate meta` to rewrite, because an undeclared key was never honoured in the first place: it was dropped at parse and so never reached storage, the matcher or the executor. No stored shape carries one, and no authored shape that ever WORKED becomes invalid. There is also no single FROM/TO rule a ledger entry could state, since what is now refused is an open set of author typos rather than a renamed key. The upgrade channel is the schema rejection itself, which is strictly more specific than any ledger line: it names the offending key at the author's own path and carries either the canonical spelling or a wrong-layer pointer at the surface that really owns the key. The one ADR-0087 entry that DOES govern this surface, `declarative-apis-endpoints-live`, is already registered for protocol 17 and is updated by this PR rather than duplicated. Measured blast radius: 0 affected entries across `examples/*` (2 endpoints) and the `cloud` repo (0 endpoints). -->
6+
7+
`ApiEndpointSchema` rejects undeclared keys (#5384), and the author-state type is named on the upgrade path (#5227)
8+
9+
`api` became a registered metadata type at #5312, which made
10+
`packages/spec/src/api/endpoint.zod.ts` an AUTHORING surface — `defineStack({ apis })`,
11+
the Studio metadata-admin form, and `PUT /meta/api/:name`'s 422 — while it was still a
12+
plain open `z.object`. An undeclared key was therefore dropped on every path: a
13+
`cacheTTL` / `objectParam` / `outputMappings` typo parsed green, published green, and the
14+
endpoint then served without the policy or projection its author wrote. That direction is
15+
fail-safe for `authRequired` alone (an unrecognized spelling leaves the default `true`
16+
standing); it was never fail-safe for the mapping, cache and rate-limit blocks.
17+
18+
The shape is now `strictObject`, so an undeclared key is a named rejection carrying the
19+
surface, the offending key and a rename. Two curated wrong-layer prescriptions ship with
20+
it:
21+
22+
- **`namespace`** — ADR-0121 D2 derives the namespace segment of `path` from
23+
`manifest.namespace`; it has never been per-endpoint, so the rejection points at the
24+
manifest instead of suggesting a rename.
25+
- **the six stored-envelope bookkeeping keys** (`packageId`, `state`, `version`,
26+
`published*`) — written onto the stored ROW by `register` / `publishPackage`, never onto
27+
a declaration.
28+
29+
**The order this landed in is the part worth keeping.** Closing the shape was measured and
30+
REFUSED first: the same schema parsed STORED rows at `buildEndpointIndex` and
31+
`gateApiItemsForPublish`, so a naked `strictObject` failed every row with
32+
`unrecognized_keys: ['packageId', 'state']` — the load-time backstop excluded the endpoint
33+
(404) and the publish gate reported a schema error in place of the ADR-0121 D6 verdict it
34+
exists to give. The debt was real and it was not in this vocabulary, so #5309 (PR #6576)
35+
paid it at the layer that owned it (`peelStoredEnvelope`). `ApiEndpointSchema` never
36+
learned a bookkeeping key.
37+
38+
**Breaking for metadata that was already silently broken.** An `apis:` entry carrying an
39+
undeclared key now fails `objectstack validate`, `objectstack build` and the metadata write
40+
path instead of publishing with the key discarded. Measured before landing: the example
41+
corpus (2 endpoints) and the `cloud` repository (0 endpoints) carry zero undeclared keys,
42+
so nothing in-tree changes verdict.
43+
44+
`api` also leaves the #4001 campaign's `STILL_STRIP` list — closed registered types 24 → 25
45+
of 26, with `view` the only entry left — and the CLI metadata gate's row moves from
46+
`NOT_YET_CLOSED` into `GATED_AT`.
47+
48+
**#5227** — no schema change. `ApiEndpoint` already denotes the AUTHOR state after ADR-0122
49+
phase 2, so omitting `authRequired` compiles; what was missing was anywhere saying so. The
50+
`declarative-apis-endpoints-live` upgrade-guide entry, whose whole safety argument is that
51+
"an omission is SAFE", now carries the type annotation that makes the omission writable:
52+
annotate declarations `ApiEndpoint`, hold parse results as `ApiEndpointParsed`.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/plugin-security': minor
4+
---
5+
6+
security: add a fail-closed authored-row-write verdict to `ISecurityService`
7+
8+
`ISecurityService` gains an optional, verdict-shaped, by-id method:
9+
10+
```ts
11+
checkAuthoredRowWrite?(
12+
object: string,
13+
recordId: string,
14+
operation: AuthoredRowWriteOperation, // 'update' | 'delete'
15+
context?: SecurityContext,
16+
): Promise< AuthoredRowWriteVerdict >; // 'admit' | 'abstain'
17+
```
18+
19+
It answers one question no existing surface could: does an **app-authored**
20+
row-level security policy admit this row for this write, on its own, with the
21+
platform's ownership floor taken out by provenance?
22+
23+
Every other method reports the **composed** RLS verdict, and sitting inside that
24+
composition is the platform's own wildcard write floor (`created_by ==
25+
current_user.id`, shipped on the `member_default` baseline every authenticated
26+
member resolves additively). So "the composed RLS admits this row" is true for
27+
the row's CREATOR whether or not any app policy mentions it — which makes it a
28+
measurably different question, not a cheaper spelling of the same one. A caller
29+
deferring to the composed answer would hand transferred records back to their
30+
former creators.
31+
32+
`admit` iff at least one applicable, non-floor policy matches the row for the
33+
operation. `abstain` in every other case — no authored policy, no match, an
34+
unreadable or cross-tenant row, a principal-less or on-behalf-of context, or any
35+
internal failure. The method never throws outward, and it is **optional**: a
36+
deployment whose security service omits it behaves byte-for-byte as before,
37+
because callers feature-detect and read absence as `abstain`.
38+
39+
`@objectstack/plugin-security` implements it on the registered `security`
40+
service, reading the verdict off the same layered RLS computation the middleware
41+
enforces with — no second RLS evaluator.
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
docs(spec): D3 迁移账本里 `resumeAuthority` 那条的验收标准不再要求手工排查 `supportsPause` 不一致 —— #6746 之后引擎自己会拒绝 (#6844)
6+
7+
`MIGRATIONS_BY_MAJOR[17].semantic``action-descriptor-resume-authority-default-flip`
8+
一条的 `acceptanceCriteria`,结尾的 ⚠️ 子句写的是:
9+
10+
> `supportsPause` is a declaration nothing enforces (#5703), so an executor whose
11+
> `execute()` returns `suspend: true` while leaving `supportsPause` false is warned
12+
> about by NEITHER channel — check those by hand against the same rule.
13+
14+
写下时属实,PR #6746(#6667,2026-08-08 合入)之后不再属实:
15+
`AutomationEngine.refuseUndeclaredSuspension` 正是拒绝这一类执行器的,拒绝点在
16+
`executeNode` 里每个 `result.suspend === true` 都要经过的那一个接缝
17+
(`packages/services/service-automation/src/engine.ts`)。于是一个 v17 升级者被告知
18+
去手工审计一类引擎已经自己拒绝的错误,而且被告知这个不一致「没有声音」,实际上它很响。
19+
20+
这不是内部注释:该字符串被 `gen:upgrade-guide` 逐字投影进
21+
`docs/protocol-upgrade-guide.md` 的 "Done when" 行,而那正是升级作者(依 ADR-0033,
22+
往往是 AI)读的那一行,所以本 PR 一并提交了重新生成的产物。
23+
24+
**改后的说法,以及为什么不是「什么都不用查了」**:两条告警通道(启动告警、
25+
`check:resume-authority-declared`)确实仍然都以 `supportsPause: true` 为触发条件,
26+
所以「不被任何告警通道覆盖」这半句保留;变化的是它现在会在运行时被拒绝,且属于
27+
guard-class —— `fault` 边路由不了(`refuseNode` 打的是 `errorClass: 'guard'`,
28+
`executeNode``errorClass === 'guard'` 时不去找 `fault` 边)。同时刻意没有过度
29+
更正:该守卫明确不判「完全没有注册描述符」的执行器(engine.ts 中
30+
"What it does NOT judge → Silence",由 `supports-pause-runtime-enforcement.test.ts`
31+
的 descriptor-less 用例钉住),这类执行器的暂停照样会被创建,只在 resume 路由上才
32+
被拒绝,所以这一条作为仍需留意的残留被写进了新文案。
33+
34+
受理面(schema 接受什么)逐字节未变 —— 本条只动账本散文与其生成产物。
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/driver-turso": minor
4+
"@objectstack/driver-sqlite-wasm": minor
5+
"@objectstack/spec": minor
6+
"@objectstack/objectql": patch
7+
---
8+
9+
feat(drivers,spec)!: `GroupByNode.alias` is honoured by the SQL faces — one aggregate, one column key (#6401)
10+
11+
`GroupByNodeSchema` has declared `alias` ("Alias for the projected group
12+
value", defaulting to `field`) for as long as the structured `groupBy` entry has
13+
existed. Exactly one execution path read it. The result: the SAME query came
14+
back with a different result-column key depending on which path the engine
15+
happened to take.
16+
17+
```ts
18+
groupBy: [{ field: 'closed_at', dateGranularity: 'month', alias: 'qtr' }]
19+
```
20+
21+
- pushed down to a driver ⇒ rows keyed **`closed_at`**
22+
- run through the in-memory fallback ⇒ rows keyed **`qtr`**
23+
24+
And the choice between them is `engine.ts`'s
25+
`allStructuredSupported && !tzRequiresInMemory` — a driver capability bit and a
26+
`timezone`, neither of which the caller can see. That is the multi-face
27+
consistency invariant broken in its quietest form: both answers are valid rows,
28+
so nothing throws and nothing looks wrong.
29+
30+
**Resolved to ENFORCE**, and the leg was chosen by measurement rather than
31+
taste. ADR-0049 splits on whether the feature already exists: a *dangling*
32+
promise is removed, a *live* one with a missing gate is enforced. `alias` is
33+
live — three consumers read it and change behaviour
34+
(`in-memory-aggregation.ts`, `MemoryDriver.performAggregation`, and
35+
`chartAggregateCategoryKey`), and the publish gate *compels* it:
36+
`validate-react-page-props.ts` errors `REACT_CHART_AXIS_UNKNOWN` unless a
37+
chart's category axis is bound to `alias ?? field`, telling the author in so
38+
many words to "bind it to" the alias. A key the build gate makes you write is
39+
not a dangling promise. The count of real non-test producers is **zero**, which
40+
is what makes enforcing safe rather than what argues against it: no shipped
41+
payload changes its result keys.
42+
43+
**What changed, on every SQL face at once** — a fix landing on one and not its
44+
twin is the #6203 shape, and `TursoDriver` picks its face from `url`:
45+
46+
- **`driver-sql`** — both limbs of the structured `groupBy` branch project
47+
`alias ?? field`: the date-bucket limb aliases the bucket expression to it,
48+
and the plain limb emits `?? as ??` (only when the name actually moves — an
49+
alias equal to the field emits no self-rename). `presentedOutput` is now keyed
50+
by the OUTPUT column, matching how the aggregation branch beside it has always
51+
worked; an aliased group value went unpresented before.
52+
- **`driver-turso` REMOTE** — the same projection, `"field" AS "alias"`. The
53+
alias reaches the statement as a quoted identifier and is therefore held to
54+
`assertSafeIdentifier`, exactly like `field`.
55+
- **`driver-sqlite-wasm`** — inherits `SqlDriver`'s compiler; covered by its own
56+
conformance suite rather than by assumption.
57+
58+
**GROUP BY still keys on the FIELD** on every face. Only the projection is
59+
renamed, so the buckets are unchanged. This is deliberate and pinned: SQLite
60+
resolves output names in `GROUP BY`, so a face that grouped by the alias would
61+
look correct here and diverge on a dialect that does not.
62+
63+
`having` needed no change and now means one thing: it is applied over the
64+
aggregated row's own columns, so a filter on a group projection references the
65+
alias on every path — previously the alias on one path and the field on the
66+
other.
67+
68+
**Conformance.** `AGGREGATION_CASES` (#6409) gains a `groupByAlias` axis and two
69+
cases. Their VALUES are an existing case verbatim — only the key moves — so they
70+
can fail only on the key, which is the point: every wrong answer in this area is
71+
a valid query returning plausible rows. `objectql`'s in-memory fallback is now
72+
**enrolled** as a fourth face, answering #6409's open question ②: it is the face
73+
the SQL three were converged onto, so the new behaviour would otherwise be
74+
pinned against nothing, and reaching it needs no engine at all —
75+
`applyInMemoryAggregation` is a pure function of rows and an AST.
76+
77+
**Reverse verification**, predicted before running. Reverting the in-memory face
78+
to `g.field`: only the two alias cases move and only ONE fails — the degenerate
79+
`alias === field` case stays green, which is why both are in the table.
80+
Reverting the harness to read `c.groupBy` instead of `c.groupByAlias ?? c.groupBy`
81+
— the copied-neighbour mistake: everything passes on an unmodified face, a false
82+
GREEN, which is the failure mode that would have made the axis vacuous.
83+
84+
**Frozen drivers (#5499), measured from source, not flipped.** `driver-memory`
85+
already returned `{ field, alias: node.alias ?? node.field }` and projects under
86+
the alias — it had independently reached the enforce answer, so it needed no
87+
alignment. `driver-mongodb` is a recorded DEBT row and the defect is wider than
88+
`alias`: `buildAggregationPipeline` types `groupBy` as `string[]` and builds
89+
`groupId[field] = '$' + field`, so a structured node — aliased or not — becomes
90+
the literal key `"[object Object]"`. It cannot take a structured `GroupByNode`
91+
at all; `mongodb-driver.ts` passes `(query as any).groupBy`, which is why `tsc`
92+
never saw it. Tracked on #6814.
93+
94+
**Compatibility.** A caller who writes `alias` and reads the result under
95+
`field` on a pushdown path will now find the value under `alias` — which is what
96+
the key has always meant on the fallback path, and what the chart gate already
97+
required. Callers who never write `alias` are unaffected: the emitted SQL is
98+
byte-identical.
99+
100+
<!-- adr-0087: not-required (no-migration-prescription) Nothing is retired: `GroupByNodeSchema.alias` keeps its declaration, its spelling and its type — it starts being HONOURED by three faces that parsed and ignored it. There is no tombstone to write and no authored metadata to rewrite, so there is no mechanical transform a migration could prescribe: every stack that validated before validates after, unchanged. The behaviour change is in the RESULT of a runtime query (a result-column key moves from `field` to `alias` on the pushdown path, converging on what the in-memory path and the chart publish gate already required), which the ledger has no channel for and no upgrader could apply a codemod to. The bang is on the changeset because callers who read that column by the field name must move, and the measured non-test producer count for the key is zero. -->
101+
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
rest 的异步导入行数上限改为直接读取 spec 的 `IMPORT_JOB_MAX_ROWS` 导出,不再自己声明一份同值字面量(#6535)。
6+
7+
**行为没有任何变化**:两处此前都是 `50_000`,改后仍是 `50_000`,上限、`413` 文案、拒绝边界全部不动。
8+
这是一次一致性收敛,不是缺陷修复——因此按 patch 计。
9+
10+
收敛掉的是一处漂移面:`packages/spec/src/api/export.zod.ts` 的那份导出带着 TSDoc,是这个
11+
上限的**对外说明**(喂给生成的 reference 表面);而真正执行拒绝的是 `packages/rest`,它此前
12+
读的是自己那份本地 `const`,两者之间只有一句 "mirrors spec" 注释相连。没有任何 gate 比较这
13+
两个数——`api-surface/api.json` 只记下 `"IMPORT_JOB_MAX_ROWS (const)"` 这个**名字**,不记它的
14+
****——所以把 spec 那份改成 20_000、执行侧纹丝不动,`pnpm test` 与全部 `check:*` 依然全绿
15+
(本 PR 实测过)。失效方向是文档说一套、系统做一套,而 `413` 文案里内插的又是 rest 那一份,
16+
连报错都会自洽地说谎。现在一处定义、两处读点(`maxRows:``413` 文案)同源。

0 commit comments

Comments
 (0)