Skip to content

Commit 94af2a1

Browse files
committed
Merge origin/main into claude/issue-5503-autonumber-readonly-strip
Conflict: packages/objectql/src/engine.ts — the `insert()` doc comment only, where #5610 (strictReadonlyWrites, #5126) and #5503 each rewrote the same block. Resolved by SUPERPOSITION, not by taking a side. Both incoming notes carried the same standing condition, and #5503 is what discharges it: - #3407: "if insert ever gains a silent strip, wire the listener at that strip site"; - #5126: "`strictReadonlyWrites` is inert here ... because insert strips nothing. If insert ever gains a strip, both members wire up together at that site." #5503 gives insert a strip (caller-supplied runtime-owned `autonumber`), so both members of `WriteObservabilityOptions` now discharge at that site: - default → strip, commit, report via `onFieldsDropped` (unchanged); - strict ON → `ReadonlyFieldRejectedError` before any driver call, and the listener deliberately does NOT fire (a refused write did not complete — #5126's own design point, mirrored). UPDATE needed no new code: the autonumber limb rides `stripReadonlyFields` → `reportDroppedFields` → `assertNoStrictDrops`, the seam #5126 already built, so a caller-supplied record number is refused at equal rank with a declared `readonly` field for free. Pinned by test rather than assumed. `ReadonlyFieldRejectedError` gained an optional `operation` (default `'update'`, so #5126's UPDATE message stays byte-identical) because the verb and the remedies genuinely differ: an INSERT refusal is always about a runtime-owned value, whose exempt writers are `isSystem` and the `preserveAudit` historical import, while `readonlyWhen` cannot lock anything on a create. No new policy was invented at the seam. #5126 states that strict "adds no second policy — it reports the existing one", and that a field the strip does not take is not rejected either; applying that verbatim is what keeps the `isSystem` / `preserveAudit` exemptions accepted under strict. Pinned in tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx
2 parents 13f63c2 + 214f67c commit 94af2a1

72 files changed

Lines changed: 5070 additions & 302 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: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
Put the 44 pre-helper alias tables under the alias-integrity gate — including the
6+
collision claim they were never measured on.
7+
8+
`alias-integrity.test.ts` (#5013) judges alias tables built by `strictObject`,
9+
which registers each one alongside the `.shape` it makes claims about. The 44
10+
call sites that predate that helper call `strictUnknownKeyError` directly and
11+
hand it a transcribed `knownKeys` array; with no shape to register, they sat
12+
outside every claim the gate makes.
13+
14+
`strictUnknownKeyError` now records its own `{ surface, knownKeys, aliases,
15+
guidance }` in an internal registry, and the gate judges that batch too. The
16+
registration lives in the factory, so no schema module changed.
17+
18+
What each claim is worth on this batch differs, and the gate says so rather than
19+
implying uniform coverage:
20+
21+
- **alias key is not a known key** and **alias target is a known key** are
22+
answered against the transcribed array. If that array has drifted from its
23+
schema, both answers inherit the drift — the gap `strictObject` exists to
24+
abolish, and only migrating a call site closes it (#5593).
25+
- **no two alias keys share an `aliasProbe`** (#5481) loses nothing: a collision
26+
is a property of the table alone. These tables were **unmeasured** on it, not
27+
clean — #5481 postdates the measurement recorded in #5483 — and the sweep they
28+
had never had comes back clean at 52 tables.
29+
30+
The target claim found a real one on its first run: `ui/app.zod.ts` shares four
31+
"start expanded" aliases across all nine navigation-item variants, but `expanded`
32+
is declared on the `group` variant alone, so on the other eight the suggestion
33+
names a key that variant also rejects — the second rejection this campaign
34+
exists to end. Filed as #5555 (the fix rewrites author-facing message text) and
35+
pinned here shrink-only, structurally, so nothing else can enter the tolerance.
36+
37+
Two supporting pieces, both deliberately visible rather than implicit: the walk
38+
now forces error maps that build themselves on first use (`data/object.zod.ts`
39+
defers its map around a temporal dead zone, and would otherwise never register),
40+
and the six prose alias targets on `ui/app.zod.ts`'s navigation items —
41+
`type: 'url' (with url)` and friends, which name a variant rather than a key —
42+
are an enumerated allowlist scoped to that surface, with a staleness check that
43+
fails if an entry stops being used.
44+
45+
The shrink-only ratchet at 44 is unchanged. What it discourages — a new call
46+
site minting a fresh second copy of a key list — is exactly as undesirable as it
47+
was before these tables gained a guard.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): `callData('delete', …)` 的 ObjectQL 兜底返回 spec 声明的 `{ object, id, success }`,与 protocol 路径同形 (#5581)
6+
7+
`callData` 是 protocol 优先 + ObjectQL 兜底,两条路径此前对「删除成功」给的是两种形状:
8+
9+
| 路径 | 此前 | 现在 |
10+
|---|---|---|
11+
| protocol(`deleteData`) | `{ object, id, success: true }` | 不变 |
12+
| ObjectQL 兜底 | `{ object, id, deleted: true }` | `{ object, id, success: true }` |
13+
14+
规范只有一个:`DeleteDataResponseSchema`(`packages/spec/src/api/protocol.zod.ts`)声明的是
15+
`{ object, id, success }`,`deleted` 从未被任何 schema 声明;公开的 HTTP 文档
16+
(`content/docs/protocol/kernel/http-protocol.mdx`)也一直写的是 `success`。所以兜底是唯一
17+
的偏离方,protocol 路径与 spec、与文档都无需改动。
18+
19+
这是 #5138 同一族缺陷的成功侧:#5138 收敛的是「记录不存在」的答案,本次收敛的是「删除成功」
20+
的答案 —— 后者是每一次正常请求都会走到的面,而非只在 id 写错时才碰到。此前按
21+
`DeleteDataResponseSchema` 写的客户端,在**未注册 `protocol`**的精简装配上会从一个 HTTP 200
22+
里读到 `success === undefined`,即「删除到底成没成功」读不出来,而调用方无从分辨自己走的是哪条
23+
路径。消费端各自兼容 `success ?? deleted` 两种拼写正是 contract-first 禁止的形状,所以修在
24+
生产方,不在消费方。
25+
26+
## ⚠️ 升级须知(行为变化)
27+
28+
**仅影响没有安装 `MetadataPlugin`(`@objectstack/metadata-protocol`,即注册 `protocol` 槽)的
29+
精简装配。** 装了该插件的部署走 protocol 优先路径,本来就返回 `success`,不受影响。
30+
31+
在这类精简装配上,以下三个面的 `DELETE` 成功体键名由 `deleted` 改为 `success`:
32+
33+
- `DELETE /api/v1/data/:object/:id`
34+
- MCP 的 `delete_record` 工具(`domains/mcp.ts``remove` 桥)
35+
- 声明式端点(`objectParams.operation: 'delete'`,#5092)
36+
37+
若你的代码读的是 `response.data.deleted`,请改读 `response.data.success` —— 这也是 spec 与
38+
公开文档自始至终声明的键。删除行为本身(含 #5138 落的「记录不存在则 404 `RECORD_NOT_FOUND`
39+
且不发出写」)完全未变,变的只有成功体拼写这一个键。
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): `os doctor` no longer treats a broken `@objectstack/cloud-connection` install as "not installed" (#5644)
6+
7+
`readInstalledPackageEntries()` reached the installed-package ledger through a
8+
dynamic `import('@objectstack/cloud-connection')` whose `catch` meant "the
9+
optional package is not installed". That is right for one of the two things it
10+
caught:
11+
12+
- **The specifier does not resolve** — the optional package really is not
13+
there. Silence is correct and unchanged: `os doctor` must run to completion in
14+
a checkout that never had it.
15+
- **The package is installed and will not load** — a pruned or unbuilt `dist/`,
16+
an interrupted install, an artefact that throws while it evaluates, a
17+
transitive dependency missing under it. It threw too, so it was answered with
18+
the same silence.
19+
20+
The ADR-0120 D5e unique-scope advisory then saw "no installed packages", found
21+
nothing to report, and the run printed:
22+
23+
```
24+
✓ Unique scope No unconfirmed installation-wide uniques for this 'isolated' environment
25+
```
26+
27+
Measured: with the package present-but-unloadable and a ledger declaring an
28+
installation-wide `unique`, that line was printed and the finding appeared
29+
nowhere, `--verbose` included. It is the same false PASS #5412 removed at the
30+
`readdir` boundary and #5413 at the entry boundary, one boundary further up —
31+
and `os serve`, loading the same package in the same directory, has always named
32+
the failure out loud.
33+
34+
The two states are now separated by **resolution**, not by the `import()` having
35+
thrown (`isModuleNotFoundError()` first — an error that is not a module-not-found
36+
error came from the package itself, so it is present by definition; then
37+
`import.meta.resolve()`, which answers "is the package there" without stating its
38+
entry file, unlike `createRequire().resolve()`). Only the genuinely-absent half
39+
is silent. The other prints an ordinary `HealthCheckResult` through the same
40+
renderer every other check uses:
41+
42+
```
43+
⚠ Unique scope Could not load the installed-package ledger reader (installed packages
44+
NOT checked for installation-wide uniques) — Cannot find module …
45+
```
46+
47+
**Warning, not error**, and the exit code is unchanged, matching its two
48+
siblings: the environment still runs; what broke is doctor's ability to see part
49+
of it. The cause is quoted from the thrower, and `--verbose` expands it together
50+
with the remedy — reinstall, or build the package in a monorepo checkout.
51+
52+
The row is **not** conditional on `.objectstack/installed-packages/` existing.
53+
Doctor cannot honestly say a ledger is absent when the constant naming the
54+
ledger's location is an export of the package that would not load.
55+
56+
One consequence worth stating: in a monorepo checkout where
57+
`packages/cloud-connection` has not been built, `os doctor` under the `isolated`
58+
posture now prints this warning instead of a clean bill. That state is exactly
59+
what sent #5612 chasing a report face that had never regressed.
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
fix(lint): report a `config.timeRelative` descriptor the sweep will refuse, at authoring time (#5496)
6+
7+
A flow start node declaring `config.timeRelative` got **zero** authoring-time
8+
diagnostics when its descriptor could not parse. The two rules that look at the
9+
slot each looked at something else: `lint-flow-patterns` decides "this is a
10+
time-relative flow" from `timeRelative != null` alone (never the shape), and
11+
`validate-flow-trigger-readiness`'s existing check reads only
12+
`timeRelative.object`, to compare it against the stack's objects. So
13+
14+
```ts
15+
config: { timeRelative: { object: 'task', field: 'due_at', offsetDays: -1 } }
16+
```
17+
18+
— three separate schema violations: `dateField` missing, `offsetDays` declared
19+
as an int **array** and written as a scalar, and `field` an unrecognized key —
20+
passed `os validate` silently. `TimeRelativeTriggerSchema` does reject it, but
21+
the only place that schema ran was **bind time**, inside
22+
`TimeRelativeTriggerPlugin.start()`, which warns and returns: the sweep is never
23+
installed, the flow reports itself armed, and the author's sole feedback is one
24+
line in a server log. For an AI author that line is outside the feedback loop
25+
entirely; `os validate` is what it reads.
26+
27+
**New rule — `flow-time-relative-descriptor-invalid` (warning).** A start node
28+
whose `config.timeRelative` is present runs that same schema at authoring time,
29+
and a failure is reported naming `config.timeRelative` with the schema's own
30+
issue list forwarded — so the diagnostic carries the missing key, the wrong type,
31+
and, for an unrecognized key, the "did you mean" the schema already computes
32+
(`field``dateField`) plus its wrong-layer guidance (a `schedule` written
33+
*inside* the descriptor is told it belongs beside it). The list is rendered
34+
exactly as the bind-time warning renders it, so the two channels tell one story.
35+
36+
Nothing is shifted except **when** the schema runs. No shape knowledge is
37+
re-implemented in the rule and no consumer-side tolerance is added: the verdict
38+
and every word of its wording remain `TimeRelativeTriggerSchema`'s, so the rule
39+
tracks the descriptor's contract as it evolves instead of drifting from a second
40+
copy of it.
41+
42+
The rule and the existing object-name check decide different facts and cannot
43+
report the same one twice — only the stack knows whether an object name exists,
44+
and only the schema knows the descriptor's shape. A descriptor wrong in both ways
45+
gets both findings, at their own paths. Canonical descriptors are unaffected:
46+
every one shipped in the repo (the showcase `Task Due Reminder`, the
47+
`content/docs` examples) parses, so this adds no diagnostic to existing apps.
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
---
2+
"@objectstack/spec": major
3+
---
4+
5+
refactor(spec)!: retire `HookContext.session.roles` — declared, read by two dead branches, never produced (#5050)
6+
7+
`session.roles` on the runtime hook context was ADR-0049's enforce-or-remove
8+
case with **neither end**: it was declared in `data/hook.zod.ts`, read by
9+
exactly two consumers, and produced by nobody. The two readers were
10+
plugin-approvals' admin exemptions — the approval record lock and the delegation
11+
write guard, each opening with `session.roles?.includes('admin')` — and both
12+
were deleted in #4839 (PR #5049) on the maintainer's ruling. The producer side
13+
was empty the whole time: ObjectQL's `buildSession()` builds the session field
14+
by field (`userId`, `organizationId`, `accessToken`, `isSystem`, `actor`, the
15+
skip flags) and has never written `roles`, and nothing else feeds a
16+
HookContext. So every read resolved `undefined`,
17+
and a guard keyed on it was dead code that merely LOOKED like an authorization
18+
decision — plus a second admin dialect competing with the one ADR-0090 D3 /
19+
ADR-0095 D3 sanction.
20+
21+
Cross-repo consumer check ran in both directions before removing anything
22+
(#4895's discipline, after #4865's tombstone was disproven by objectui):
23+
`cloud` has zero `session.roles` while its hook consumers really do read
24+
`hookContext?.session?.userId` (`service-cloud/src/marketplace-visibility-plugin.ts`,
25+
`control-plane-org-scope-plugin.ts`) — a positive control in the same run;
26+
`objectui` has zero, and its `roles` are the `/auth/me` **user** payload, a
27+
different surface that is untouched.
28+
29+
One neighbour is called out rather than folded in, because mistaking it for a
30+
producer would read as refuting the whole finding (#4865's lesson): an **action**
31+
body's `ctx.session` is a different, untyped object built by `runtime`'s
32+
`buildActionSession()`, and it does populate a `roles` key from `ec.positions`.
33+
It never becomes a HookContext and no schema types it, so it is neither evidence
34+
against this retirement nor fixed by it — it is filed and tracked apart.
35+
36+
**Nothing observable changes.** A key nobody wrote and nothing read cannot alter
37+
a single decision — this is the declaration catching up with the runtime, not a
38+
behaviour change.
39+
40+
FROM → TO:
41+
42+
- `HookContext.session.roles` (`@objectstack/spec/data`) → removed. Delete the
43+
key. To gate a hook on the caller, read `ctx.session.userId` /
44+
`ctx.session.isSystem`; to judge PRIVILEGE, ask the security service, which
45+
evaluates capability grants (`permissions`), placements (`positions`) and the
46+
derived posture off the execution context (ADR-0095 D3) — never a role-name
47+
string comparison.
48+
49+
The retirement kit: the key is **tombstoned**, not deleted, because
50+
`HookContextSchema` is deliberately not `.strict()` (strictness there would turn
51+
an engine-internal enrichment into a breaking change for anyone parsing a
52+
context they were handed, as `provenance` was in #3712) — a plain delete would
53+
have stripped it in silence, the #3733 / ADR-0104 failure. `retiredKey()` gives
54+
both channels instead: `tsc` types the key `never` at any producer, and a parse
55+
raises the prescription itself. There is **no** ADR-0087 D2 conversion and
56+
nothing for `os migrate meta` to rewrite: a HookContext is built per operation
57+
by the engine and never stored, so no `sys_metadata` row, example or template
58+
can carry the key — the `openApi31` (#4579) / `activationEvents` (#4657) shape,
59+
registered as the `hook-context-session-roles-retired` **semantic** migration at
60+
major 17 so the prescription still reaches `spec-changes.json`, the generated
61+
upgrade guide and the `spec_changes` MCP tool. The four export/def ratchets are
62+
unchanged by design: this narrows a nested key inside a surviving def, which
63+
`api-surface.json`, `authorable-surface.json` (whose walk records top-level keys
64+
per def), `api-surface-signatures.json` and `json-schema.manifest.json` are all
65+
blind to — the enum-narrowing reading of the two, not the whole-def one.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
"@objectstack/metadata": patch
3+
---
4+
5+
fix(metadata): `capabilities.write` now means BOTH directions — a writable datasource loader must implement `delete()` (#5276)
6+
7+
`MetadataLoader` declared `save?` and no `delete`, so `capabilities.write` meant
8+
two different things at the two ends of an item's life: to `register()` it meant
9+
"persist into me", and to `unregister()` it guaranteed nothing at all.
10+
`unregister()` duck-typed `delete` at the call site and, when a loader had none,
11+
**silently skipped it** — then dropped the registry entry, invalidated the list
12+
cache and announced a `deleted` event anyway. The caller (Studio/Setup, REST
13+
DELETE, the CLI, a package teardown) was told the delete succeeded while the row
14+
stayed in the loader's store, was read straight back out by the next
15+
`list()`/`get()`, and survived every restart with nothing to retry it.
16+
17+
Two changes, both making the declaration binding instead of decorative:
18+
19+
- **`MetadataLoader` now declares `delete?(type: string, name: string): Promise<void>`.**
20+
The capability is stated on the contract, next to `save?`, instead of being
21+
guessed at by each caller. A loader implemented against the interface can now
22+
see that the method exists.
23+
- **`MetadataManager.registerLoader()` rejects the combination that cannot
24+
honour it.** A loader declaring `protocol: 'datasource:'` **and**
25+
`capabilities.write: true` **without** a `delete()` method is refused at
26+
registration with an error naming the loader, the consequence, and both
27+
repairs. `registerLoader()` is the sole writer of the loader map — the
28+
constructor's `config.loaders` funnel through it — so the combination can no
29+
longer reach the runtime and lose a deletion there.
30+
31+
**Does this affect you?** Only if you register a custom metadata loader that
32+
declares `protocol: 'datasource:'` with `capabilities.write: true`. If it does
33+
and has no `delete()`, registration now throws where it previously succeeded and
34+
quietly discarded your deletions. Two ways to fix it, both stated in the error:
35+
36+
1. implement `async delete(type: string, name: string): Promise<void>` on the
37+
loader, removing the item from its store (`DatabaseLoader` in this package is
38+
the reference implementation); or
39+
2. if the loader is genuinely read-only, declare `capabilities.write: false` — a
40+
read-only `datasource:` loader registers without complaint and is never
41+
written to in the first place.
42+
43+
Loaders on the other protocols (`file:`, `memory:`, `http:`, `s3:`) are
44+
unaffected in either direction: `MetadataManager` never persists to them at
45+
runtime, so it has no deletion of its own to take back, and they may declare
46+
`capabilities.write` without a `delete()` exactly as before. The one
47+
`datasource:` loader shipped in this package, `DatabaseLoader`, has always
48+
implemented `delete()` and is unchanged.
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): `RecordHighlightsField` declares `readonly` (#5176)
6+
7+
The object form of a `record:highlights` entry now declares an optional
8+
`readonly: boolean`. It marks a highlight chip as non-editable — use it for
9+
columns a hook or automation maintains, which must not be hand-edited from the
10+
record header.
11+
12+
```ts
13+
{
14+
type: 'record:highlights',
15+
properties: {
16+
fields: [
17+
'name',
18+
{ name: 'supply_share', type: 'number', readonly: true },
19+
],
20+
},
21+
}
22+
```
23+
24+
**Why this is a spec change and not a renderer detail.** The renderer's
25+
`HeaderHighlight` gate already refuses inline editing on a chip carrying
26+
`readonly`, but the key was not declared here — and the object member is not
27+
`.strict()`, so `RecordHighlightsField` **silently stripped** it:
28+
29+
```
30+
input { fields: [ { name: 'supply_share', readonly: true, type: 'number' } ] }
31+
parsed { fields: [ { name: 'supply_share', type: 'number' } ] }
32+
```
33+
34+
That worked end to end only because per-component props are not parsed on the
35+
live load path today (`PageComponentSchema.properties` is
36+
`z.record(z.string(), z.unknown())`, so the bag rides through untouched). The
37+
moment that gate is wired up, an authored `readonly` becomes either a silent
38+
strip — a machine-owned column quietly editable again, with no diagnostic
39+
anywhere — or a hard parse error. Declaring the key makes the authored
40+
declaration and the enforced behaviour the same fact, which is what ADR-0049
41+
asks for: it is enforced on arrival, not declared-and-inert.
42+
43+
For authors — including AI authors — the key now appears in the generated
44+
component reference, and a misspelling (`readOnly`, `read_only`) is a wrong key
45+
rather than a second de-facto contract the renderer happens to honour.
46+
47+
Purely additive: `readonly` is optional and no default is materialized, so an
48+
entry that does not author it parses exactly as before, and the bare-string form
49+
of a highlight field is unchanged.

0 commit comments

Comments
 (0)