Skip to content

Commit 30fb366

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-18336-retire-walled-legacy-grant-dual-read
2 parents 4da89b8 + aadea24 commit 30fb366

63 files changed

Lines changed: 3349 additions & 432 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: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
**BREAKING** — retire the CEL predicate arms of `ServiceLevelIndicator.successCriteria`
6+
and `TraceSamplingConfig.composite[].condition`, the two observability predicates nothing
7+
ever evaluated.
8+
9+
Both slots were `z.union([<a structured arm>, <the evaluated expression schema>])`. The
10+
expression arm parsed, normalized a bare string to `{ dialect: 'cel', source }`,
11+
registered, and was served back — and **nothing anywhere evaluated it**. An identity scan
12+
over the whole tree finds every hit for `successCriteria`, `ServiceLevelIndicatorSchema`
13+
and `TraceSamplingConfigSchema` outside `packages/spec/src` to be a generated artefact or
14+
prose; inside it the only readers are the schemas' own unit tests and the two census tests
15+
that enumerate expression slots. No service, plugin, runtime or CLI path reads either key.
16+
So an author — very often an AI reading the generated reference page (ADR-0033) — who
17+
wrote `successCriteria: 'p95 < 300ms'` got a green parse and no signal, indistinguishable
18+
from a predicate that ran and answered.
19+
20+
ADR-0049 enforce-or-remove; maintainer ruling 2026-09-18 (director decision batch #160
21+
item 3, letter A). By the standing criterion that a declared-but-unread capability is kept
22+
only when mainstream platforms in the domain have it: application platforms do not carry
23+
SLI success criteria or trace-sampling conditions as authorable application metadata —
24+
that lives in observability infrastructure (SLO products, OTel sampling policy) and is
25+
structured there, not a free expression. The `cron-declared-unwired` family was retired
26+
outright under the same ADR after the same measurement.
27+
28+
## FROM → TO
29+
30+
| you wrote (17.4 and earlier) | write instead |
31+
| --- | --- |
32+
| `successCriteria: 'p95 < 300ms'` | `successCriteria: { threshold: 300, operator: 'lt', percentile: 0.95 }` — the structured rule this slot has always carried |
33+
| `successCriteria: { dialect: 'cel', source: 'p95 < 300ms' }` | the same structured rule; the envelope spelling goes with the bare-string one |
34+
| `condition: 'record.amount > 10'` on a composite sampling branch | `condition: { service: 'api', attributes: { 'http.route': '/v1/orders' } }` — a structured filter object carrying no `dialect` key |
35+
| `condition: { dialect: 'cel', source: 'record.amount > 10' }` | the same structured filter; an object carrying `dialect` is refused as an expression attempt |
36+
37+
**The one-line fix:** delete the predicate and write the structured shape the slot already
38+
carried. A criterion or a sampling rule the structured shape cannot express has no home in
39+
application metadata at all — it belongs in the SLO product or the OpenTelemetry sampler
40+
configuration that actually evaluates it. ⛔ Do not translate a predicate into a threshold
41+
by guessing the number: nothing was evaluating it, so there is no behaviour to preserve and
42+
a wrong number is worse than an absent one.
43+
44+
## The retirement kit
45+
46+
- **Neither KEY is retired — one ARM of each key's union is.** `successCriteria` and
47+
`condition` both survive with their structured arm intact, so `retiredKey()` and an
48+
ADR-0087 D2 strip are both the wrong tool: they retire a key. The prescription hangs on
49+
the surviving schema's own `error` map, dispatched on `issue.input` — the
50+
`HookBodyCapability` / `object.managedBy: 'system'` pattern for a narrowing a key
51+
survives.
52+
- **Where the prescription reaches, measured on zod 4.4.** A schema's `error` map is
53+
consulted for the top-level `invalid_type` a NON-OBJECT raises, and not for the child
54+
issues a wrong-shaped OBJECT raises. So on `successCriteria` the bare-string spelling
55+
carries the prescription and the `{ dialect, source }` envelope is refused by the
56+
structured arm's own missing-key issues (`threshold`, `operator`). On `condition` both
57+
spellings carry it, because the structured arm is a record whose aborting `dialect`
58+
refine sees the object itself. Pinned both ways in the schemas' unit tests, the negative
59+
included: a value refused for a reason that is NOT the retirement must not borrow its
60+
sentence.
61+
- **ADR-0087 disposition: a D3 SEMANTIC entry**, `observability-cel-predicates-retired`,
62+
not a D2 conversion. A predicate is an intent that no threshold/operator pair or
63+
attribute filter records; a mechanical strip would delete what the author meant and leave
64+
no trace of which SLI or which sampling branch lost it — and it would not even be lossless
65+
in the weak sense, because `successCriteria` is REQUIRED (a strip leaves an SLI that no
66+
longer parses) and a composite branch stripped of its `condition` declares no condition at
67+
all. That is the one place this retirement parts company with the two precedents it copies
68+
its MECHANISM from: `crypto.hash` on `HookBodyCapability` and `managedBy: 'system'` both
69+
ALSO registered a D2 conversion, because for each of them a mechanical rewrite existed.
70+
Here none does, which is what makes D3 the right disposition rather than merely an
71+
available one. The prescriptions therefore carry **no** `os migrate meta` sentence — that
72+
sentence is owed only where a conversion covers the surface.
73+
- **The same-major D3 record is absorbed, per the playbook's 「同 major 记账」.** The
74+
`evaluated-expression-slots-source-required` entry landed into this same unpublished step,
75+
and it enumerated these two slots among its 36 declaring positions while instructing the
76+
upgrader to give a sampling `condition` a dialect and a non-blank `source` — the exact
77+
envelope this head now refuses. Both entries first ship together, so the composite of the
78+
two changes is the retirement alone: that entry now reads 34 positions, names the two
79+
absentees and why, and routes them to this retirement instead of to its own repair.
80+
- **The surviving accept sets are pinned beside the refusals.** `successCriteria` still
81+
takes `{ threshold, operator, percentile? }`; a composite `condition` still takes any
82+
filter object carrying no `dialect` key — `{ source: 'x' }` included, because `source`
83+
alone is an ordinary filter key and the retirement narrowed the `dialect` door only.
84+
- **FOUR published JSON Schemas change projection direction**, and it is mechanical rather
85+
than chosen: the retired arm held the last `.transform()` in each of these subtrees, so
86+
each def now projects in output mode instead of falling back to the input shape. All four
87+
lose `x-io: input`, and what each gains differs:
88+
89+
| published schema | gains |
90+
| --- | --- |
91+
| `system/MetricsConfig` | `default: []` on `slis`, plus 8 `required` members |
92+
| `system/TracingConfig` | `default: {"type":"always_on","rules":[]}` on `sampling`, plus 4 `required` members |
93+
| `system/ServiceLevelIndicator` | one `required` member, `enabled` |
94+
| `system/TraceSamplingConfig` | one `required` member, `rules` |
95+
96+
Only the first two carry a `default` move, so only those two are declarable in
97+
`DEFAULT_CHANGES_BY_MAJOR` — the nested pair's `required` growth has no ratchet row to
98+
live in and is stated here instead. A `required` that lists defaulted keys is this repo's
99+
existing output-mode convention, not a new one, and the same-category control
100+
`system/CacheConfig` is untouched. The reference pages show the same signature: the nested
101+
type cells of both pages lose the `?` from their default-bearing keys. **No runtime default
102+
moves** — measured twice, by byte-identity of the untouched `.default(…)` and by parsing a
103+
minimal config on the built package.
104+
105+
## What is deliberately NOT in this change
106+
107+
- **The structured arms.** `{ threshold, operator, percentile }` and the sampling filter
108+
record are equally unread today. The ruling says so and leaves them to their own card:
109+
they carry no dialect and are outside the expression ledger's remit.
110+
- **`skills/objectstack-formula/SKILL.md`**, which still lists `metrics` / `tracing` under
111+
`structured | cel`. The ruling assigns that correction to the skills lane, at tier, and
112+
this diff does not touch it.
113+
- **`packages/spec/src/shared/expression.zod.ts`.** `EvaluatedExpressionInputSchema` is
114+
untouched and stays the schema of every remaining evaluated slot; what left is two
115+
references to it.
116+
117+
Shipped as `minor` under the repo's launch-window convention, in which `major` is refused
118+
by `check-changeset-no-major` and breaking-ness is carried by the banner above plus the
119+
ADR-0087 disposition rather than by the level.
120+
121+
Clause-②: yes (narrowing)
122+
123+
<!-- adr-0087: registered observability-cel-predicates-retired -->
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/objectql": minor
3+
"@objectstack/rest": minor
4+
"@objectstack/metadata-protocol": minor
5+
"@objectstack/lint": minor
6+
"@objectstack/verify": minor
7+
---
8+
9+
The remaining raw `FieldSchema.reference` readers now **REFUSE** a carrier they cannot read, instead of answering "no target" (#18550). The previous release routed the arbiter (`referenceCarrierOf`) and the lint target readers; these were the measured residue of the same ruling — every reader, not just the arbiter.
10+
11+
`FieldSchema.reference` is `z.string().optional()`, so `ObjectSchema.safeParse` refuses an object- or array-valued carrier at the contract door. These reads are the other door: the one a value reaches only when it never went through parse — a hand-built fixture, a raw `registerObject`, a stored row rehydrated past its schema.
12+
13+
**`@objectstack/objectql`** — both of the delete cascade's carrier reads (`planCascadeAtomicity` and `cascadeDeleteRelations`). This is the one with a measurable runtime consequence, and it is why the level is not `patch`:
14+
15+
```
16+
before acct=1 task=1
17+
delete RESOLVED true <- success reported to the caller
18+
after acct=0 task=1 <- an ORPHANED master_detail row
19+
```
20+
21+
An unreadable carrier made the relation invisible to the cascade, so the parent was deleted, the detail row stayed, and the caller was told the delete succeeded — no `restrict` refusal, no `set_null`, nothing logged. It now refuses before any row is touched.
22+
23+
**`@objectstack/rest`** — the public-form lookup picker's field-def fallback. The field def is also hoisted out of the metadata fetch's `catch {}`, so an unreadable carrier is no longer reported as `LOOKUP_TARGET_MISSING`: "no target is declared" and "the declared target cannot be read" want different fixes from whoever owns the metadata.
24+
25+
**`@objectstack/metadata-protocol`** — the seed dependency graph, which also retires an `as string` cast that asserted exactly what its truthiness guard had not checked.
26+
27+
**`@objectstack/lint`** — the four remaining target readers: `masterDetailCount` (`validate-expressions`), the `displayField` consumer edge (`validate-field-consumers`), the field and action-param targets (`validate-object-references`), and `masterOf` (`validate-sharing-rule-enforceability`).
28+
29+
**`@objectstack/verify`**`relationTarget`, which no longer degrades an unreadable carrier to the generic "has no `reference` target" an object with no relationship metadata at all receives.
30+
31+
`null`, `undefined` and `''` are ABSENCE, not a wrong shape, and still answer `undefined` at every one of these sites — a field is allowed to name no target, and `StrictField` declares `reference` nullable. Each site's absence answer is pinned alongside its refusal.
32+
33+
Upgrading: nothing conformant changes. A non-string `reference` could not be authored, stored or parsed before this release either; what changes is that one now fails loudly at the read instead of being read as an absent target. If a test asserted the old silence, assert the refusal instead.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
'@objectstack/cloud-connection': patch
3+
---
4+
5+
docs(cloud-connection): cite the cloud control-plane decisions as `cloud ADR-NNNN` instead of bare numbers that resolve to this repo's own records (#18762)
6+
7+
AGENTS.md Prime Directive 13 is explicit — an ADR "lives in the repository whose
8+
code it governs", and a cloud decision is cited as `cloud ADR-NNNN`, "never as a
9+
bare number, which `scripts/check-adr-anchors.mjs` resolves against *this*
10+
registry (the two number independently)". The rule landed; the stock this
11+
package already carried was never swept.
12+
13+
Read against this repository's registry, the bare numbers pointed at real but
14+
unrelated records:
15+
16+
- `ADR-0008``docs/adr/0008-metadata-repository-and-change-log.md`, *Metadata
17+
Repository, Change Log & Subscription (M0 → M4)* — zero occurrences of
18+
"control plane", "cloud-connection" or "Phase 1"/"Phase 2".
19+
- `ADR-0007``docs/adr/0007-settings-manifest-and-kv-store.md`, *Settings —
20+
Manifest + K/V Store + Resolver*. The cloud ADR-0007 these lines mean is the
21+
one this repo's own ADR-0003 status line already names: the decision that
22+
redefined `sys_package_installation` as management-plane desired state and put
23+
runtime truth in the `LocalManifestSource` ledger.
24+
- `ADR-0009``docs/adr/0009-execution-pinned-metadata.md`, *Execution-Pinned
25+
Metadata* — not the marketplace Setup-navigation ownership decision the lines
26+
describe.
27+
28+
That is worse than citing a number nobody has. A dangling id stops a reader; an
29+
id that resolves lets them believe they read the right page and walk away with
30+
the wrong decision.
31+
32+
18 citations now carry the `cloud` qualifier, in the spelling this package
33+
already used elsewhere for the very same numbers — `cloud ADR-0008` in
34+
`connection-credential-store.ts`, `cloud ADR-0007 step ⑤` in
35+
`local-manifest-source.ts`, `cloud ADR-0009 P2a` in `marketplace-ui.ts`'s own
36+
header. All three numbers already carried both spellings inside this one
37+
package, and `marketplace-ui.ts` carried both inside a single file — qualified in
38+
its header on line 4, bare on lines 16 and 43.
39+
40+
What actually reaches a consumer of this package:
41+
42+
- The npm `description` field, which is the sentence shown on the package page.
43+
- `README.md`, including the closing pointer that already said "in the cloud
44+
repository" while writing the number bare.
45+
- The published `.d.ts`, which carries the module and plugin docblocks.
46+
47+
No behaviour moves. No type, export, route, schema or runtime path is touched —
48+
this is citation spelling and prose only, which is why it ships as a patch rather
49+
than silently. No ADR record is written or edited. `packages/cloud-connection/CHANGELOG.md`
50+
is deliberately untouched: it is published history, and a released entry is
51+
amended in a dedicated docs-only PR, never as a rider on code changes.
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
fix(spec): `spec-changes.json`'s aggregate export diff declares the release pair it really spans (#18978)
6+
7+
Clause-②: yes (widening) — one new OPTIONAL key on a published artifact (`aggregate.surfaceScope`)
8+
and one new optional field on `SpecChangesSchema`. Nothing is renamed, retired or reshaped: the
9+
schema still ACCEPTS a record without it, every existing key keeps its spelling and meaning, and
10+
`perMajor` and the `release` section are byte-identical. Contract-review tier.
11+
12+
`aggregate.added` / `aggregate.removed` are not registry-derived. A release-time api-surface diff
13+
fills them by comparing the artifact being published against the previously **published** one, so
14+
they span **one release** — while the record they sit in is keyed by protocol major (`from: 10,
15+
to: 17`) and every entry carries only `since: 17` / `removedIn: 17`, with
16+
`perMajor[16 → 17].added` at `0` beside it. Nothing in the file distinguished one minor's slice
17+
from the whole major-boundary delta.
18+
19+
Measured on the published `@objectstack/spec@17.4.0` Release asset: `aggregate.added` = **225**,
20+
`aggregate.removed` = **51**, every entry `since`/`removedIn` = 17 — and set-identical to a
21+
recomputed `17.3.0 → 17.4.0` diff of the two tarballs' own `api-surface/` snapshots. It was the
22+
minor's delta wearing a major's label.
23+
24+
**What ships now.** A record whose export arrays are non-empty carries the version pair they were
25+
diffed between:
26+
27+
```bash
28+
jq '.aggregate | {from, to, surfaceScope, added: (.added | length), removed: (.removed | length)}' \
29+
node_modules/@objectstack/spec/spec-changes.json
30+
```
31+
32+
- `surfaceScope: { fromVersion, toVersion }` present ⇒ `added`/`removed` span exactly that
33+
published-version pair. ⛔ They are **not** the `from``to` major delta, and never were.
34+
- `surfaceScope` absent ⇒ the record carries no export diff at all and `added`/`removed` are
35+
empty. ⛔ Read that as "this record does not say", never as "nothing was added between `from`
36+
and `to`" — the same rule the `release` section already states for itself.
37+
- `from` / `to` still answer the major-boundary question for `converted` / `migrated`, which are
38+
registry-derived and unaffected.
39+
40+
**Refused at the producer and at the publish gate, in both directions.** The generator reads the
41+
previous version off the previous artifact's own `package.json`, omits the arrays loudly when it
42+
cannot read one, and refuses outright to write a non-empty unlabelled array.
43+
`scripts/check-release-spec-changes.mjs` — which until now checked the `release` section and not
44+
the aggregate — recomputes the aggregate's claim from the two tarballs and refuses an absent,
45+
mislabelled or untrue scope. Its self-test roster grows from 15 batteries to 23.
46+
47+
**Nothing previously honest moved.** The committed registry-only projection and every `perMajor`
48+
record carry no new key at all; the committed `spec-changes.json` changes on its `$comment` line
49+
and nowhere else. The published schema is deliberately not narrowed — every manifest published so
50+
far carries an unscoped diff and must keep parsing.

0 commit comments

Comments
 (0)